STR:

1. Visit https://webxr.today
2. Reload the page

Expected
Page is reloaded

Actual:
Page is broken

Reloading the page worked in GV Nightly 79.0.20200615092624 and is broken in 79.0.20200616092900

This is a ship blocker for FxR.

Randall, does it reproduce in Desktop by chance?

Works -> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3ce9af6580b2748bd34837cb3d1aa074c6f76436&tochange=419a897d7e4b9489ad51b51044f154e5bcbb605e
Fails -> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ebf050bcdc162b30b45087f11a69c2ab6f8b090c&tochange=419a897d7e4b9489ad51b51044f154e5bcbb605e

Got bogus information from mozregression GUI.

ni? to take a look at the regression

Not very plausible that this was regressed by my patch, as that pertains to wasm SIMD only, but I'm investigating.

There's both script and CSS involved, though off-hand it seems a bit more likely to be a regression from my changes. Randal, can you double-check the regression range?

There's a:

<link rel="preload" href="/css/index.css" as="style" onload="this.onload=null;this.rel='stylesheet'">

whose load event is not firing. That smells a lot like something my change could've plausibly caused :)

As far as I can tell, the page remains broken with both wasm SIMD disabled and the optimizing compiler disabled that was affected by that patch, and has no wasm content. So this looks increasingly unlikely to me to be the problem.

Moderately sure this is bug 1599160 then. I'll take a look.

I can run it again. I find the mozregression GUI UI to be confusing.

I think this exposes an existing issue with our preload implementation.

Switching to cli for mozregression gave the expected regressing: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3ce9af6580b2748bd34837cb3d1aa074c6f76436&tochange=419a897d7e4b9489ad51b51044f154e5bcbb605e

Sorry for tagging the wrong commit.

So, the issue is that if we don't ever create a PreloaderBase for a given preload we'll fire an error event for that node... Though when we hit the stylesheet cache we don't create an StreamLoader.

Honza, we could do multiple things to fix this I think...

• Maybe we don't even need the PreloaderBase machinery for stylesheets, and we can just use the regular css::Loader machinery, which already can take care of firing load / error events for <link rel=stylesheet>... Though that changes the event we fire in the case of bogus @imports, to match what <link rel=stylesheet> does... Perhaps that's not bad?

• Otherwise, I think the thing that should inherit from PreloaderBase might want to be SheetLoadData, perhaps?

(Or even StyleSheet, but not a fan of bloating them even more)

This is just cleanup while I was going through this code.

This makes it easy to see why your test is not failing without your
patch, for example ;)

Note that we log only when the URIs differ, which I think is a
reasonable compromise in verbosity.

Depends on D80287

When we hit the stylesheet cache for an existing stylesheet in a
different document:

• There's no existing preload in the PreloadService (because the sheet
  was loaded in another document).

• We don't create a PreloaderBase (because we don't trigger a load at
  all).

• And thus we believe the load has just errored, which is wrong...

Fix it by properly passing through the "already complete" status from
PreloadStyle.

Note that there's still another issue, which is that we'd still hit this
broken path if two stylesheets with the same URI are loading in
different documents and the CSS loader coalesces them.

For that, I think we'd have to make SheetLoadData the thing that
actually implements PreloaderBase. This is kind of an obscure case, and
SheetLoadData has a pretty complex lifetime already (keeps alive a whole
lot of stuff), so I'd prefer to do this in a separate bug.

Depends on D80288

Ok, so... Sent a necessary fix that fixes this. I'll spin off the PreloaderBase change, if Honza agrees, to another bug.

This shouldn't have any behavior change, but it makes the code make a
bit more sense. Rather than counting inline stylesheets like a pending
load, we won't (but note that any @import inside it will).

The SheetLoadData::mURL it's supposed to be the url of the stylesheet,
so for StyleSheet::Replace it should be null.

Depends on D80289

So as to make sense in a world where we can coalesce loads across
documents. The per-load object is the SheetLoadData, so this way we
guarantee that we fire the right events in presence of the load
coalescing that the SharedStyleSheetCache does.

Depends on D80379

https://hg.mozilla.org/mozilla-central/rev/6bcac6d68384
https://hg.mozilla.org/mozilla-central/rev/0be160e4f5ae

https://hg.mozilla.org/mozilla-central/rev/5855c8067f8d

https://hg.mozilla.org/mozilla-central/rev/e847c2e2c84e
https://hg.mozilla.org/mozilla-central/rev/be056dc9e0f1