Closed Bug 1358832 Opened 7 years ago Closed 7 years ago

0.98ms uninterruptible reflow at swapDocShells@chrome://global/content/bindings/browser.xml:1414:13

Categories

(Core :: DOM: Core & HTML, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
mozilla55
Iteration:
55.5 - May 15
Performance Impact ?
Tracking Status
firefox55 --- verified
firefox57 --- verified

People

(Reporter: rjward0, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Whiteboard: [ohnoreflow][photon-performance][qa-commented])

Attachments

(1 file)

Here's the stack:

swapDocShells@chrome://global/content/bindings/browser.xml:1414:13
_swapBrowserDocShells@chrome://browser/content/tabbrowser.xml:3145:13
swapBrowsersAndCloseOther@chrome://browser/content/tabbrowser.xml:3042:15
adoptTab@chrome://browser/content/tabbrowser.xml:3583:11
onxbldrop@chrome://browser/content/tabbrowser.xml:6863:11
Flags: qe-verify?
Priority: -- → P2
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf:p1][photon-performance]
Why is this p1?
(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> The actual flush is at:
> http://searchfox.org/mozilla-central/rev/
> 313e5199bf58200f158c6fcbe193e41b88ed58a6/dom/base/nsFrameLoader.cpp#1937
Is that really the right place? Are you using single process FF?

http://searchfox.org/mozilla-central/rev/313e5199bf58200f158c6fcbe193e41b88ed58a6/dom/base/nsFrameLoader.cpp#1494-1495
would be for remote tabs.
Anyhow, the relevant code has been there since swapping frameloaders was added.
bz might recall the reasoning.
Flags: needinfo?(bzbarsky)
I wish I had documented this....

It's possible that there was a weird flash of white or whatnot when dragging between different-sized windows?  Either that or test failures would be my best guesses at this point.

If removing those flushes does not cause orange and if dragging between different-sized windows doesn't show weird visual artifacts, they can probably go away.
Flags: needinfo?(bzbarsky)
ok, thanks. Let me push removal patch to try.
[qf:p1] -> [qf] for reconsideration since (quoting Michael Layzell) "[it] only happens when dragging tabs into other windows, and is such a minor performance hitch".
Whiteboard: [ohnoreflow][qf:p1][photon-performance] → [ohnoreflow][qf][photon-performance]
Comment on attachment 8865637 [details] [diff] [review]
no_flush_in_swap.diff

Can't see relevant failures on try, and locally this looked ok.
But this is stuff which needs some manual testing on all the desktop platforms.
Attachment #8865637 - Flags: review?(bzbarsky)
Assignee: nobody → bugs
Priority: P2 → P1
Comment on attachment 8865637 [details] [diff] [review]
no_flush_in_swap.diff

r=me
Attachment #8865637 - Flags: review?(bzbarsky) → review+
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec8394bff424
don't flush layout when swapping frameloaders, r=bz
https://hg.mozilla.org/mozilla-central/rev/ec8394bff424
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #5)
> I wish I had documented this....
> 
> It's possible that there was a weird flash of white or whatnot when dragging
> between different-sized windows?  Either that or test failures would be my
> best guesses at this point.
> 
> If removing those flushes does not cause orange and if dragging between
> different-sized windows doesn't show weird visual artifacts, they can
> probably go away.

(In reply to Olli Pettay [:smaug] from comment #10)
> Can't see relevant failures on try, and locally this looked ok.
> But this is stuff which needs some manual testing on all the desktop
> platforms.

Marco, can we get some QA help to verify nothing regressed here?
Flags: qe-verify? → qe-verify+
Iteration: --- → 55.5 - May 15
QA Contact: adrian.florinescu
Depends on: 1373220
For QA:

For testing this one, you'll want to test dragging tabs from one window into another pre-existing window.

Might also be worth testing the case where a tab is dragged into a window on a different display.
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf][photon-performance][qa-commented]
Environment:
57.0a1 20170824100243
55.0.2 20170814142917

Ubuntu 16.04, Windows 10x64, Mac OSX 10.12

Tested on: 
-multi-display drag and drop and/or different display resolutions ;
-drag and drop + resizing up and down;
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Component: DOM → DOM: Core & HTML
Performance Impact: --- → ?
Whiteboard: [ohnoreflow][qf][photon-performance][qa-commented] → [ohnoreflow][photon-performance][qa-commented]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: