Issue 1561 - Move unit tests out of the browser`s content area

This patch got a lot larger than I expected, sorry about that. Some unit tests had to be adjusted because the chrome <=> content security boundary moved so e.g. frames created by unit tests need to communicate to the test differently. However, QUnit also got better at catching errors in these frames apparently. Pop-up blocker tests aren't currently e10s-compatible but I plan to address this separately.

[Wladimir Palant, 2014-11-15 23:53:42]

This fixes the remaining issues, only remaining problem being that pop-up
blocker tests still don`t work with e10s (issue with the compatibility shims it
seems). Other than that, all the remaining test issues already existed before.

[tschuster, 2014-11-16 12:47:55]

On 2014/11/15 23:53:42, Wladimir Palant wrote:
> This fixes the remaining issues, only remaining problem being that pop-up
> blocker tests still don`t work with e10s (issue with the compatibility shims
it
> seems). Other than that, all the remaining test issues already existed before.

LGTM. When I first saw this I assumed you moved most of the tests into the child
and would be using the messageManager to communicate. But at least this works
now and they probably wont't break it.

[tschuster, 2014-11-16 12:50:52]

LGTM

http://codereview.adblockplus.org/5745141503492096/diff/5629499534213120/chro...
File chrome/content/settings.xul (right):

http://codereview.adblockplus.org/5745141503492096/diff/5629499534213120/chro...
chrome/content/settings.xul:30: <menuitem value="" label="&tests.all;"/>
This is super awesome, being able to select tests like this.

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
File chrome/content/index.html (right):

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
chrome/content/index.html:28: result = ["tests/" + test + ".js" for (test of
getTests())];
Please do [for (test  off getTests()) "tests/" + test + ".js].
The proposed syntax for ES7 is the reverse of the current one.
http://wiki.ecmascript.org/doku.php?id=harmony:array_comprehensions

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
File chrome/content/tests/policy.js (right):

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
chrome/content/tests/policy.js:83: '</script>',
This kind of stuff would be perfect for the new ES6 templates strings, sadly not
yet in Firefox stable...

[tschuster, 2014-11-16 12:53:54]

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
File chrome/content/tests/elemhide.js (right):

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
chrome/content/tests/elemhide.js:129: frame.addEventListener("frameready",
function()
When I first saw this I didn't immediately think this was a custom event. can
you rename this: abp:frameready or something like that?

[Wladimir Palant, 2014-11-17 14:28:13]

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
File chrome/content/index.html (right):

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
chrome/content/index.html:28: result = ["tests/" + test + ".js" for (test of
getTests())];
On 2014/11/16 12:50:53, tschuster wrote:
> Please do [for (test  off getTests()) "tests/" + test + ".js].
> The proposed syntax for ES7 is the reverse of the current one.
> http://wiki.ecmascript.org/doku.php?id=harmony:array_comprehensions

The link you gave here lists the syntax I used. And while MDN and some other
pages mention the other syntax, I am unable to find an authoritative source for
that. All I've got is that comprehensions *might* be in ES7 and they *might* use
the reverse syntax. But so far I'm not sure whether we should really switch to
the new syntax.

[tschuster, 2014-11-17 16:28:36]

On 2014/11/17 14:28:13, Wladimir Palant wrote:
>
http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
> File chrome/content/index.html (right):
> 
>
http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
> chrome/content/index.html:28: result = ["tests/" + test + ".js" for (test of
> getTests())];
> On 2014/11/16 12:50:53, tschuster wrote:
> > Please do [for (test  off getTests()) "tests/" + test + ".js].
> > The proposed syntax for ES7 is the reverse of the current one.
> > http://wiki.ecmascript.org/doku.php?id=harmony:array_comprehensions
> 
> The link you gave here lists the syntax I used. And while MDN and some other
> pages mention the other syntax, I am unable to find an authoritative source
for
> that. All I've got is that comprehensions *might* be in ES7 and they *might*
use
> the reverse syntax. But so far I'm not sure whether we should really switch to
> the new syntax.

Okay sure, it's probably going to change a few times before it's standardized in
a few years or so ;)

[Wladimir Palant, 2014-11-17 18:58:44]

Ok, if there is no documented decision I'd rather go with the
backwards-compatible variant for now. New patch uploaded.

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
File chrome/content/tests/elemhide.js (right):

http://codereview.adblockplus.org/5745141503492096/diff/5741031244955648/chro...
chrome/content/tests/elemhide.js:129: frame.addEventListener("frameready",
function()
On 2014/11/16 12:53:54, tschuster wrote:
> When I first saw this I didn't immediately think this was a custom event. can
> you rename this: abp:frameready or something like that?

Done.

[Wladimir Palant, 2014-12-11 17:18:25]

Tom, are you fine with the final patch?

[tschuster, 2014-12-13 12:15:25]

On 2014/12/11 17:18:25, Wladimir Palant wrote:
> Tom, are you fine with the final patch?

Yes! Sorry, I must have missed the new patch.
LGTM