1759738 - Don't send a new HTTP request when speculative CSS parse is invalidated by encoding

Status: NEW

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

Description

[Andrey Lushnikov]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.51 Safari/537.36

Steps to reproduce:

consider the following example:

1. install python and pip
2. install twisted: pip install twisted
3. pun the following script:

from twisted.web import server, resource, http
from twisted.internet import reactor

class Simple(resource.Resource):
    isLeaf = True
    def render_GET(self, request: http.Request):
        print(f"{request.method} {request.uri}")
        if request.uri == b"/":
            request.setHeader(b"content-type", b"text/html")
            return b"<link rel='stylesheet' href='./one-style.css'>"
        elif request.uri == b"/one-style.css":
            request.setHeader(b"content-type", b"text/css")
            return b"body { background: red; }"
        else:
            request.setResponseCode(404)
            return b"404"

site = server.Site(Simple())
reactor.listenTCP(8080, site)
reactor.run()

4. navigate to http://localhost:8080 in Firefox 97

Actual results:

Firefox sends requests to one-style.css twice

Expected results:

Firefox should send request once.

Downstream issue: https://github.com/microsoft/playwright/issues/12789

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Networking: HTTP' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Kershaw Chang [:kershaw]]

I can reproduce this locally.
Looks like both requests are created from nsHtml5TreeOpExecutor.
First one:

#01: mozilla::net::HttpChannelChild::AsyncOpen(nsIStreamListener*)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x8f85fc]
#02: mozilla::css::Loader::LoadSheet(mozilla::css::SheetLoadData&, mozilla::css::Loader::SheetState, mozilla::css::Loader::PendingLoad)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x4452e48]
#03: mozilla::css::Loader::InternalLoadNonDocumentSheet(nsIURI*, mozilla::css::StylePreloadKind, mozilla::css::SheetParsingMode, mozilla::css::Loader::UseSystemPrincipal, mozilla::Encoding const*, nsIReferrerInfo*, nsICSSLoaderObserver*, mozilla::CORSMode, nsT[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x445c004]
#04: mozilla::css::Loader::LoadSheet(nsIURI*, mozilla::css::StylePreloadKind, mozilla::Encoding const*, nsIReferrerInfo*, nsICSSLoaderObserver*, mozilla::CORSMode, nsTSubstring<char16_t> const&)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x445c548]
#05: mozilla::dom::Document::PreloadStyle(nsIURI*, mozilla::Encoding const*, nsTSubstring<char16_t> const&, mozilla::dom::ReferrerPolicy, nsTSubstring<char16_t> const&, mozilla::css::StylePreloadKind)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x162de88]
#06: nsHtml5TreeOpExecutor::PreloadStyle(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, bool)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0xf45558]
#07: nsHtml5TreeOpExecutor::FlushSpeculativeLoads()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0xf7d5a0]
#08: nsHtml5LoadFlusher::Run()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0xf834b8]
#09: mozilla::SchedulerGroup::Runnable::Run()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x32a648]
#10: mozilla::RunnableTask::Run()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x35bed8]

Second one:

#01: mozilla::net::HttpChannelChild::AsyncOpen(nsIStreamListener*)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x8f8600]
#02: mozilla::css::Loader::LoadSheet(mozilla::css::SheetLoadData&, mozilla::css::Loader::SheetState, mozilla::css::Loader::PendingLoad)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x4452e4c]
#03: mozilla::css::Loader::LoadStyleLink(mozilla::dom::LinkStyle::SheetInfo const&, nsICSSLoaderObserver*)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x445a514]
#04: mozilla::dom::LinkStyle::DoUpdateStyleSheet(mozilla::dom::Document*, mozilla::dom::ShadowRoot*, nsICSSLoaderObserver*, mozilla::dom::LinkStyle::ForceUpdate)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x16afbd8]
#05: nsHtml5DocumentBuilder::UpdateStyleSheet(nsIContent*)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0xf34788]
#06: nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*)[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0xf7e5d0]
#07: nsHtml5TreeOpExecutor::RunFlushLoop()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0xf7d924]
#08: nsHtml5ExecutorFlusher::Run()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0xf831e8]
#09: mozilla::SchedulerGroup::Runnable::Run()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x32a648]
#10: mozilla::RunnableTask::Run()[/Users/changkershaw/work/gecko/objdir/toolkit/library/build/XUL +0x35bed8]

Comment 3

[Edgar Chen [:edgar]]

17:21.96 INFO: Last good revision: 8203040487de0b0845bad7c75020a63e79a04e79
17:21.96 INFO: First bad revision: e135fe933c4e31482e91c77617a27c04b3a44eef
17:21.96 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8203040487de0b0845bad7c75020a63e79a04e79&tochange=e135fe933c4e31482e91c77617a27c04b3a44eef

It seems like a regression of bug 1701828. Henri, could you take a look? Thanks!

Comment 4

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

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

Comment 5

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2025-12-29)]

I haven't debugged this yet, but the likely reason is that we first speculatively inherit UTF-8 into the style sheet and then the document ends up being windows-1252, so we need to inherit another encoding into the style sheet and, when doing so, we don't load from cache.

Declaring the encoding of the HTML document should avoid this as a server-side fix.

Comment 6

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2025-12-29)]

(In reply to Henri Sivonen (:hsivonen) from comment #5)

Declaring the encoding of the HTML document should avoid this as a server-side fix.

This indeed works.

We need hold onto the bytes somewhere and not send a new HTTP request if the speculative CSS parse is invalidated by encoding at time of use differing from encoding at time of speculation.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Yeah, to be clear either using <meta charset=utf-8> or declaring the content encoding in a header should avoid this. This is because lacking other encoding declarations we assume utf-8 for the preload, but then if the document ends up having a different encoding we might need to load it again.