The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla13

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ferongr, Assigned: bz)

Tracking

(Blocks: 1 bug)

Trunk
mozilla13
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
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.
Status: UNCONFIRMED → NEW
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.
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)....
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.)

r=dbaron
Attachment #603604 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/83a33626d965
Whiteboard: [need review]
Target Milestone: --- → mozilla13
Flags: in-testsuite-
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f925f2f8d1fd for Windows build bustage
Re-pushed as https://hg.mozilla.org/integration/mozilla-inbound/rev/4bd80687cb32 because the relevant failure had nothing to do with this patch.
https://hg.mozilla.org/mozilla-central/rev/4bd80687cb32
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Blocks: 741556
Depends on: 758885
You need to log in before you can comment on or make changes to this bug.