1051157 - TEST-UNEXPECTED-FAIL, Exception caught: Permission denied to access property 'TestRunner', at: http://test1.example.org/tests/SimpleTest/SimpleTest.js (27), location: http://test1.example.org/tests/js/xpconnect/tests/mochitest/test_bug789713.html

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

This is showing up in all mochitest-4 runs, but is apparently non-fatal. Appears to have started when bug 1049999 landed (visible in the Try run too).

https://tbpl.mozilla.org/php/getParsedLog.php?id=45566252&tree=Mozilla-Inbound

Ubuntu VM 12.04 mozilla-inbound opt test mochitest-4 on 2014-08-08 15:44:17 PDT for push d8b1d2b3e762
slave: tst-linux32-spot-475

16:03:04     INFO -  1791 INFO TEST-START | /tests/js/xpconnect/tests/mochitest/test_bug789713.html
16:03:04     INFO -  1792 INFO TEST-UNEXPECTED-FAIL, Exception caught: Permission denied to access property 'TestRunner', at: http://test1.example.org/tests/SimpleTest/SimpleTest.js (27), location: http://test1.example.org/tests/js/xpconnect/tests/mochitest/test_bug789713.html
16:03:04     INFO -  1793 INFO JavaScript error: http://test1.example.org/tests/SimpleTest/SimpleTest.js, line 48: Permission denied to access property 'TestRunner'
16:03:04     INFO -  1794 INFO TEST-OK | /tests/js/xpconnect/tests/mochitest/test_bug789713.html | took 273ms

Comment 1

[Martijn Wargers (dead)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #0)
> This is showing up in all mochitest-4 runs, but is apparently non-fatal.
> Appears to have started when bug 1049999 landed (visible in the Try run too).

Ugh, yeah, I see it :(
Ahmed, why did this TEST-UNEXPECTED-FAIL not cause tbpl to go orange? That was the purpose of bug 1049999.

I can see why the test is causing this, it should be easy fixable.

Comment 2

[Martijn Wargers (dead)]

Never mind, I understand. Tbpl is relying on the summary to turn orange.
In this case, the summary didn't count this orange, because we're dumping TEST-UNEXPECTED-FAIL directly out, without updating TestRunner UI. But we can't, because we can't access the TestRunner parent window when this happens, because that is what this bug is about.

Perhaps, I misunderstood SimpleTest.js's philosophy and should it never, even when it tries to access a parent Window when it would cause an uncaught exception.

Comment 3

[Martijn Wargers (dead)]

Attachment: 1051157.diff

Probably better to just fix the test.

Comment 4

[Ed Morley [:emorley]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #2)
> Never mind, I understand. Tbpl is relying on the summary to turn orange.
> In this case, the summary didn't count this orange, because we're dumping
> TEST-UNEXPECTED-FAIL directly out, without updating TestRunner UI. But we
> can't, because we can't access the TestRunner parent window when this
> happens, because that is what this bug is about.
> 
> Perhaps, I misunderstood SimpleTest.js's philosophy and should it never,
> even when it tries to access a parent Window when it would cause an uncaught
> exception.

I've filed bug 1051775 for making this turn the run orange; let's use this bug just for the test_bug789713.html fix :-)

Comment 5

[Bobby Holley (:bholley)]

Comment on attachment 8470566 [details] [diff] [review]
1051157.diff

Review of attachment 8470566 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with those fixes.

::: js/xpconnect/tests/mochitest/file_bug789713.html
@@ +36,5 @@
> +  window.parent.postMessage(pass, '*');
> +}
> +
> +// We can't set document.domain on mochi.test, because it's forbidden to set
> +// document.domain to a TLD.

This can all just go into the declarative part now:

<iframe src="file_empty" onload="go()"></iframe>

Make sure to move the <iframe> under the <script> tag when you do that.

::: js/xpconnect/tests/mochitest/test_bug789713.html
@@ +42,4 @@
>      pass = false;
>    } catch (e) { pass = pass && /Permission denied/.exec(e.message); }
>    window.parent.postMessage(pass, '*');
>  }

You can also remove go() now, right?

Comment 6

[Martijn Wargers (dead)]

Attachment: 1051157.diff (for check-in)

This is with the review comments addressed.

(In reply to Bobby Holley (:bholley) from comment #5)
> You can also remove go() now, right?

Doh! Yes.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d541d505f303

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d541d505f303

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b66729c044f0