Implement basics of link preloading by TimvdLippe · Pull Request #37036 · servo/servo

TimvdLippe · 2025-05-17T09:06:10Z

These changes allow a minimal set of checks for font-src
CSP checks to pass.

Part of #4577
Part of #35035

TimvdLippe · 2025-05-17T09:07:01Z

I split this PR up in 2 commits, so that you can see which WPT tests would be fixed with the actual implementation. The reason I didn't submit 2 PR's is that as soon as you implement the basics, a lot of WPT tests start to FAIL instead of TIMEOUT. Therefore, we end up changing the expectations twice resulting in a lot of noise.

TimvdLippe · 2025-05-17T09:09:19Z

components/script/dom/htmllinkelement.rs

+            let this = Trusted::new(self);
+            self.owner_global()
+                .task_manager()
+                .performance_timeline_task_source()


I don't understand why this would be necessary and in fact, it's not even working yet. The test I use is tests/wpt/tests/preload/preload-referrer-policy.html. I can confirm that it preloads fine and it loads the script. Also, the performance timing entries are there if you add the following:

--- a/tests/wpt/tests/preload/preload-referrer-policy.html +++ b/tests/wpt/tests/preload/preload-referrer-policy.html @@ -1,7 +1,7 @@ <!DOCTYPE html> <meta charset=utf-8> <title>The preload's referrerpolicy attribute should be respected</title> -<meta name="timeout" content="long"> + <script src="resources/dummy.js?link-header-preload2"></script> <script src="/common/get-host-info.sub.js"></script> <script src="/common/utils.js"></script> @@ -52,6 +52,8 @@ const loaders = { const loaded = new Promise(resolve => script.addEventListener('load', resolve)); document.body.appendChild(script); await loaded; + console.log(performance.getEntries()); + console.log(script.src); return {unsafe: location.href, actualReferrer: window.referrers[id], entries: performance.getEntriesByName(script.src).length} }, };

However, the performance timings of run 1 are there, when executing run 2. That means that when the load event fires on the link, then the performance timing entries haven't been committed yet. Hence the performance.getEntriesByName(script.src).length results in 0. If you were to run that code in the second run with the src of the first run, it would show up data as expected.

I though the performance_timeline_task_source was the trick here to queue a task to ensure the rest has already been commited, but that's not enough.

TimvdLippe · 2025-05-17T09:09:54Z

components/shared/net/mime_classifier.rs

