1812422 - CSS restyles load images repeatedly

Status: NEW

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Florian Quèze [:florian]]

(Continuing the conversation from the now resolved bug 1812034)

There were multiple cases recently where we noticed images were unexpectedly being loaded as part of restyles triggered by changing CSS variables from JS code.

There was bug 1812034 where image loads were fast.

There was https://github.com/mozilla/pdf.js/issues/15958 (https://share.firefox.dev/3DeNo1g)
There are lots of imgLoader::LoadImage resource://pdf.js/web/images/toolbarButton-*.svg label frames, which I find troubling for a profile of something that only sets a CSS variable to update loading progress. These image loads are not during the "Image Paint" markers, they happen earlier, during the Styles phase. They are very visible in this profile because it was from a local build where omni.ja hasn't be packaged, and we see main thread I/O (nsLocalFile::GetLastModifiedTime).

There was also https://github.com/mozilla/pdf.js/issues/15836 (https://share.firefox.dev/3DfZFCQ) with "imgLoader::LoadImage resource://pdf.js/web/images/shadow.png" happening repeatedly while scrolling on pdf.js.

Is there something that could be optimized in the css platform code to avoid these image loads?

And as a side topic, the profiler has "Image Load" markers recorded at https://searchfox.org/mozilla-central/rev/e1b4aec181f4e630fd9b08ac29c1d00a56ae5eed/image/ImageFactory.cpp#124 but we never have them in the previously mentioned profiles. Is ImageFactory::CreateImage really the right place for these markers?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

If the image is used via a CSS variable like they are, it is expected that every time the element gets restyled it triggers a new load, unfortunately, since it gets substituted again. See also bug 1199054.

That said, for images that are already loaded we shouldn't need to revalidate the image in the image cache, because the image should be in the list of available images.

It seems however that we call ValidateSecurityInfo even if we can reuse the image without validation. That seems unnecessary?

The main question is tho why are we trying to revalidate in the image cache for those resource:// URIs given this code... That might deserve some investigation, and also there seems to be some low-hanging fruit there... Is https://share.firefox.dev/3DfZFCQ on a debug build by any chance? I wouldn't expect so much time to be spent on some of those things...

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Bug 1812488 should make this much cheaper.