[tbplbot fails] Intermittent failure in test_suspend.html: wrong url, already suspended, already resumed over and over

RESOLVED FIXED in mozilla2.0b8

Status

()

defect
RESOLVED FIXED
9 years ago
a month ago

People

(Reporter: khuey, Assigned: mstange)

Tracking

({intermittent-failure})

Trunk
mozilla2.0b8
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1282397897.1282398760.29607.gz

s: talos-r3-fed64-030
3440 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/src/threads/test/test_suspend.html | Wrong url! - got http://mochi.test:8888/tests/dom/src/threads/test/suspend_iframe.html, expected "about:blank"
3441 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/src/threads/test/test_suspend.html | Already suspended? - got true, expected false
3442 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/src/threads/test/test_suspend.html | Already resumed? - got true, expected false

repeated a few hundred times.
We also hit this several times a day on various non-release trees, but I hadn't noticed that tbpl was failing to actually comment when it's starred.
Summary: Intermittent failure in test_suspend.html: wrong url, already suspended, already resumed over and over → [tbplbot fails] Intermittent failure in test_suspend.html: wrong url, already suspended, already resumed over and over
(Assignee)

Updated

9 years ago
Assignee: nobody → mstange
Status: NEW → ASSIGNED
(Assignee)

Updated

9 years ago
Blocks: 598482
(Assignee)

Comment 16

9 years ago
Posted patch add a flushSplinter Review
With my patch from bug 598482 I hit this failure on all platforms consistently.

The problem was that calling iframe.history.back() didn't restore test_suspend.html from bfcache, but reloaded it instead. (badOnloadCallback didn't catch the reload because it wasn't attached as a load handler correctly.)

Why didn't test_suspend.html come from the bfcache? Because an attribute change had kicked it out. This attribute change was caused by a delayed reflow flush. Without the patch from bug 598482, anything that causes invalidations causes reflows to be flushed pretty much instantly (one trip through the event loop). But with that patch, they are flushed from the refresh driver timer, and that can be too late.
When is "too late"? When the new document is shown, because that's when the content viewer of the previous document is stored on the session history entry. Any change to the stored document after that point will cause the entry to drop the cached content viewer.

(How can a reflow change attributes? For example through nsGfxScrollFrameInner::FinishReflowForScrollbar...)
Attachment #493291 - Flags: review?(bent.mozilla)
(Assignee)

Comment 17

9 years ago
Maybe nsDocumentViewer should do the flush before it adds itself to the session history entry. Boris, do you think that's a good idea?
Comment on attachment 493291 [details] [diff] [review]
add a flush

Neat! Thanks for digging in to this!
Attachment #493291 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 19

9 years ago
http://hg.mozilla.org/mozilla-central/rev/4d4065d46e80
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
> Maybe nsDocumentViewer should do the flush before it adds itself

No, that seems like a terrible idea; it introduces another place script can run during the unload sequence, and does work we don't really need to do in the common case of not coming back to the page.

Are we really hitting a reflow from the refresh driver?  The refresh driver should be suspended while the page is in bfcache...  See nsRefreshDriver::Freeze.  Is that not working?

Also, perhaps we should special-case scrollbar attr changes in the shentry mutation observers and not drop bfcache for those.
Whiteboard: [orange]
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.