+
+use crate::LoadContext;

 pub struct MimeClassifier {


I moved this into net_traits since script does not depend on net. Since it is a relatively clean class with barely any dependencies, I suppose that's fine?

TimvdLippe · 2025-05-17T09:10:26Z

Let's see what WPT says: https://github.com/TimvdLippe/servo/actions/runs/15083738556

TimvdLippe · 2025-05-17T09:13:28Z

tests/wpt/meta/preload/preload-csp.sub.html.ini

@@ -0,0 +1,3 @@
+[preload-csp.sub.html]


It's strange to me that the font-src CSP tests pass, but this one doesn't. I double-checked if the request has all the correct information and added various println! statements in rust-content-security-policy, but wasn't able to figure out what we are still missing in create_link_request.

It's because we don't support track requests yet. All the other requests are correctly blocked.

components/script/dom/htmllinkelement.rs

TimvdLippe · 2025-05-18T10:52:01Z

https://github.com/TimvdLippe/servo/actions/runs/15095179190

This PR is now ready for review. Some of the WPT tests are not working, but I expect to work when we figure out the timing problem with performance entries.

servo-wpt-sync · 2025-05-18T10:53:58Z

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#52624) with upstreamable changes.

servo-wpt-sync · 2025-05-18T12:24:44Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

TimvdLippe · 2025-05-18T12:42:11Z

https://github.com/TimvdLippe/servo/actions/runs/15095884918

servo-wpt-sync · 2025-05-18T12:44:55Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

servo-wpt-sync · 2025-05-18T13:08:54Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

servo-wpt-sync · 2025-05-18T13:12:58Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

components/shared/net/tests/parsable_mime/text/xml/feed.rss

components/script/dom/htmllinkelement.rs

TimvdLippe · 2025-05-20T18:34:25Z

https://github.com/TimvdLippe/servo/actions/runs/15145203412

servo-wpt-sync · 2025-05-20T18:34:34Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

servo-wpt-sync · 2025-05-20T18:40:17Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

TimvdLippe · 2025-05-20T19:01:13Z

https://github.com/TimvdLippe/servo/actions/runs/15145681714

servo-wpt-sync · 2025-05-20T19:03:30Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

TimvdLippe · 2025-05-27T12:16:22Z

Reverted back the optional destination change, as that appears to break a lot of WPT tests. Confirming we are back green now: https://github.com/TimvdLippe/servo/actions/runs/15274995022

servo-wpt-sync · 2025-05-27T12:18:55Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

TimvdLippe · 2025-05-27T14:05:29Z

/preload/preload-error.sub.html appears to be flaky. Every WPT run it reports something different and I keep on updating the expectations, only for it to flake on the next run. Do we have another cache which does not take into account CSP values?

TimvdLippe · 2025-05-27T14:09:41Z

Ah, according to https://disabled-wpt.goose.icu/#filter=preload%2Fpreload-error.sub.html it is indeed flaky :/

Part of servo#35035 Signed-off-by: Tim van der Lippe <[email protected]>

That way, we can depend on it from the script package. Also update several of the mime matching methods to match the specification and add more relevant media types to it. Signed-off-by: Tim van der Lippe <[email protected]>

These changes allow a minimal set of checks for font-src CSP checks to pass. Part of servo#4577 Part of servo#35035 Signed-off-by: Tim van der Lippe <[email protected]>

servo-wpt-sync · 2025-05-27T14:18:48Z

📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#52624).

TimvdLippe · 2025-05-28T20:28:34Z

That WPT failure is a flake per my above comment. It also flakes in other browsers. Can we mark it as intermittent and file an issue with WPT to get it addressed?

simonwuelker · 2025-05-29T07:37:13Z

That WPT failure is a flake per my above comment. I

Sorry for missing this. I filed #37177.

simonwuelker · 2025-05-29T10:09:38Z

@jdm is there a way to mark a test as having both intermittent PASSes and FAILs or do I need to open a second issue?

mrobinson · 2025-05-29T10:18:40Z

@jdm is there a way to mark a test as having both intermittent PASSes and FAILs or do I need to open a second issue?

You can add expected: [PASS, FAIL] to the .ini metadata for the test, which should allow it to accept both results.

jdm · 2025-05-29T11:27:49Z

When an issue is not being recognized by our systems and merges are failing, usually reopening the issue or readding the label will help.

* main: (510 commits) DevTools: Fix empty `debugger > source` panel (servo#37197) dom: implement signal abort on controller and signal (servo#37192) build(deps): bump parking_lot from 0.12.3 to 0.12.4 (servo#37199) layout: Split overflow calculation after fragment tree construction (servo#37203) build(deps): bump parking_lot_core from 0.9.10 to 0.9.11 (servo#37202) build(deps): bump lock_api from 0.4.12 to 0.4.13 (servo#37201) build(deps): bump cc from 1.2.24 to 1.2.25 (servo#37198) Constellation can now optionally report memory usage when the page is loaded. (servo#37151) Implement Input `type=text` UA Shadow DOM (servo#37065) constellation: Wait for canvas thread to shut down before shutting down system font service (servo#37182) Add slot default display style test (servo#37189) Send synthetic keydown/keyup at ime_insert_text (servo#37175) script: Let canvas serialization to image fail gracefully (servo#37184) Implement basics of link preloading (servo#37036) compositor: Add an initial RefreshDriver (servo#37169) pixels: Add limitation to max image total bytes length (servo#37172) Chore: Remove unused variable in `transition-zero-duration-with-delay.html` (servo#37179) build(deps): bump ohos-ime from 0.2.0 to 0.3.0 (servo#37180) Add a user agent style for the `<slot>` element (servo#37174) build(deps): bump hitrace from 0.1.4 to 0.1.5 (servo#37170) ...

TimvdLippe commented May 17, 2025

View reviewed changes

jdm reviewed May 17, 2025

View reviewed changes

components/script/dom/htmllinkelement.rs Show resolved Hide resolved

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from 8ac839e to 6e8c000 Compare May 18, 2025 10:51

TimvdLippe marked this pull request as ready for review May 18, 2025 10:52

TimvdLippe requested a review from gterzian as a code owner May 18, 2025 10:52

servo-wpt-sync mentioned this pull request May 18, 2025

Implement basics of link preloading web-platform-tests/wpt#52624

Merged

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from 6e8c000 to 01716fa Compare May 18, 2025 12:22

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from 01716fa to bc9aa09 Compare May 18, 2025 12:42

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from bc9aa09 to f33d2d9 Compare May 18, 2025 13:06

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from f33d2d9 to c4c5f67 Compare May 18, 2025 13:10

simonwuelker requested changes May 18, 2025

View reviewed changes

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from c4c5f67 to 684de99 Compare May 20, 2025 18:31

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from 684de99 to c8905ec Compare May 20, 2025 18:37

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from c8905ec to b603fb3 Compare May 20, 2025 19:00

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from b603fb3 to a626119 Compare May 27, 2025 12:15

TimvdLippe added 3 commits May 27, 2025 16:15

Add baseline for preload WPT tests

fbc961d

Part of servo#35035 Signed-off-by: Tim van der Lippe <[email protected]>

Move mime classifier to net_traits

667d26b

That way, we can depend on it from the script package. Also update several of the mime matching methods to match the specification and add more relevant media types to it. Signed-off-by: Tim van der Lippe <[email protected]>

Implement basics of link preloading

dfa5501

These changes allow a minimal set of checks for font-src CSP checks to pass. Part of servo#4577 Part of servo#35035 Signed-off-by: Tim van der Lippe <[email protected]>

TimvdLippe force-pushed the pr-timvdlippe-start-implementation-preload branch from a626119 to dfa5501 Compare May 27, 2025 14:15

TimvdLippe requested a review from simonwuelker May 27, 2025 14:16

simonwuelker approved these changes May 28, 2025

View reviewed changes

simonwuelker added this pull request to the merge queue May 28, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 28, 2025

jdm added this pull request to the merge queue May 28, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 28, 2025

simonwuelker added this pull request to the merge queue May 29, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 29, 2025

jdm added this pull request to the merge queue May 29, 2025

Merged via the queue into servo:main with commit 36e4886 May 29, 2025
21 checks passed

TimvdLippe deleted the pr-timvdlippe-start-implementation-preload branch May 29, 2025 13:36

Uh oh!

Conversation

TimvdLippe commented May 17, 2025

Uh oh!

TimvdLippe commented May 17, 2025

Uh oh!

TimvdLippe May 17, 2025

Choose a reason for hiding this comment

Uh oh!

TimvdLippe May 17, 2025

Choose a reason for hiding this comment

Uh oh!

TimvdLippe commented May 17, 2025

Uh oh!

TimvdLippe May 17, 2025

Choose a reason for hiding this comment

Uh oh!

TimvdLippe May 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TimvdLippe commented May 18, 2025

Uh oh!

servo-wpt-sync commented May 18, 2025

Uh oh!

servo-wpt-sync commented May 18, 2025

Uh oh!

TimvdLippe commented May 18, 2025

Uh oh!

servo-wpt-sync commented May 18, 2025

Uh oh!

servo-wpt-sync commented May 18, 2025

Uh oh!

servo-wpt-sync commented May 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimvdLippe commented May 20, 2025

Uh oh!

servo-wpt-sync commented May 20, 2025

Uh oh!

servo-wpt-sync commented May 20, 2025

Uh oh!

TimvdLippe commented May 20, 2025

Uh oh!

servo-wpt-sync commented May 20, 2025

Uh oh!

TimvdLippe commented May 27, 2025

Uh oh!

servo-wpt-sync commented May 27, 2025

Uh oh!

TimvdLippe commented May 27, 2025

Uh oh!

TimvdLippe commented May 27, 2025

Uh oh!

servo-wpt-sync commented May 27, 2025

Uh oh!

Uh oh!

Uh oh!

TimvdLippe commented May 28, 2025

Uh oh!

simonwuelker commented May 29, 2025

Uh oh!

Uh oh!

simonwuelker commented May 29, 2025

Uh oh!

mrobinson commented May 29, 2025

Uh oh!

jdm commented May 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects