Intermittent browser_tabopen_reflows.js | unexpected uninterruptible reflow 'ssi_getWindowDimension@...'

RESOLVED FIXED in Firefox 24

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: RyanVM, Assigned: ttaubert)

Tracking

({intermittent-failure})

Trunk
Firefox 26
intermittent-failure
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox24 fixed, firefox25 fixed, firefox26 affected)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=23770843&tree=Mozilla-Inbound

Ubuntu VM 12.04 mozilla-inbound opt test mochitest-browser-chrome on 2013-06-04 09:51:54 PDT for push ca43cd65708b
slave: tst-linux32-ec2-356

09:56:46     INFO -  TEST-START | chrome://mochitests/content/browser/browser/base/content/test/browser_tabopen_reflows.js
09:56:46     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_tabopen_reflows.js | expected uninterruptible reflow 'stop@chrome://global/content/bindings/browser.xml|addTab@chrome://browser/content/tabbrowser.xml|'
09:56:46     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_tabopen_reflows.js | expected uninterruptible reflow 'openLinkIn@chrome://browser/content/utilityOverlay.js|openUILinkIn@chrome://browser/content/utilityOverlay.js|BrowserOpenTab@chrome://browser/content/browser.js|'
09:56:46  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_tabopen_reflows.js | unexpected uninterruptible reflow 'ssi_getWindowDimension@resource:///modules/sessionstore/SessionStore.jsm|@resource:///modules/sessionstore/SessionStore.jsm|ssi_updateWindowFeatures@resource:///modules/sessionstore/SessionStore.jsm|ssi_collectWindowData@resource:///modules/sessionstore/SessionStore.jsm|@resource:///modules/sessionstore/SessionStore.jsm|ssi_forEachBrowserWindow@resource:///modules/sessionstore/SessionStore.jsm|ssi_getCurrentState@resource:///modules/sessionstore/SessionStore.jsm|ssi_saveState@resource:///modules/sessionstore/SessionStore.jsm|ssi_onTimerCallback@resource:///modules/sessionstore/SessionStore.jsm|ssi_observe@resource:///modules/sessionstore/SessionStore.jsm|'
09:56:46     INFO -  Stack trace:
09:56:46     INFO -      JS frame :: chrome://mochitests/content/browser/browser/base/content/test/browser_tabopen_reflows.js :: observer.reflow :: line 77
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_getWindowDimension :: line 3960
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: <TOP_LEVEL> :: line 2429
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_updateWindowFeatures :: line 2430
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_collectWindowData :: line 2594
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: <TOP_LEVEL> :: line 2466
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_forEachBrowserWindow :: line 3783
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_getCurrentState :: line 2462
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_saveState :: line 3640
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_onTimerCallback :: line 1167
09:56:46     INFO -      JS frame :: resource:///modules/sessionstore/SessionStore.jsm :: ssi_observe :: line 585
09:56:46     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
09:56:46     INFO -  TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_tabopen_reflows.js | expected uninterruptible reflow 'get_scrollPosition@chrome://global/content/bindings/scrollbox.xml|_fillTrailingGap@chrome://browser/content/tabbrowser.xml|_handleNewTab@chrome://browser/content/tabbrowser.xml|onxbltransitionend@chrome://browser/content/tabbrowser.xml|'
09:56:46     INFO -  INFO TEST-END | chrome://mochitests/content/browser/browser/base/content/test/browser_tabopen_reflows.js | finished in 377ms
(Assignee)

Updated

5 years ago
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Summary: Intermittent browser_tabopen_reflows.js | unexpected uninterruptible reflow... → Intermittent browser_tabopen_reflows.js | unexpected uninterruptible reflow 'ssi_getWindowDimension@...'
(Assignee)

Comment 1

5 years ago
Created attachment 758221 [details] [diff] [review]
don't flush layout when getting window dimensions in sessionstore

Window dimensions don't change very often so we don't need to flush layout every time when retrieving the current size. All this data will also be saved when quitting the browser so we should almost never save false dimensions. In case of a crash we don't really care about the unlikely case of restoring the wrong window size if all other data has been restored.
Attachment #758221 - Flags: review?(dao)
Comment on attachment 758221 [details] [diff] [review]
don't flush layout when getting window dimensions in sessionstore

The documentElement's width and height have different semantics than outerWidth/outerHeight, e.g. the latter contain window borders.
Attachment #758221 - Flags: review?(dao) → review-
(Assignee)

Comment 3

5 years ago
(In reply to Dão Gottwald [:dao] from comment #2)
> The documentElement's width and height have different semantics than
> outerWidth/outerHeight, e.g. the latter contain window borders.

I thought so, too but after trying this out I always got the same values for the bounds and outerWidth/Height. What do you mean by window borders? I couldn't find a way to make them yield different values on Linux at least.
(Assignee)

Comment 4

5 years ago
OTOH, I'm totally with you about not changing things that we don't know the whole specifics about. So... we'll hit this probably more often that we want to access some property without causing layout flushes and we can't have a getXWithoutFlushing() for every property out there.

I'm currently investigating a method that suppresses layout flushes. We could call that before accessing such properties and unsuppress flushes afterwards again. Will file a separate bug for it.
(Assignee)

Updated

5 years ago
Depends on: 879733
(Assignee)

Comment 5

5 years ago
Created attachment 758529 [details] [diff] [review]
don't flush layout when getting window dimensions in sessionstore, v2

This patch would use the new API from bug 879733.
Attachment #758221 - Attachment is obsolete: true
(In reply to Tim Taubert [:ttaubert] from comment #3)
> (In reply to Dão Gottwald [:dao] from comment #2)
> > The documentElement's width and height have different semantics than
> > outerWidth/outerHeight, e.g. the latter contain window borders.
> 
> I thought so, too but after trying this out I always got the same values for
> the bounds and outerWidth/Height. What do you mean by window borders?

Borders around the window, rendered by the OS.

> I couldn't find a way to make them yield different values on Linux at least.

Then maybe you just don't have any window borders there or outerWidth/Height don't work as expected on Linux. You can try it on Windows in a window that isn't maximized.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Comment 60

5 years ago
Hi Tim, any new ideas here?
Flags: needinfo?(ttaubert)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 62

5 years ago
Created attachment 773609 [details] [diff] [review]
add SessionStore.getWindowDimensions() and tabPreviews.capture() to the reflow exclusion list

I think that it would be best to add the lines causing reflows to the exclusion list for now and file follow-ups to get rid of them later.
Attachment #758529 - Attachment is obsolete: true
Attachment #773609 - Flags: review?(avihpit)
Flags: needinfo?(ttaubert)
(Assignee)

Updated

5 years ago
Depends on: 892154
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Updated

5 years ago
Attachment #773609 - Flags: review?(avihpit) → review?(dao)

Updated

5 years ago
Attachment #773609 - Flags: review?(dao) → review+
(Assignee)

Comment 66

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/16cc89373e12
(Reporter)

Comment 67

5 years ago
https://hg.mozilla.org/mozilla-central/rev/16cc89373e12
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 71

4 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/ccf2230984bb
status-firefox24: --- → affected
status-firefox25: --- → affected
status-firefox26: --- → affected
Target Milestone: Firefox 25 → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Comment 75

4 years ago
Looks like this recently re-broke :(
Flags: needinfo?(ttaubert)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 88

4 years ago
Pushed a small test fix that will fix the regression:

https://hg.mozilla.org/integration/fx-team/rev/103d76816075
Flags: needinfo?(ttaubert)
(Reporter)

Comment 89

4 years ago
I'll assume that the latest fix doesn't need uplifting since it's a 26-only regression. Thanks!
status-firefox24: affected → fixed
status-firefox25: affected → fixed
status-firefox26: affected → fixed
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/mozilla-central/rev/103d76816075
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Comment hidden (Treeherder Robot)
Comment 92 is pre-merge of m-c to inbound.
Comment hidden (Treeherder Robot)
Sadly now not so lucky...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Updated

4 years ago
status-firefox26: fixed → affected
Target Milestone: Firefox 26 → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 98

4 years ago
Pushed another test fix that takes care of the recent changes for tabPreviews.capture():

https://hg.mozilla.org/integration/fx-team/rev/a90744bf6198
(In reply to Tim Taubert [:ttaubert] from comment #98)
> Pushed another test fix that takes care of the recent changes for
> tabPreviews.capture():

What recent changes?
(Assignee)

Comment 100

4 years ago
Sorry, that should've said the recent changes to its stack trace. It's still the same function causing the reflows but with a slightly different stack trace. It's also possible that something changed in the way stack traces are reported?
Comment hidden (Treeherder Robot)
(Reporter)

Comment 102

4 years ago
https://hg.mozilla.org/mozilla-central/rev/a90744bf6198
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.