Closed Bug 879375 Opened 11 years ago Closed 11 years ago

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

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 26
Tracking Flags:
Tracking Status
firefox24 --- fixed
firefox25 --- fixed
firefox26 --- affected

People

(Reporter: RyanVM, Assigned: ttaubert)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 2 obsolete files)

add SessionStore.getWindowDimensions() and tabPreviews.capture() to the reflow exclusion list
2.21 KB, patch
dao
: review+
Details | Diff | Splinter Review 
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: nobody → ttaubert
Status: NEW → ASSIGNED
Summary: Intermittent browser_tabopen_reflows.js | unexpected uninterruptible reflow... → Intermittent browser_tabopen_reflows.js | unexpected uninterruptible reflow 'ssi_getWindowDimension@...'
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-
(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.
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.
Depends on: 879733
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.
Hi Tim, any new ideas here?
Flags: needinfo?(ttaubert)
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)
Depends on: 892154
Attachment #773609 - Flags: review?(avihpit) → review?(dao)
Attachment #773609 - Flags: review?(dao) → review+
https://hg.mozilla.org/mozilla-central/rev/16cc89373e12
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Looks like this recently re-broke :(
Flags: needinfo?(ttaubert)
Pushed a small test fix that will fix the regression:

https://hg.mozilla.org/integration/fx-team/rev/103d76816075
Flags: needinfo?(ttaubert)
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
https://hg.mozilla.org/mozilla-central/rev/103d76816075
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Comment 92 is pre-merge of m-c to inbound.
Sadly now not so lucky...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
status-firefox26: fixed → affected
Target Milestone: Firefox 26 → ---
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?
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?
https://hg.mozilla.org/mozilla-central/rev/a90744bf6198
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: