Add performance.speculations.preloads for speculation measurement [chromium/src : main]
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kouhei Ueno (Gerrit)
Kouhei Ueno added 2 comments![]( "Open in Gerrit")
[Exposed=Window, RuntimeEnabled=SpeculationMeasurement] readonly attribute SpeculationData speculations;Since speculations returns a new object on every access, it doesn't maintain object identity, which is typically expected for attributes. According to the W3C Web Platform Design Principles:
"Returning a new value from a getter each time is not allowed. If this does not hold, the getter should be a method."
To align with this and existing Performance API patterns like getEntries(), we should consider making this a method (e.g., getSpeculations()) instead of a getter.
V8CrossOriginMode PreloadData::crossorigin() const {This reverse-engineering works for the current mapping, but it might be a bit brittle if network modes or credentials mappings evolve in the future.
It might be more robust to store the original attribute value (or a mapped enum) in PreloadInfo directly when the preload is created in ResourceFetcher.
Related details
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 2 comments![]( "Open in Gerrit")
[Exposed=Window, RuntimeEnabled=SpeculationMeasurement] readonly attribute SpeculationData speculations;Since speculations returns a new object on every access, it doesn't maintain object identity, which is typically expected for attributes. According to the W3C Web Platform Design Principles:
"Returning a new value from a getter each time is not allowed. If this does not hold, the getter should be a method."
To align with this and existing Performance API patterns like getEntries(), we should consider making this a method (e.g., getSpeculations()) instead of a getter.
Done
This reverse-engineering works for the current mapping, but it might be a bit brittle if network modes or credentials mappings evolve in the future.
It might be more robust to store the original attribute value (or a mapped enum) in PreloadInfo directly when the preload is created in ResourceFetcher.
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kouhei Ueno (Gerrit)
Kouhei Ueno added 5 comments![]( "Open in Gerrit")
Please make sure the measurement/recording code is also flag guarded - only enabled when the document has SpeculationMeasurement enabled
used = MonotonicTimeToDOMHighResTimeStamp(info.used_time.value());Do we want to use the default TimeOrigin() here?
preloads.push_back(MakeGarbageCollected<PreloadData>(The bindings logics (conversion of kCrossOriginAttributeFoo to V8CrossOriginMode::Enum, ResourceTypeToAsAttributeString, MonotonicTimeToDOMHighResTimeStamp) should be handled in PreloadData binding getters.
kCrossOriginAttributeNotSet;This shouldn't be needed? You can look inside `resource_request_`
String ResourceTypeToAsAttributeString(ResourceType type) {I find this to be unfortunate that we need to have this + https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/resource.cc;l=1249;bpv=1;bpt=1?q=return%20%22manifest%22;&ss=chromium
Would it be too difficult to merge?
Related details
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 4 comments![]( "Open in Gerrit")
used = MonotonicTimeToDOMHighResTimeStamp(info.used_time.value());Do we want to use the default TimeOrigin() here?
I believe so.
The bindings logics (conversion of kCrossOriginAttributeFoo to V8CrossOriginMode::Enum, ResourceTypeToAsAttributeString, MonotonicTimeToDOMHighResTimeStamp) should be handled in PreloadData binding getters.
Done
kCrossOriginAttributeNotSet;This shouldn't be needed? You can look inside `resource_request_`
AFAICT resource_request_ has the mode and credentials mode, but not the attribute value. I can decipher the value from those, but didn't we say in a previous iteration that we should avoid doing that?
String ResourceTypeToAsAttributeString(ResourceType type) {I find this to be unfortunate that we need to have this + https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/resource.cc;l=1249;bpv=1;bpt=1?q=return%20%22manifest%22;&ss=chromium
Would it be too difficult to merge?
They seem to be returning different values. One is returning the `as` attribute value, while the other is returning a string that describes the resource type (e.g. "style" vs "CSS Stylesheet")
Related details
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kouhei Ueno (Gerrit)
Kouhei Ueno added 6 comments![]( "Open in Gerrit")
Please make sure the measurement/recording code is also flag guarded - only enabled when the document has SpeculationMeasurement enabled
Please address this.
return performance_->MonotonicTimeToDOMHighResTimeStamp(used_time_.value());Can we omit `performance_` by annotating the idl with [CallWith=ScriptState], and we can access WindowPerformance from ScriptState*.
kCrossOriginAttributeNotSet;Yoav Weiss (@Shopify)This shouldn't be needed? You can look inside `resource_request_`
AFAICT resource_request_ has the mode and credentials mode, but not the attribute value. I can decipher the value from those, but didn't we say in a previous iteration that we should avoid doing that?
Hmm ok. I searched for better alternatives but I couldn't come up with something better. Would you add a comment this is specifically for the speculation measurement API?
String ResourceTypeToAsAttributeString(ResourceType type) {Yoav Weiss (@Shopify)I find this to be unfortunate that we need to have this + https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/resource.cc;l=1249;bpv=1;bpt=1?q=return%20%22manifest%22;&ss=chromium
Would it be too difficult to merge?
They seem to be returning different values. One is returning the `as` attribute value, while the other is returning a string that describes the resource type (e.g. "style" vs "CSS Stylesheet")
Acknowledged
struct PreloadInfo {(Copying over the comments from the chat)
Taking a step back, between PreloadInfo, PreloadInfoWithUrl, and PreloadData, there seems to be a fair amount of duplication and double-copying of data. I suspect we can remove GetPreloadRecords(), and potentially eliminate the other intermediate structs entirely if we store PreloadData directly in ResourceFetcher (leveraging [CallWith=ScriptState] for time conversion).
Related details
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 4 comments![]( "Open in Gerrit")
Kouhei UenoPlease make sure the measurement/recording code is also flag guarded - only enabled when the document has SpeculationMeasurement enabled
Please address this.
Done
return performance_->MonotonicTimeToDOMHighResTimeStamp(used_time_.value());Can we omit `performance_` by annotating the idl with [CallWith=ScriptState], and we can access WindowPerformance from ScriptState*.
Done
kCrossOriginAttributeNotSet;Yoav Weiss (@Shopify)This shouldn't be needed? You can look inside `resource_request_`
Kouhei UenoAFAICT resource_request_ has the mode and credentials mode, but not the attribute value. I can decipher the value from those, but didn't we say in a previous iteration that we should avoid doing that?
Hmm ok. I searched for better alternatives but I couldn't come up with something better. Would you add a comment this is specifically for the speculation measurement API?
Done
struct PreloadInfo {(Copying over the comments from the chat)
Taking a step back, between PreloadInfo, PreloadInfoWithUrl, and PreloadData, there seems to be a fair amount of duplication and double-copying of data. I suspect we can remove GetPreloadRecords(), and potentially eliminate the other intermediate structs entirely if we store PreloadData directly in ResourceFetcher (leveraging [CallWith=ScriptState] for time conversion).
PreloadInfoWithUrl is gone.
PreloadInfo and PreloadData are both needed IMO (the former to store preload info in ResourceFetcher in platform/ and the latter as a ScriptWrappable web-exposed entries in core/)
Related details
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa added 7 comments![]( "Open in Gerrit")
// https://github.com/WICG/speculative_load_measurementThis link should be placed right before `class PreloadData`, not `class ScriptState`.
kCrossOriginAttributeNotSet;Can we make this `std::optional<CrossOriginAttributeValue>` to avoid it from being used for security checks in unrelated contexts?
String ResourceTypeToAsAttributeString(ResourceType type) {Can we place this function next to `PreloadHelper::GetResourceTypeFromAsAttribute()` to keep this consistent with that?
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/loader/preload_helper.cc;l=394;drc=d4203fb18d457eb93924af6bb2d24c954402978e
Also, can we add a link to a corresponding spec part that defines the string representation of resource types? I guess it's `preload destination` and `module preload destination` linked from:
https://html.spec.whatwg.org/multipage/semantics.html#attr-link-as
// Only track <link rel=preload> in preload_records_ for the\`preload_records_\` (adding back quotes)
if (!href) href = supportFileUrl(as);Can we wrap this braces? It's more consistent.
```
if (!href) {
href = supportFileUrl(as);
}
```
if (crossorigin !== null) el.crossOrigin = crossorigin;Ditto(braces)
if (crossorigin !== null) el.crossOrigin = crossorigin;Ditto(braces)
Related details
- Kouhei Ueno
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 6 comments![]( "Open in Gerrit")
Can we make this `std::optional<CrossOriginAttributeValue>` to avoid it from being used for security checks in unrelated contexts?
Done
String ResourceTypeToAsAttributeString(ResourceType type) {Can we place this function next to `PreloadHelper::GetResourceTypeFromAsAttribute()` to keep this consistent with that?
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/loader/preload_helper.cc;l=394;drc=d4203fb18d457eb93924af6bb2d24c954402978eAlso, can we add a link to a corresponding spec part that defines the string representation of resource types? I guess it's `preload destination` and `module preload destination` linked from:
https://html.spec.whatwg.org/multipage/semantics.html#attr-link-as
Done
// Only track <link rel=preload> in preload_records_ for theYoav Weiss (@Shopify)\`preload_records_\` (adding back quotes)
Done
Can we wrap this braces? It's more consistent.
```
if (!href) {
href = supportFileUrl(as);
}
```
Done
if (crossorigin !== null) el.crossOrigin = crossorigin;Yoav Weiss (@Shopify)Ditto(braces)
Done
if (crossorigin !== null) el.crossOrigin = crossorigin;Yoav Weiss (@Shopify)Ditto(braces)
Done
Related details
- Hiroki Nakagawa
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 1 comment![]( "Open in Gerrit")
// https://github.com/WICG/speculative_load_measurementThis link should be placed right before `class PreloadData`, not `class ScriptState`.
Done
Related details
- Hiroki Nakagawa
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa voted and added 2 comments![]( "Open in Gerrit")
Votes added by Hiroki Nakagawa
| Code-Review | +1 |
2 comments
// This is the inverse of GetResourceTypeFromAsAttribute().This can return types that are not supported in `GetResourceTypeFromAsAttribute()`. Can we avoid returning them and instead add NOTREACHED()?
Related details
- Kouhei Ueno
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa added 1 comment![]( "Open in Gerrit")
LTGM, thanks!
LGTM 😜
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 2 comments![]( "Open in Gerrit")
// This is the inverse of GetResourceTypeFromAsAttribute().This can return types that are not supported in `GetResourceTypeFromAsAttribute()`. Can we avoid returning them and instead add NOTREACHED()?
Done
Yoav Weiss (@Shopify)(Copying over the comments from the chat)
Taking a step back, between PreloadInfo, PreloadInfoWithUrl, and PreloadData, there seems to be a fair amount of duplication and double-copying of data. I suspect we can remove GetPreloadRecords(), and potentially eliminate the other intermediate structs entirely if we store PreloadData directly in ResourceFetcher (leveraging [CallWith=ScriptState] for time conversion).
PreloadInfoWithUrl is gone.
PreloadInfo and PreloadData are both needed IMO (the former to store preload info in ResourceFetcher in platform/ and the latter as a ScriptWrappable web-exposed entries in core/)
Resolving
Related details
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/59321.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Related details
- Kouhei Ueno
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
14 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/core/loader/preload_helper.h
Insertions: 2, Deletions: 1.
@@ -105,7 +105,8 @@
const String& as);
// Returns the "as" attribute value string for a given ResourceType.
- // This is the inverse of GetResourceTypeFromAsAttribute().
+ // Only supports types returned by GetResourceTypeFromAsAttribute();
+ // crashes with NOTREACHED() for unsupported types.
// https://html.spec.whatwg.org/C/#preload-destination
static String GetAsAttributeFromResourceType(ResourceType type);
};
```
```
The name of the file: third_party/blink/renderer/core/loader/preload_helper.cc
Insertions: 9, Deletions: 14.
@@ -417,28 +417,23 @@
case ResourceType::kScript:
return "script";
case ResourceType::kCSSStyleSheet:
- case ResourceType::kXSLStyleSheet:
return "style";
case ResourceType::kTextTrack:
return "track";
case ResourceType::kFont:
return "font";
- case ResourceType::kAudio:
- return "audio";
- case ResourceType::kVideo:
- return "video";
- case ResourceType::kManifest:
- return "manifest";
- case ResourceType::kSpeculationRules:
- return "speculationrules";
- case ResourceType::kDictionary:
- return "dictionary";
- case ResourceType::kSVGDocument:
- return "document";
case ResourceType::kRaw:
+ return "fetch";
+ case ResourceType::kAudio:
+ case ResourceType::kVideo:
+ case ResourceType::kManifest:
+ case ResourceType::kSpeculationRules:
+ case ResourceType::kDictionary:
+ case ResourceType::kSVGDocument:
+ case ResourceType::kXSLStyleSheet:
case ResourceType::kLinkPrefetch:
case ResourceType::kMock:
- return "fetch";
+ NOTREACHED();
}
}
```
Change information
Add performance.speculations.preloads for speculation measurement
Expose preload measurement data on the Performance object.
This provides a live API that can be queried at any time, not just
during page unload.
The "used" attribute contains a coarsened timestamp indicating when the
resource was used.
The "crossorigin" and "as" attributes contain the relevant attributes
of the fetched resource.
Co-Authored-By: AI (Claude Opus 4.6 via Pi) <nor...@pi.dev>
- M android_webview/test/data/web_tests/webexposed/global-interface-listing-expected.txt
- M third_party/blink/renderer/bindings/generated_in_core.gni
- M third_party/blink/renderer/bindings/idl_in_core.gni
- M third_party/blink/renderer/core/loader/preload_helper.cc
- M third_party/blink/renderer/core/loader/preload_helper.h
- M third_party/blink/renderer/core/timing/build.gni
- M third_party/blink/renderer/core/timing/performance.cc
- M third_party/blink/renderer/core/timing/performance.h
- M third_party/blink/renderer/core/timing/performance.idl
- A third_party/blink/renderer/core/timing/preload_data.cc
- A third_party/blink/renderer/core/timing/preload_data.h
- A third_party/blink/renderer/core/timing/preload_data.idl
- A third_party/blink/renderer/core/timing/speculation_data.cc
- A third_party/blink/renderer/core/timing/speculation_data.h
- A third_party/blink/renderer/core/timing/speculation_data.idl
- M third_party/blink/renderer/core/timing/window_performance.cc
- M third_party/blink/renderer/core/timing/window_performance.h
- M third_party/blink/renderer/platform/loader/fetch/fetch_parameters.cc
- M third_party/blink/renderer/platform/loader/fetch/fetch_parameters.h
- M third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
- M third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
- A third_party/blink/web_tests/external/wpt/speculation-rules/speculation-measurement/performance-speculations-unused-preload.tentative.html
- A third_party/blink/web_tests/external/wpt/speculation-rules/speculation-measurement/performance-speculations-used-preload.tentative.html
- A third_party/blink/web_tests/external/wpt/speculation-rules/speculation-measurement/support/preloaded-data.txt
- A third_party/blink/web_tests/external/wpt/speculation-rules/speculation-measurement/support/preloaded-script.js
- A third_party/blink/web_tests/external/wpt/speculation-rules/speculation-measurement/support/preloaded-style.css
- A third_party/blink/web_tests/external/wpt/speculation-rules/speculation-measurement/support/speculation-measurement-utils.js
- M third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt
Code-Review: +1 by Hiroki Nakagawa
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/59321
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |