Closed Bug 732667 Opened 8 years ago Closed 8 years ago

Browser freezes at Github source view when hovering the source with the mouse


(Core :: CSS Parsing and Computation, defect)

Windows XP
Not set





(Reporter: ferongr, Assigned: bzbarsky)


(Blocks 1 open bug, )



(1 file)


1. Visit URL
2. Move mouse in and out of the source area
3. Observe spikes in CPU usage/browser hangs.

I used about:jank and this is the top entry.

372 - c-CSS::ProcessRestyles.
Bug 705877 helps here some, but not enough....

I _think_ the key is still this rule in the github-(hash here).css file:

  tr:hover .line-comments{background:#fafafa!important;}

and this rule in github2-(hash here).css:

  #files tr:hover .add-bubble{opacity:1.0;filter:alpha(opacity=100);}

The big source listing is inside a <div id="files"> and inside a <table class="lines"> in that.  So hovering over it or off it causes a restyle of the whole table row.  But the table only has one row, so we restyle the whole source listing.
Depends on: 705877
Ever confirmed: true
So one interesting thing: on this page, there are no elements matching .line-comments or .add-bubble.

If I change the first rule to "tr:hover * { background:#fafafa!important; }" then I see the same slowness in WebKit (though Opera is still fast).

What WebKit seems to do is to handle :hover changes by flagging the RenderStyle for the thing that the :hover selector was matched against as depending on hover state (just a boolean bit in the RenderStyle).  Then if hover state changes on something that has such a bit set, they recompute style.  So in this case no class="line-comments" means the tr:hover is never even matched and no work needs to be done on hover changes.

This might be worth trying, actually.
Of course if I can figure out what Opera is doing that would be even better.  ;)
Er, nevermind.  I must have been testing Opera on the original page, not my modified one.  With the selector modified to have '*' in it, Opera is likewise slow.
This basically copies how we handle the existing slow-selector flags, but since this really sets flags on an element based on that element itself, would it make more sense to put the bit on the style context?  That would incidentally work better when nodes are moved in the DOM (something the slow selector flags don't handle well because they stick around)....
Attachment #603604 - Flags: review?(dbaron)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Comment on attachment 603604 [details] [diff] [review]
Only do checks for :hover selectors when hover state changes on nodes which have relevant hover rules.

I suppose this seems easier than setting bits on the style context.  (I'm not sure if sibling sharing would be a worry with the style context approach -- it seems unlikely that siblings would have identical style except for susceptibility to :hover styling, though it is possible.)

Attachment #603604 - Flags: review?(dbaron) → review+
Whiteboard: [need review]
Target Milestone: --- → mozilla13
Flags: in-testsuite-
Re-pushed as because the relevant failure had nothing to do with this patch.
Closed: 8 years ago
Resolution: --- → FIXED
Blocks: 741556
You need to log in before you can comment on or make changes to this bug.