Closed Bug 1382248 Opened 2 years ago Closed 2 years ago

autoscroll.png is loaded but not shown at browser startup

Categories

(Firefox :: General, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 57
Iteration:
57.2 - Aug 29
Tracking Status
firefox57 --- fixed

People

(Reporter: johannh, Assigned: perry)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [reserve-photon-performance])

Attachments

(1 file, 2 obsolete files)

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.
Whiteboard: [photon-performance] [triage]
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.
Whiteboard: [photon-performance] [triage] → [reserve-photon-performance]
Flags: qe-verify?
Priority: -- → P3
Assignee: nobody → jiangperry
Status: NEW → ASSIGNED
Priority: P3 → P1
Attachment #8892141 - Flags: review?(mconley)
Flags: qe-verify? → qe-verify-
Comment on attachment 8892141 [details] [diff] [review]
Bug 1382248 - Lazy load autoscroll.png

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

Great, thanks Perry!
Attachment #8892141 - Flags: review?(mconley) → review+
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.
(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.
Attachment #8895906 - Flags: review?(florian)
Attachment #8892141 - Attachment is obsolete: true
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.
Attachment #8895906 - Flags: review?(florian) → review+
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc7527bdb79f
avoid loading autoscroll.png at startup. r=florian
The failure should now be fixed by bug 1371230's patch that makes geo icons load only on their respective platforms.
Flags: needinfo?(jiangperry)
Kept the `mousethrough`-related lines together.
Attachment #8895906 - Attachment is obsolete: true
Attachment #8897959 - Flags: review+
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6eedff8e9549
avoid loading autoscroll.png at startup. r=florian
https://hg.mozilla.org/mozilla-central/rev/6eedff8e9549
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Iteration: --- → 57.2 - Aug 29
You need to log in before you can comment on or make changes to this bug.