783015 - Chrome refreshdriver runs all the time

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

I was randomly checking where to optimize responsiveness and noticed that the
chrome refresh driver runs all the time at full speed. It happens even if there is
just one Firefox window open and the only page is about:blank.
(in that case the content refreshdriver doesn't run)


(Another thing not clear to me at all is why we have separate refreshdriver for content and chrome)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Looks like we run the driver all the time because there are 2 image requests.
Investigating ...

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

chrome://browser/skin/tabbrowser/connecting.png
chrome://browser/skin/tabbrowser/loading.png
are all the time in the refresh driver.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Not sure if this is a layout bug or Firefox/Toolkit bug.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

These images are used by <xul:image> tags that have a list-style-image assigned. If the tab's not busy they're hidden using 'display: none'.

https://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.css#44

I suspect a layout bug here.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Back to layout. It really should remove imgIRequests from refreshdriver if those won't be painted.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

So I think this is because ImageLoader registers image request even if there isn't any
frames using it.
Trying to fix this.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: WIP

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=43ad063bd61c

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 652268 [details] [diff] [review]
WIP

Breaks animated backgrounds.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

To Kyle who wrote all this new code.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Olli Pettay [:smaug] from comment #9)
> Comment on attachment 652268 [details] [diff] [review]
> WIP
> 
> Breaks animated backgrounds.

Or maybe not. They are broken anyway, Bug 783220

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

(This is killing my battery)

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/?tree=Try&rev=67f1c1ef4586

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

actually use presContext and don't recall the method.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/e33c175f6807

Comment 16

[u279076]

Can someone please advise QA how to verify this bug is fixed for Firefox 17?

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

that is hard. I profiled FF, and added some printfs to refreshdriver to figure out 
this.

but I haven't seen this after the fix.

Comment 18

[u279076]

Okay, thanks Olli. I'm going to flag this [qa-] meaning QA won't allocate resources to trying to verify this fix. If you or someone else finds a way to reliably verify this, please do so.

Thank you.