Closed Bug 906814 Opened 11 years ago Closed 11 years ago

Intermittent test_iframe_sandbox_navigation.html | There are 8 navigation tests that should pass - got 7, expected 8

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Version:
24 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla24
Tracking Flags:
Tracking Status
firefox24 --- fixed
firefox25 --- unaffected
firefox26 --- unaffected

People

(Reporter: RyanVM, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure) 

This seems to have started on mozilla-beta (Fx24) on Friday. The only recent landing I can see that might have had an effect on that was the fix for bug 885140 being uplifted there. But we aren't seeing this on m-c or Aurora. Anyway, it appears to be semi-frequent, mostly on Android, but we also hit an occurrence on Linux today.

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

Android Armv6 Tegra 250 mozilla-beta opt test mochitest-2 on 2013-08-16 15:04:15 PDT for push fa0546b58c42
slave: tegra-082

19543 INFO TEST-START | /tests/content/html/content/test/test_iframe_sandbox_navigation.html
19544 INFO TEST-PASS | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | Test 1: this navigation should be allowed by a sandboxed document
19545 INFO TEST-PASS | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | Test 2: this navigation should be allowed by a sandboxed document
19546 INFO TEST-PASS | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | documents should NOT have the same principal if they are sandboxed without allow-same-origin and the first document is navigated to the second
19547 INFO TEST-PASS | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | sandboxed document's attempt to access parent after navigation blocked, as not same-origin.
19548 INFO TEST-PASS | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | allow-same-origin is no longer in effect after reload - parent access blocked.
19549 INFO TEST-PASS | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | top navigation should be allowed by a document sandboxed with 'allow-top-navigation'
19550 INFO TEST-PASS | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | top navigation should be allowed by a document sandboxed with 'allow-top-navigation'
19551 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | There are 8 navigation tests that should pass - got 7, expected 8
19552 INFO TEST-END | /tests/content/html/content/test/test_iframe_sandbox_navigation.html | finished in 9214ms
Perhaps bug 885140 didn't fix the issue with the test completely.  Seems like it fixed it for FF 25 and 26, but not FF 24.  Perhaps because of iframe sandbox code that landed in FF 25?  Perhaps because of changes in the iframe code in general in FF 24 or 25?
If bug 885140 isn't hitting as frequently as this bug is in FF 24 Beta, perhaps we should role back 885140 from beta.
(In reply to Tanvi Vyas [:tanvi] from comment #2)
> Perhaps bug 885140 didn't fix the issue with the test completely.  Seems
> like it fixed it for FF 25 and 26, but not FF 24.  Perhaps because of iframe
> sandbox code that landed in FF 25?  Perhaps because of changes in the iframe
> code in general in FF 24 or 25?

This looks like a different issue, but may have been uncovered by the change for bug 885140.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #0)
> 19551 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/html/content/test/test_iframe_sandbox_navigation.html | There
> are 8 navigation tests that should pass - got 7, expected 8

This is really odd, because there are only 7 that should pass and the "8" in the message "There are 8 navigation tests that should pass ...", comes from a variable that is just set to 7 and then only read.

Before the first attempt to fix this bug it would have been set to 8, could it be that the first patch hasn't been applied, but I would expect this to be failing all the time in that case.
(In reply to Bob Owen from comment #4)
> Before the first attempt to fix this bug it would have been set to 8, could
> it be that the first patch hasn't been applied, but I would expect this to
> be failing all the time in that case.

Found and checked the code ... this is what has happened.
The previous attempted fix for bug 885140 (revision 7ff04bb944aa on inbound), was applied to m-c and Aurora, but not beta.

The new patch without the old one is causing the tests to complete too early and so sometimes it hasn't finished all the tests.
I'm a bit surprised that it merged cleanly without the old patch.
Aha! I did indeed miss the first patch for the beta uplift (explains why it's OK on Aurora - the first patch landed when m-c was still Fx25). We'll see if that fixes it.
https://hg.mozilla.org/releases/mozilla-beta/rev/5954b0946800

As for why it wasn't perma-fail, no clue.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #6)
> Aha! I did indeed miss the first patch for the beta uplift (explains why
> it's OK on Aurora - the first patch landed when m-c was still Fx25). We'll
> see if that fixes it.
> https://hg.mozilla.org/releases/mozilla-beta/rev/5954b0946800

Thanks Ryan.

> 
> As for why it wasn't perma-fail, no clue.

The new patch without the old one, meant it was recording an extra attempted test, which means that it was completing too soon.
When all the tests have been attempted, it waits for a second to give all the "fails if bad" tests a chance to complete.
Some of the time this is enough time for all the tests to pass, sometimes it isn't.
Going to be optimistic and call this fixed.
Status: NEW → RESOLVED
Closed: 11 years ago
status-firefox24: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Thanks for investigating this so quickly Bob!
(In reply to Tanvi Vyas [:tanvi] from comment #10)
> Thanks for investigating this so quickly Bob!

No problem, just happened to have a bit of free time and was sitting here applying the changes I made to tests for bug 886262, to my new tests patch for bug 766282.
You need to log in before you can comment on or make changes to this bug.