Consider to speculatively load css imports (@import) from inline styles
Categories
(Core :: DOM: HTML Parser, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox71 | --- | fixed |
People
(Reporter: smaug, Assigned: emilio)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
.
|
||
Comment 1•3 years ago
|
||
To be precise, the situation here is:
<script src="really-slow-loading-script.js"></script>
<style>
@import url("some-style-sheet");
</style>
Right now we block the parser on the script, and don't end up starting the import load until the script finishes loading and we parse the <style>.
In an ideal world, the HTML prescan we keep doing (which would kick off a <link rel="stylesheet" url="some-style-sheet"> load) would deal with this case and start the load for "some-style-sheet". The HTML parser can obviously grab the string inside the <style> element, but then we need to actually find the @imports in it.
|
Assignee | |
Comment 2•3 years ago
|
||
While we're doing this, we should also consider doing the same for @font-face (maybe, at least, since I guess the font could not get used).
I know WebKit (and I guess Blink) does preload imports, though their analyzer was a bit crappy and failed to recognize @imports without quotes: https://bugs.webkit.org/show_bug.cgi?id=191466
|
|
||
Comment 3•3 years ago
|
||
maybe, at least, since I guess the font could not get used
Right, that is the issue with fonts...
|
|
Assignee | |
Comment 4•3 years ago
|
||
It shouldn't be too terrible to add a CSS parser mode that stops after import rules, and triggers loads for them... I don't think it'd have many side effects, other than maybe allocating Servo NamespaceRule / ImportRule objects and such. We already parse a bunch off-main-thread.
|
|
Assignee | |
Comment 5•3 years ago
|
||
Or alternatively we could go with the crappy "tokenizer" approach that WebKit / Blink have:
If we're just looking for @import that may be even simpler... Not sure.
|
|
Assignee | |
Comment 6•3 years ago
|
||
Henri, do you know where could I get the stylesheet text content from the parser code? Also, would they be utf8? Or utf16?
|
||
Comment 7•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)
Henri, do you know where could I get the stylesheet text content from the parser code?
First, in https://searchfox.org/mozilla-central/source/parser/html/nsHtml5TreeBuilderHSupplement.h add a style nesting counter (below the comment with exclamation marks about mBuilder). (Counter instead of a boolean thanks to SVG being able to nest elements within style.)
In the below methods, changes should only go on the off-the-main-thread code path, which is the case where mBuilder is null.
In https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/parser/html/nsHtml5TreeBuilderCppSupplement.h#829 if the element is style in either the HTML or SVG namespace, increment the counter (in the existing blocks that now handle line numbers for HTML and SVG style). In https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/parser/html/nsHtml5TreeBuilderCppSupplement.h#900 decrement the counter.
In https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/parser/html/nsHtml5TreeBuilderCppSupplement.h#651 if the counter is nonzero, process the text. Note that a single text node could be split across multiple calls to this method.
For an example of how to ship the URLs to the main thread, see
https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/parser/html/nsHtml5TreeBuilderCppSupplement.h#236
Also, would they be utf8? Or utf16?
UTF-16, unfortunately.
|
|
||
Updated•3 years ago
|
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Comment 8•3 years ago
|
||
Doesn't recognize all the edge cases, but I think this should be good enough.
Let me know if you think something common is missing.
|
|
Assignee | |
Comment 9•3 years ago
|
||
That doesn't make much sense to me.
|
|
Assignee | |
Comment 10•3 years ago
|
||
We don't bother handling the nested element case amazingly. We'd instead stop
at the inner <style> element and drop the URLs from the outer.
But I think that's ok. Any good way to test this? I've verified it does the
right thing looking at the CSS loader logs, but... :)
|
|
Assignee | |
Comment 11•3 years ago
|
||
|
||
Comment 12•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b2678d2e55fb Add a best-effort @import rule scanner for the parser. r=bzbarsky https://hg.mozilla.org/integration/autoland/rev/a8ac28c2bfc7 Stop speculatively trying to load <svg:style xlink:href>. r=bzbarsky https://hg.mozilla.org/integration/autoland/rev/ad3822105edc Speculatively load @import rules inside <style>. r=bzbarsky,hsivonen
|
||
Comment 13•3 years ago
|
||
Backed out 3 changesets (bug 1546783) on request by emilio
Backout: https://hg.mozilla.org/integration/autoland/rev/5f8f1210274a23f602560e209cd147a09f0cb9d8
|
|
Assignee | |
Updated•3 years ago
|
|
|
||
Comment 14•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/78d0e221f6a4 Add a best-effort @import rule scanner for the parser. r=bzbarsky https://hg.mozilla.org/integration/autoland/rev/dfccd3155829 Stop speculatively trying to load <svg:style xlink:href>. r=bzbarsky https://hg.mozilla.org/integration/autoland/rev/56599f0a87a4 Speculatively load @import rules inside <style>. r=bzbarsky,hsivonen
|
|
||
Comment 15•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ce851932daee Test that we preload @import stylesheets. r=bzbarsky
|
||
Comment 16•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/78d0e221f6a4
https://hg.mozilla.org/mozilla-central/rev/dfccd3155829
https://hg.mozilla.org/mozilla-central/rev/56599f0a87a4
https://hg.mozilla.org/mozilla-central/rev/ce851932daee
Description
•