Right now the CSS loader cache is per-document. We should be able to cache the parsed stylesheets in the same docgroup, which should speed up navigations, etc.

Some things so I don't forget:

• If we want to do this for inline sheets, we need to either key it off the base URI, or track whether there were relative URIs, or something.
• That's not an issue for <link> because the base uri is the sheet uri, IIRC, but worth double-checking.

Err, someday I'll learn how to write.

TODO:

• Proper base URI handling (both for inline and out-of-line styles).
• Eviction.

So I have a patch that mostly work, that is on the phab revision above.

Try run is here. There are some remaining issues:

• There was a bug with constructable stylesheets which is fixed already.
• Quite a few leaks-until-shutdown that I'll have to investigate, but I don't expect it to be too terrible.
• Some tests that rely on us not hitting the cache, which should be fixed.

So over-all looks pretty promising.

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=d10f4829f30e01ff5edbc8c0fc18095ea548e64a&newProject=try&newRevision=2f3b40532ee54ebcb3f1b640410f817b54aa93c3&framework=1
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=d10f4829f30e01ff5edbc8c0fc18095ea548e64a&newProject=try&newRevision=2f3b40532ee54ebcb3f1b640410f817b54aa93c3&framework=10

Looking really good (note that the "replayed" / "non-replayed" regressions aren't really regressions, see bug 1640984).

Otherwise the stylesheet loads that are supposed to take long are
instant and the test fails when some of the other tests are run first.

Use a different URL so as to not hit the cache.

Trying to use nsTHashtable<SheetLoadDataHashKey> yields the following
error:

0:01.28 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:288:8: error: multiple overloads of 'RemoveEntry' instantiate to the same signature 'void (mozilla::SheetLoadDataHashKey )'
0:01.28 void RemoveEntry(EntryType aEntry) { mTable.RemoveEntry(aEntry); }
0:01.28 ^
0:01.28 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/mozilla/css/Loader.h:600:38: note: in instantiation of template class 'nsTHashtable<mozilla::SheetLoadDataHashKey>' requested here
0:01.28 nsTHashtable<SheetLoadDataHashKey> mLoadsPerformed;
0:01.28 ^
0:01.28 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:264:8: note: previous declaration is here
0:01.28 void RemoveEntry(KeyType aKey) {

I tried and failed to std::enable_if my way to victory, but then
realized I could probably use nsTHashtable<const SheetLoadDataHashKey>,
so that the compiler wouldn't complain about the overload (they're
identical, so they don't really matter).

But doesn't work either, unless I have this patch, because:

0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:504:36: error: call to implicitly-deleted copy constructor of 'const mozilla::SheetLoadDataHashKey'
0:12.57   new (mozilla::KnownNotNull, aTo) EntryType(std::move(*fromEntry));
0:12.57                                    ^         ~~~~~~~~~~~~~~~~~~~~~
0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:478:13: note: in instantiation of member function 'nsTHashtable<const mozilla::SheetLoadDataHashKey>::s_CopyEntry' requested here
0:12.57           : s_CopyEntry,
0:12.57             ^
0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:163:16: note: in instantiation of member function 'nsTHashtable<const mozilla::SheetLoadDataHashKey>::Ops' requested here
0:12.57       : mTable(Ops(), sizeof(EntryType), PLDHashTable::kDefaultInitialLength) {}
0:12.57                ^
0:12.57 /home/emilio/src/moz/gecko-2/layout/style/Loader.cpp:359:9: note: in instantiation of member function 'nsTHashtable<const mozilla::SheetLoadDataHashKey>::nsTHashtable' requested here
0:12.57 Loader::Loader()
0:12.57         ^
0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/mozilla/css/Loader.h:97:3: note: copy constructor is implicitly deleted because 'SheetLoadDataHashKey' has a user-declared move constructor
0:12.57   SheetLoadDataHashKey(SheetLoadDataHashKey&& toMove)
0:12.57   ^
0:13.08 1 error generated

This one was easier to fix though. Not too proud of this one.

This matches other browsers, our current behavior, and the test
expectations from various WPTs.

Depends on D77457

So that they keep testing the network cache / url classifier / etc...

Depends on D77842

https://hg.mozilla.org/mozilla-central/rev/13966685242f
https://hg.mozilla.org/mozilla-central/rev/3b1c5204e833

This test checks for the protocol field of the performance entry,
which will not be set if the link comes from the new shared sheet cache.

This subtest is doing a preload, then document.write()-ing the meta
referrer, and checking that we do two loads, not one.

We're intentionally not including the referrer in the cache key anymore,
as otherwise that'd defeat the cross-document sharing, so I think we
should just change the expectation of this test / remove it.

This is a somewhat similar situation to bug 1642325, fwiw :)

This test is using async_test, while the others are using promise_test,
which are not synchronized with each other.

This test loads a <link> element with the import url stylesheet, which
sets the color property. Color is inherited, so it can affect the
shadow descendants of the trees in the other tests.

Only the following test checks the computed value of the color property,
so in practice the race doesn't happen right now because the url is
never cached.

But with the stylesheet caching improvements, if you hit the cache for
the link, it is fast enough in a way in which you might be running the
following test after the link has loaded, but before the load event
has been dispatched.

Use promise_test to properly synchronize between this test and the
following ones, to avoid flaky failures with cross-document stylesheet
caching.

Same issue, if the link is cached, it'll load instantly and fail the
test.

https://hg.mozilla.org/mozilla-central/rev/79c2788498a8

The patches should be green now btw.

Make the stylesheet cache respect the same headers as the image cache
does. This makes no-cache stylesheets work as they do now, which is
useful for developers that want to develop sites locally, and for
shift-reloads, etc.

Depends on D77842

This basically undoes D77842, but it was better done on top than just
removing the patch from the stack. I could squash them if desired.

The previous patch to respect caching headers makes tests much much more
happy, to the point where I'm not sure whether we really need this or
not. Your call whether we should keep it or not.

Depends on D78659

DAMP refreshes the page that it loads, but doesn't clear the stylesheet
cache, so the second+ time around some of the zoom messages will not be
sent.

This is actually a wider issue, that I plan to fix in bug 1645180.
Meanwhile don't wait for these messages.

Backed out for damp failures.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=edb0e17f3c984994dec1ba84f509e78428de022b&searchStr=damp&selectedTaskRun=O9pxz4ktTZ-hcVgFqtOJ-w.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306016743&repo=autoland&lineNumber=1128

Backout: https://hg.mozilla.org/integration/autoland/rev/feb96d158c7e226802aa26326b789c01b35f017b

The complicated test loads a lot of iframes. Now that with my changes we
coalesce stylesheet loads across documents it's expected to have way
less network loads for this test, which has a lot of facebook iframes
that load multiple stylesheets each.

The value is the one that made it reliably pass on my machine.

https://hg.mozilla.org/mozilla-central/rev/c5b351d19a30
https://hg.mozilla.org/mozilla-central/rev/a86e9b4f395f
https://hg.mozilla.org/mozilla-central/rev/f4e67ab1b25f
https://hg.mozilla.org/mozilla-central/rev/68c388ffb9e2
https://hg.mozilla.org/mozilla-central/rev/3545a36ff2c8
https://hg.mozilla.org/mozilla-central/rev/7c0cd0b64399
https://hg.mozilla.org/mozilla-central/rev/b704a8ee697c
https://hg.mozilla.org/mozilla-central/rev/419a897d7e4b

(In reply to Pulsebot from comment #29)

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3f28494f0e7
Allow caching stylesheets across documents. r=heycam
https://hg.mozilla.org/integration/autoland/rev/466cfd0ad5db
Add a performance resource entry when hitting the cache for the first time
in a document. r=mayhemer,valentin
https://hg.mozilla.org/integration/autoland/rev/c7ef05dae614
Better integration of the shared stylesheet cache with the network cache.
r=tnikkel,mayhemer,heycam
https://hg.mozilla.org/integration/autoland/rev/0b0ff0ad8db7
Stop synthesizing performance timing entries for cache sheets. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/ce1521b895e1
Clear the shared stylesheet cache in some mochitests. r=mayhemer,robwu
https://hg.mozilla.org/integration/autoland/rev/64b42ac358e6
Add a test for the interaction between webRequest and the stylesheet cache.
r=robwu

== Change summary for alert #26229 (as of Tue, 16 Jun 2020 03:38:17 GMT) ==

Improvements:

12% about_newtab_with_snippets linux64-shippable-qr opt e10s stylo 83.14 -> 72.83
8% about_newtab_with_snippets linux64-shippable opt e10s stylo 75.76 -> 69.52
5% about_newtab_with_snippets linux64-shippable-qr opt e10s stylo 76.61 -> 72.43
2% tp5o_webext linux64-shippable opt e10s stylo 287.94 -> 282.15

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26229

(In reply to Razvan Maries from comment #32)

Backout: https://hg.mozilla.org/integration/autoland/rev/feb96d158c7e226802aa26326b789c01b35f017b

== Change summary for alert #26191 (as of Fri, 12 Jun 2020 11:10:02 GMT) ==

Regressions:

13% about_newtab_with_snippets linux64-shippable-qr opt e10s stylo 72.86 -> 82.46
11% about_newtab_with_snippets linux64-shippable opt e10s stylo 69.57 -> 77.42

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26191

== Change summary for alert #26234 (as of Tue, 16 Jun 2020 13:00:37 GMT) ==

Improvements:

12% about_newtab_with_snippets linux64-shippable-qr opt e10s stylo 82.93 -> 72.98
9% about_newtab_with_snippets linux64-shippable opt e10s stylo 76.16 -> 69.52

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26234

Emilio: was this disabled on nightly recently? Was this the cause of the perf improvement around june 15th?
see from matrix:

• and the warm page load improvement returned to the original baseline yesterday: https://treeherder.mozilla.org/perf.html#/graphs?highlightAlerts=1&series=mozilla-central,1938343,1,10&series=mozilla-central,1938355,1,10&series=autoland,2134505,1,10&series=autoland,2134378,1,10&series=mozilla-central,2531981,1,10&series=mozilla-central,2531403,1,10&timerange=2592000

Hmm, maybe https://hg.mozilla.org/mozilla-central/rev/061e049e9546d8e40877e872d7c0c8ddc2c3c74c explains that?

It seems plausible the improvement comes from this bug. But that patch is a correctness fix. My guess is that we run tests with the cache disabled (no-cache headers or such), but that bug was causing us to hit the cache anyway, which translated to that improvement. It should be easy to confirm that hypothesis.

Randell, do you have time to push to try with https://hg.mozilla.org/mozilla-central/rev/061e049e9546d8e40877e872d7c0c8ddc2c3c74c reverted and see if that explains it? Otherwise ni? me back and I'm happy to do that.

I checked, and we do hit that in ebay.com. In particular, in the test there's a massive stylesheet from https://ir.ebaystatic.com/rs/v/qgbfiofgwa4elldzxh2gc202fyf.css?proc=DU:N which has the following in the response:

• cache-control: public, max-age=31536000, immutable

But also:

• pragma: no-cache

(Which is really odd)

So we hit the "need to re-do the request over again". That combination of headers is really odd. Honza, should we ignore the pragma header if cache-control is specified?

Note that I can't reproduce the same in the "live" ebay.com site. Loading https://ir.ebaystatic.com/rs/v/3ecneapu2m1mza1q2lfy2ir3jqi.css?proc=DU:N we do hit the cache and it doesn't have the pragma header.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #42)

I checked, and we do hit that in ebay.com. In particular, in the test there's a massive stylesheet from https://ir.ebaystatic.com/rs/v/qgbfiofgwa4elldzxh2gc202fyf.css?proc=DU:N which has the following in the response:

• cache-control: public, max-age=31536000, immutable

But also:

• pragma: no-cache

(Which is really odd)

So we hit the "need to re-do the request over again". That combination of headers is really odd. Honza, should we ignore the pragma header if cache-control is specified?

According that Pragma is an old (and somewhat deprecated) technology, Cache-control should take a priority. In this case it's definitely a web site/server config bug. But it clearly is trying to say "this is really cacheable for a long time."

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #39)

Emilio: was this disabled on nightly recently? Was this the cause of the perf improvement around june 15th?
see from matrix:

• and the warm page load improvement returned to the original baseline yesterday: https://treeherder.mozilla.org/perf.html#/graphs?highlightAlerts=1&series=mozilla-central,1938343,1,10&series=mozilla-central,1938355,1,10&series=autoland,2134505,1,10&series=autoland,2134378,1,10&series=mozilla-central,2531981,1,10&series=mozilla-central,2531403,1,10&timerange=2592000

As expected the graph went back down after bug 1649807 landed.

This came up on SUMO (1297319): F5 doesn't load the latest style sheet changes from the webserver even with

• browser.cache.check_doc_frequency => 1
• browser.cache.disk.enable => false

It seems the developer needs to take extra steps to view the changes, some of which could impact other users:

(A) Use Ctrl+F5 (or Shift+click the Reload button, or press Ctrl+Shift+r), which is wasteful with other resources and a new habit to learn

(B) Set devtools.cache.disabled => false in about:config, which is wasteful with other resources

(C) Serve .css files with cache-control: no-store during development

(D) Modify the link tag href with something like ?timestamp to bypass the cache

I don't know the best way forward on this issue, but would it be appropriate to document the change on the release notes for now: https://developer.mozilla.org/docs/Mozilla/Firefox/Releases/79

Could you file a separate bug for that? browser.cache.check_doc_frequency=1 seems something that we should check for this stuff.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #48)

Could you file a separate bug for that? browser.cache.check_doc_frequency=1 seems something that we should check for this stuff.

Okay, I'm sure it's not artfully described, but: bug 1656630

(In reply to jscher2000 from comment #47)

I don't know the best way forward on this issue, but would it be appropriate to document the change on the release notes for now: https://developer.mozilla.org/docs/Mozilla/Firefox/Releases/79

Hopefully this "Caching change" subsection is sensible enough as a stopgap until someone who understand it better can make any necessary revisions:

https://wiki.developer.mozilla.org/docs/Mozilla/Firefox/Releases/79#CSS

So... now when I press Alt + Tab, F5, nothing changes. This is driving me bonkers. I can't think of something I do more often, and rely on for its quickness than this keystroke sequence.

Were we thinking about developers at all? I have lost hours already.

Let me know if there's some way I can help.

(In reply to costa from comment #51)

Let me know if there's some way I can help.

Are you on Firefox 80? Does it work on Nightly? I think you're describing bug 1664405 (we'd incorrectly cache some no-cache responses). It will be fixed in 81.

If you're seeing something that doesn't work on Nightly please file a bug and cc me or drop the link here and I'll take a look.

I'm on Firefox 80 on Ubuntu 18.04. I just checked in Nightly. The issue remains.

Steps to reproduce:

1. Make any HTML page with linked CSS stylesheet.
2. Serve the page up with a simple HTTP server on localhost.
3. Visit the page on localhost.
4. Change the background color or any other CSS property.
5. Go back to the browser and press F5 (refresh).

Behavior: The page loads with the same background color and no sign of the CSS edit.

Expected behavior: The page loads with the new background color.(In reply to Emilio Cobos Álvarez (:emilio) from comment #52)

(In reply to costa from comment #51)

Let me know if there's some way I can help.

Are you on Firefox 80? Does it work on Nightly? I think you're describing bug 1664405 (we'd incorrectly cache some no-cache responses). It will be fixed in 81.

If you're seeing something that doesn't work on Nightly please file a bug and cc me or drop the link here and I'll take a look.

The copy here: https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/79#CSS seems to shed some light. "A simple reload will not revalidate CSS..."

(In reply to costa from comment #54)

The copy here: https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/79#CSS seems to shed some light. "A simple reload will not revalidate CSS..."

That is not true. Reloads should revalidate CSS if the server says so. Please file a separate bug and please specify which server are you using for serving the web page and the headers the stylesheet gets served with if possible too.

The Node simple server: https://www.npmjs.com/package/http-server
And the Python simple server: https://www.askpython.com/python-modules/python-httpserver

Both produce this bug.

If I open the web dev tools in Firefox a normal refresh works fine and reloads CSS. It only fails when the dev tools are closed. So, if I open the network tab, then everything is there as needed.

I'm just trying to help. Hope this is helpful. I really really really don't want to switch to Chrome. Ugh.... I hate the idea of only one browser in the world.

Please do file a new bug, discussing on closed bugs is not useful. I can't reproduce with python -m http.server.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #55)

(In reply to costa from comment #54)

The copy here: https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/79#CSS seems to shed some light. "A simple reload will not revalidate CSS..."

That is not true. Reloads should revalidate CSS if the server says so.

Simple reloads revalidate all the other files by re-requesting them with an If-Modified-Since header. It is only CSS files that are the exception now, requiring developers either to press Ctrl+F5 instead of F5 or to disable cache during development.

Usually a server will not specify a short duration in its cache-control header for a style sheet. Adding the following in a .htaccess file will allow F5 to revalidate .css files (for a webapp residing in a single directory on the server):

<FilesMatch "\.css$">
    Header set Cache-Control "public, max-age=0"
</FilesMatch>

So that is a potential workaround, and since regular visitors to the site also will have enhanced caching of CSS, perhaps it is a change that also would benefit other Firefox users affected by this change.

Ctrl + F5 changes the scroll position. Doesn't really work. I guess I can fiddle with my server. My best suggestion is to make an exception for localhost, though I can see how that might cause problems.

I invalidate my cache when I push updates to my sites generally, so my users won't have any issues, and will benefit from the caching.

Development is just hampered in an odd way.

My cache is disabled, but it seems this options in the dev tools is only for when the toolbox is open. Is there a way to make it persist?

(In reply to jscher2000 from comment #58)

Simple reloads revalidate all the other files by re-requesting them with an If-Modified-Since header. It is only CSS files that are the exception now, requiring developers either to press Ctrl+F5 instead of F5 or to disable cache during development.

This is, afaict, not true. If-Modified-Since is used as well for CSS, when it has to be used. For example, python -m http.server doesn't set a max-age, and thus we always revalidate. See this gist for what I see when I try to reproduce what's described above.

If you're seeing something that worked before and not now on Nightly (which isn't bug 1660611 which is known), please, please file a bug with concrete steps to reproduce so that I can take a look. But let's move the discussion to that bug rather than here.

Filed a new bug here: https://bugzilla.mozilla.org/show_bug.cgi?id=1664929

Let me try to reproduce with your example.

Is this fixed, its happening in my blogging site where am not able to apply the dark background theme for the <pre> and <code> syntax tags. Tried clearing cache, disabled cache through .htaccess but no use and i have to stick with a light theme. Any solution? Adding a sample URL where the pre and code tags are there.

Sample URL - Eg: https://itsmycode.com/python-jsonpath/

(In reply to srinivasr2007 from comment #62)

Is this fixed, its happening in my blogging site where am not able to apply the dark background theme for the <pre> and <code> syntax tags. Tried clearing cache, disabled cache through .htaccess but no use and i have to stick with a light theme. Any solution? Adding a sample URL where the pre and code tags are there.

Sample URL - Eg: https://itsmycode.com/python-jsonpath/

if needed i can create a small project in github and show the demo. Let me know

Yes, please. File a new bug with steps to reproduce and I can take a look. A small repro would be ideal, yeah :)