Closed Bug 1357061 Opened 7 years ago Closed 7 years ago

0.98ms uninterruptible reflow caused by underflow event at get_scrollClientRect@chrome://global/content/bindings/scrollbox.xml:131:18

Categories

(Firefox :: Tabbed Browser, defect, P1)

Product:
Firefox
Component:
Tabbed Browser
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Iteration:
55.4 - May 1
Project Flags:
Performance Impact ?
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: florian, Assigned: florian)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file)

Patch
1.27 KB, patch
dao
: review+
Details | Diff | Splinter Review 
Here's the stack:

get_scrollClientRect@chrome://global/content/bindings/scrollbox.xml:131:18
ensureElementIsVisible@chrome://global/content/bindings/scrollbox.xml:241:15
onxblunderflow@chrome://global/content/bindings/scrollbox.xml:765:13


This happens when increasing the width of a browser window. I don't understand how this action could cause the current tab to become invisible, because if there's no overflow anymore, all tabs are visible, right?

The relevant code is at http://searchfox.org/mozilla-central/rev/a7334b2896ed720fba25800e11e24952e6037d77/toolkit/content/widgets/scrollbox.xml#762
Flags: qe-verify?
Priority: -- → P2
Not sure. This could be a workaround for weird layout behavior like http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/browser/base/content/tabbrowser.xml#5945-5953
Depends on: 1356705
Flags: needinfo?(dao+bmo)
So should we just try and ask QA to verify with some exploratory testing? Or is bug 1356705 going to fix this?
(In reply to Florian Quèze [:florian] [:flo] from comment #3)
> So should we just try and ask QA to verify with some exploratory testing?

Fine by me.

> Or is bug 1356705 going to fix this?

Given the current WIP patch, it would remove the layout flush, but leave behind this potentially useless code.
Attached patch Patch — — Splinter Review 

        Attachment #8859596 -
        Flags: review?(dao+bmo)

    
  
Assignee: nobody → florian
Status: NEW → ASSIGNED

    
  
Flags: qe-verify? → qe-verify+

        Attachment #8859596 -
        Flags: review?(dao+bmo) → review+
Iteration: --- → 55.4 - May 1
Priority: P2 → P1
QA Contact: adrian.florinescu

    
    

    
  
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e59d06de614
remove potentially useless ensureElementIsVisible call (that currently causes a sync reflow) when an arrowscrollbox handles an underflow event, r=dao.

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/3e59d06de614
Status: ASSIGNED → RESOLVED
Closed: 7 years ago

          status-firefox55:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

    
  
No longer blocks: photon-performance-triage
No longer depends on: 1356705

    
    

    
  
We requested QA here because we don't really know what the lines we removed here were doing. It's code related to one (or more?) tab(s) making us leave the overflow mode on the tab bar.

    
  
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf][photon-performance][qa-commented]

    
    

    
  
Removing the qe+ flag from this issue since the change is from 55 iteration + the areas that might've presented regressions have been already covered in different bug verification or exploratory/regression testing.
Flags: qe-verify+
Flags: qe-verify-
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

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: