1382248 - autoscroll.png is loaded but not shown at browser startup

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[Johann Hofmann [:johannh]]

According to browser_startup_image.js, chrome://global/skin/icons/autoscroll.png is loaded during browser startup on all platforms, but not shown. 

For startup performance and memory usage, we should avoid loading images that aren't going to be displayed.

Comment 1

[Florian Quèze [:florian]]

The straight forward fix is likely to just add the hidden attribute in http://searchfox.org/mozilla-central/rev/a83a4b68974aecaaacdf25145420e0fe97b7aa22/toolkit/content/widgets/browser.xml#1184

But it seems we could completely avoid this code path during startup and create this lazily the first time autoscroll is used.

Comment 2

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1382248 - Lazy load autoscroll.png

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8892141 [details] [diff] [review]
Bug 1382248 - Lazy load autoscroll.png

Review of attachment 8892141 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks Perry!

Comment 4

[Florian Quèze [:florian]]

Do you have a green try run for this? I'm surprised http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/browser/base/content/tabbrowser.xml#2129 doesn't trigger your lazy getter during startup or soon after it.

Comment 5

[Perry Jiang [:perry] [no longer employee, use ni?]]

(In reply to Florian Quèze [:florian] [:flo] from comment #4)
> Do you have a green try run for this? I'm surprised
> http://searchfox.org/mozilla-central/rev/
> b52285fffc13f36eca6b47de735d4e4403b3859e/browser/base/content/tabbrowser.
> xml#2129 doesn't trigger your lazy getter during startup or soon after it.

I took a closer look and it turns out autoscroll.png wasn't loaded at startup but sometime after (without a user's request), so the tests didn't catch that ... will fix that.

Comment 6

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1382248 - avoid loading autoscroll.png at startup

Comment 7

[Florian Quèze [:florian]]

Comment on attachment 8895906 [details] [diff] [review]
Bug 1382248 - avoid loading autoscroll.png at startup

Review of attachment 8895906 [details] [diff] [review]:
-----------------------------------------------------------------

I would prefer a patch that completely avoids any _createAutoScrollPopup call until it's really going to be shown, but I guess this is good enough.

::: toolkit/content/widgets/browser.xml
@@ +1263,5 @@
>            <![CDATA[
>              const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
>              var popup = document.createElementNS(XUL_NS, "panel");
>              popup.className = "autoscroller";
>              // We set this attribute on the element so that mousemove

This comment relates to the 'mousethrough' attribute, so please don't separate it from the popup.setAttribute("mousethrough", "always"); line.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc7527bdb79f
avoid loading autoscroll.png at startup. r=florian

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/49085d3db3c1 for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=123101202&repo=mozilla-inbound

Comment 10

[Perry Jiang [:perry] [no longer employee, use ni?]]

The failure should now be fixed by bug 1371230's patch that makes geo icons load only on their respective platforms.

Comment 11

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1382248 - avoid loading autoscroll.png at startup

Kept the `mousethrough`-related lines together.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6eedff8e9549
avoid loading autoscroll.png at startup. r=florian

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6eedff8e9549