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)
Core
DOM: Core & HTML
Tracking
()
People
(Reporter: rjward0, Assigned: smaug)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ohnoreflow][photon-performance][qa-commented])
Attachments
(1 file)
1.65 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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
Updated•7 years ago
|
Flags: qe-verify?
Priority: -- → P2
Comment 1•7 years ago
|
||
The JS code is: http://searchfox.org/mozilla-central/rev/313e5199bf58200f158c6fcbe193e41b88ed58a6/toolkit/content/widgets/browser.xml#1413 this.swapFrameLoaders(aOtherBrowser); The actual flush is at: http://searchfox.org/mozilla-central/rev/313e5199bf58200f158c6fcbe193e41b88ed58a6/dom/base/nsFrameLoader.cpp#1937
Component: Untriaged → DOM
Product: Firefox → Core
Updated•7 years ago
|
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf:p1][photon-performance]
Assignee | ||
Comment 2•7 years ago
|
||
Why is this p1?
Assignee | ||
Comment 3•7 years ago
|
||
(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.
Assignee | ||
Comment 4•7 years ago
|
||
Anyhow, the relevant code has been there since swapping frameloaders was added. bz might recall the reasoning.
Flags: needinfo?(bzbarsky)
Comment 5•7 years ago
|
||
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)
Assignee | ||
Comment 6•7 years ago
|
||
ok, thanks. Let me push removal patch to try.
Assignee | ||
Comment 7•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=df1336b738da2465f7a19961ac8bdf7e43ba7db7
Assignee | ||
Comment 8•7 years ago
|
||
Comment 9•7 years ago
|
||
[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]
Assignee | ||
Comment 10•7 years ago
|
||
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)
Updated•7 years ago
|
Assignee: nobody → bugs
Priority: P2 → P1
Comment 11•7 years ago
|
||
Comment on attachment 8865637 [details] [diff] [review] no_flush_in_swap.diff r=me
Attachment #8865637 -
Flags: review?(bzbarsky) → review+
Comment 12•7 years ago
|
||
Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ec8394bff424 don't flush layout when swapping frameloaders, r=bz
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ec8394bff424
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 14•7 years ago
|
||
(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+
Updated•7 years ago
|
Iteration: --- → 55.5 - May 15
Updated•7 years ago
|
QA Contact: adrian.florinescu
Updated•7 years ago
|
Blocks: photon-perf-tabs
Updated•7 years ago
|
No longer blocks: photon-performance-triage
Updated•7 years ago
|
Blocks: photon-performance-triage
Updated•7 years ago
|
No longer blocks: photon-performance-triage
Comment 15•7 years ago
|
||
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]
Comment 16•7 years ago
|
||
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;
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Updated•2 years ago
|
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.
Description
•