Closed
Bug 1362542
Opened 8 years ago
Closed 7 years ago
4.26ms uninterruptible reflow at _updateTabsVisibilityStatus@chrome://browser/content/tabbrowser.xml:7481:1
Categories
(Firefox :: Tabbed Browser, enhancement, P1)
Firefox
Tabbed Browser
Tracking
()
People
(Reporter: sfleiter, Assigned: dao)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ohnoreflow][reserve-photon-performance])
Attachments
(1 file)
Here's the stack:
_updateTabsVisibilityStatus@chrome://browser/content/tabbrowser.xml:7481:1
onxblpopupshowing@chrome://browser/content/tabbrowser.xml:7591:11
Reporter | ||
Comment 1•8 years ago
|
||
Happens when clicking on the down arrow (listing open tabs) right next to the new tab button every time for me
Updated•8 years ago
|
Component: Untriaged → Tabbed Browser
Updated•8 years ago
|
Flags: qe-verify?
Priority: -- → P2
Updated•8 years ago
|
Blocks: photon-perf-tabs
Updated•8 years ago
|
No longer blocks: photon-performance-triage
Updated•8 years ago
|
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf:p3][photon-performance]
Updated•8 years ago
|
Flags: qe-verify? → qe-verify-
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(dao+bmo)
Updated•8 years ago
|
Priority: P2 → P3
Whiteboard: [ohnoreflow][qf:p3][photon-performance] → [ohnoreflow][qf:p3][reserve-photon-performance]
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(dao+bmo)
Updated•8 years ago
|
Priority: P3 → P1
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8871655 [details]
Bug 1362542 - Avoid flusing layout when updating tabs' visiblity status for the overflow menu.
https://reviewboard.mozilla.org/r/143176/#review147528
Thanks for looking at this! The patch looks reasonable, but I have a question:
_updateTabsVisibilityStatus is called both from the popupshowing event handler, and from a 'scroll' event listener. For the popupshowing case, the user hasn't interacted very recently with the tab strip so I think there's no doubt that getBoundsWithoutFlushing will get correct results. For the case where the user scrolls through the tab strip while the panel is open, I'm less sure (I can't test this on Mac where the native menu popup eats all mouse events). Have you verified that scrolling the tabstrip still updates the visible tabs correctly? If not, do we need to call _updateTabsVisibilityStatus from a MozAfterPaint listener for the 'scroll' case?
Unrelated to what the patch changes: I wonder how this code performs when there are many tabs. Would adding a binary search for the position of the first visible tab be worth the effort (potentially in a follow-up)?
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Florian Quèze [:florian] [:flo] (away until Monday May 29th) from comment #4)
> Comment on attachment 8871655 [details]
> Bug 1362542 - Avoid flusing layout when updating tabs' visiblity status for
> the overflow menu.
>
> https://reviewboard.mozilla.org/r/143176/#review147528
>
> Thanks for looking at this! The patch looks reasonable, but I have a
> question:
>
> _updateTabsVisibilityStatus is called both from the popupshowing event
> handler, and from a 'scroll' event listener. For the popupshowing case, the
> user hasn't interacted very recently with the tab strip so I think there's
> no doubt that getBoundsWithoutFlushing will get correct results. For the
> case where the user scrolls through the tab strip while the panel is open,
> I'm less sure (I can't test this on Mac where the native menu popup eats all
> mouse events). [...]
This doesn't work on any platform. I'll remove the scroll event listener.
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8871655 [details]
Bug 1362542 - Avoid flusing layout when updating tabs' visiblity status for the overflow menu.
https://reviewboard.mozilla.org/r/143176/#review147666
Thanks!
Attachment #8871655 -
Flags: review?(florian) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/90075cb286d6
Avoid flusing layout when updating tabs' visiblity status for the overflow menu. r=florian
Comment 9•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•7 years ago
|
Iteration: --- → 55.7 - Jun 12
Updated•3 years ago
|
Performance Impact: --- → P3
Whiteboard: [ohnoreflow][qf:p3][reserve-photon-performance] → [ohnoreflow][reserve-photon-performance]
You need to log in
before you can comment on or make changes to this bug.
Description
•