1648736 - Webpages render previously cached CSS changes

Status: VERIFIED FIXED

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

Description

[Alex Gibson [:agibson]]

Attachment: image.png

Nightly 79.0a1 (2020-06-25) (64-bit)
macOS 10.15.5

When making changes to web pages on localhost, Nightly is rendering the previously cached version of CSS files, despite changes being loaded and reflected in DevTools.

The project I'm working on is https://github.com/mozilla/bedrock, but this is likely reproducable elsewhere.

STR:

1. Load a webpage that references a CSS file in a Nightly tab.
2. Make a small change to the CSS file, for example change the color of a background or some text.
3. Refresh the browser tab.

Expected results:

Nightly should render the CSS change.

Actual results:

Nightly still renders the previously cached CSS, despite the browser (and DevTools) having loaded and parsed the changed properties.

Notice in the screenshot attached, DevTools correctly identifies the color of the heading as hotpink, but Nightly still renders the previous color (black). A second refresh fixes things and picks up the change.

I can't reproduce this issue in release or dev edition.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

So this is technically a regression from bug 1599160 (or follow-ups), unsurprisingly... But that means that your development server is sending us headers that allow us to cache stuff. It seems like devtools re-fetches everything again on their own, which is why you see it disagreeing with the page.

Can you give more information about your environment? What server are you using for development? Can you post an screenshot of the "Response" tab in the network panel for the stylesheet?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Ok, I can repro with a very simple python -m http.server and so on. I'll dig.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1648736 - Don't mark a load as performed on a given document until it has actually finished. r=#style

Consider the case where we have an expired entry in the cache, and we
load a new document.

We get an speculative load from the HTML parser. That's great, and we
see the entry is expired and actually fired the load.

But then, we actually get to the load that the <link> element performs,
and we see that we've already performed this load, so instead of peeking
the in-progress load, we go ahead and peek the expired "complete" cache
entry, which is not what we want.

By marking a load as performed only once it has finished, we avoid the
complete sheet cache, and gloom onto the existing load instead, which is
the correct thing to do.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1599160

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b6e6e9ef6a5
Don't mark a load as performed on a given document until it has actually finished. r=heycam

Comment 6

[Razvan Maries]

Backed out for perma failures.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=WO_Wjsz7Qd-KUjBtQf6hYg.0&resultStatus=testfailed%2Cbusted%2Cexception&revision=a1a0390cd1a1c868fe908387824532115cbf223c&searchStr=reftest&failure_classification_id=2

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

Backout: https://hg.mozilla.org/integration/autoland/rev/54cec9fc8d4ea7e0964806d925961147dfa71410

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/061e049e9546
Don't mark a load as performed on a given document until it has actually finished. r=heycam

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/061e049e9546

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9159671 [details]
Bug 1648736 - Don't mark a load as performed on a given document until it has actually finished. r=#style

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Create two files:

t.html

<!doctype html>
<link rel="stylesheet" href="t.css">

t.css

:root { background: red }

In that same directory run python -m http.server. Go to http://0.0.0.0:8000/t.html, and ensure you see a red page.

Change the contents of t.css to be :root { background: green }. Refresh the previous page.

You should see green instead of red.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple one-liner to avoid hitting the wrong cache.
• String changes made/needed: none

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9159671 [details]
Bug 1648736 - Don't mark a load as performed on a given document until it has actually finished. r=#style

Approved for 79.0b2.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/004daccbbdcf

Comment 12

[Cristian Baica [:cbaica], Release Desktop QA]

I have managed to reproduce the issue using Nightly 79.0a1 (2020-06-25).
The issue is verified using Fx80.0a1 and Fx79.0b2 on Windows 10 and Ubuntu 18.04 based on the steps provided in comment 9. After the refresh, the color is correctly changed to green (updated color of the .css file).

Comment 13

[Steven Singer]

I think this change is breaking sites. See bug 1673885.