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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[ferongr]

STR

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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

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.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Of course if I can figure out what Opera is doing that would be even better.  ;)

Comment 4

[Boris Zbarsky [:bzbarsky]]

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.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Only do checks for :hover selectors when hover state changes on nodes which have relevant hover rules.

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)....

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.)

r=dbaron

Comment 7

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83a33626d965

Comment 8

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f925f2f8d1fd for Windows build bustage

Comment 9

[Boris Zbarsky [:bzbarsky]]

Re-pushed as https://hg.mozilla.org/integration/mozilla-inbound/rev/4bd80687cb32 because the relevant failure had nothing to do with this patch.

Comment 10

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/4bd80687cb32