Last Comment Bug 732667 - Browser freezes at Github source view when hovering the source with the mouse
: Browser freezes at Github source view when hovering the source with the mouse
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla13
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
https://github.com/MayhemYDG/4chan-x/...
Depends on: 705877 758885
Blocks: 741556
  Show dependency treegraph
 
Reported: 2012-03-02 22:25 PST by ferongr
Modified: 2012-10-09 08:57 PDT (History)
7 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Only do checks for :hover selectors when hover state changes on nodes which have relevant hover rules. (3.64 KB, patch)
2012-03-06 22:44 PST, Boris Zbarsky [:bz] (Out June 25-July 6)
dbaron: review+
Details | Diff | Review

Description ferongr 2012-03-02 22:25:04 PST
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 [:bz] (Out June 25-July 6) 2012-03-02 22:49:31 PST
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 [:bz] (Out June 25-July 6) 2012-03-02 23:03:51 PST
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 [:bz] (Out June 25-July 6) 2012-03-02 23:04:10 PST
Of course if I can figure out what Opera is doing that would be even better.  ;)
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-03-02 23:27:29 PST
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 [:bz] (Out June 25-July 6) 2012-03-06 22:44:31 PST
Created attachment 603604 [details] [diff] [review]
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-7 (review requests must explain patch) 2012-03-12 20:55:01 PDT
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 [:bz] (Out June 25-July 6) 2012-03-12 21:01:26 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/83a33626d965
Comment 8 Phil Ringnalda (:philor) 2012-03-12 22:14:27 PDT
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f925f2f8d1fd for Windows build bustage
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-03-12 22:58:15 PDT
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] 2012-03-13 06:00:38 PDT
https://hg.mozilla.org/mozilla-central/rev/4bd80687cb32

Note You need to log in before you can comment on or make changes to this bug.