Bug 199162 - Cross-origin ongoing prefetches should be reused for top-level navigations
Summary: Cross-origin ongoing prefetches should be reused for top-level navigations
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords:
Depends on: 194539
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-24 08:43 PDT by Rob Buis
Modified: 2023-01-25 05:02 PST (History)
13 users (show)

See Also:


Attachments
Patch (12.95 KB, patch)
2019-06-24 08:49 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (20.28 KB, patch)
2019-06-25 13:08 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (20.32 KB, patch)
2019-06-26 01:25 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-highsierra (3.25 MB, application/zip)
2019-06-26 02:40 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews112 for mac-highsierra (2.98 MB, application/zip)
2019-06-26 03:18 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews212 for win-future (13.66 MB, application/zip)
2019-06-26 03:34 PDT, EWS Watchlist
no flags Details
Patch (22.44 KB, patch)
2019-06-26 10:56 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (21.62 KB, patch)
2019-07-01 01:09 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (23.13 KB, patch)
2019-07-01 06:32 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (23.21 KB, patch)
2019-07-03 00:10 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (23.24 KB, patch)
2019-07-03 06:10 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (22.56 KB, patch)
2019-07-27 11:07 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (29.66 KB, patch)
2019-08-22 08:24 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (29.66 KB, patch)
2019-08-22 08:39 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (17.69 KB, patch)
2019-08-22 08:55 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (21.57 KB, patch)
2019-08-23 00:18 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (21.48 KB, patch)
2019-08-23 12:45 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (21.21 KB, patch)
2019-10-10 01:45 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (22.40 KB, patch)
2019-10-10 02:43 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews210 for win-future (13.80 MB, application/zip)
2019-10-10 18:29 PDT, EWS Watchlist
no flags Details
Patch (21.33 KB, patch)
2020-02-25 06:24 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (21.34 KB, patch)
2020-09-09 05:54 PDT, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2019-06-24 08:43:42 PDT
Cross-origin prefetches should be reused for top level navigations.
Comment 1 Rob Buis 2019-06-24 08:49:46 PDT
Created attachment 372766 [details]
Patch
Comment 2 Rob Buis 2019-06-25 13:08:29 PDT
Created attachment 372854 [details]
Patch
Comment 3 Rob Buis 2019-06-26 01:25:01 PDT
Created attachment 372908 [details]
Patch
Comment 4 EWS Watchlist 2019-06-26 02:40:26 PDT
Comment on attachment 372908 [details]
Patch

Attachment 372908 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12578887

New failing tests:
http/wpt/prefetch/navigate-reuse-prefetch.html
Comment 5 EWS Watchlist 2019-06-26 02:40:28 PDT
Created attachment 372909 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 6 EWS Watchlist 2019-06-26 03:18:42 PDT
Comment on attachment 372908 [details]
Patch

Attachment 372908 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12578911

New failing tests:
http/wpt/prefetch/navigate-reuse-prefetch.html
Comment 7 EWS Watchlist 2019-06-26 03:18:44 PDT
Created attachment 372910 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 8 EWS Watchlist 2019-06-26 03:34:47 PDT
Comment on attachment 372908 [details]
Patch

Attachment 372908 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12578978

New failing tests:
http/wpt/prefetch/navigate-reuse-prefetch.html
Comment 9 EWS Watchlist 2019-06-26 03:34:50 PDT
Created attachment 372911 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 10 Rob Buis 2019-06-26 10:56:23 PDT
Created attachment 372936 [details]
Patch
Comment 11 youenn fablet 2019-06-28 12:07:54 PDT
Comment on attachment 372936 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372936&action=review

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:421
> +        if (auto session = networkProcess().networkSession(loadParameters.sessionID)) {

auto*

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:422
> +            auto url = loadParameters.request.url();

const auto&

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:423
> +            if (session->waitingForPendingPrefetch(url)) {

I would rename waitingForPendingPrefetch to hasOngoingPrefetch.

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:936
> +void NetworkConnectionToWebProcess::prefetchFinished(const PAL::SessionID& sessionID, const URL& url, WebCore::ResourceResponse&& response, RefPtr<WebCore::SharedBuffer>&& buffer)

We are not consistent in const PAL::SessionID& vs. PAL::SessionID.
I slightly prefer PAL::SessionID.

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:942
> +                m_networkResourceLoaders.add(crossOriginPrefetch->identifier(), crossOriginPrefetch.releaseNonNull());

I am not clear why we need this.
If the crossOriginPrefetch is finished, why are we keeping the NetworkResourceLoader in the map? Is it for being able to clear it shortly after?

If session->hasPendingNavigation(url) is true, session->takeCrossOriginPrefetchLoad(url) should be true as well in most cases but maybe not all?

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:943
> +            auto loadParameters = session->takePendingNavigation(url);

We are doing two HashMap queries hasPendingNavigation and takePendingNavigation. With find, we could do just one.

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:946
> +            loader->start();

Could be rewritten as something like m_networkResourceLoaders.add(loadParameters.identifier, WTFMove(loader)).iterator->value->start();

> Source/WebKit/NetworkProcess/NetworkSession.cpp:237
> +    if (auto ret = m_pendingCrossOriginPrefetchLoads.take(url))

s/ret/result

> LayoutTests/http/wpt/prefetch/resources/navigate-redirect.html:9
> +  document.body.appendChild(link);

You can probably use .sub.html to write it directly as
<link rel="prefetch" href=""http://{{host[alt][]}}:{{ports[http][0]}}/WebKit/prefetch/resources/prefetched-main-resource-redirect.py"
Comment 12 Rob Buis 2019-07-01 01:09:17 PDT
Created attachment 373213 [details]
Patch
Comment 13 Rob Buis 2019-07-01 06:32:16 PDT
Created attachment 373229 [details]
Patch
Comment 14 Rob Buis 2019-07-01 08:09:27 PDT
Comment on attachment 372936 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372936&action=review

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:421
>> +        if (auto session = networkProcess().networkSession(loadParameters.sessionID)) {
> 
> auto*

Done.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:422
>> +            auto url = loadParameters.request.url();
> 
> const auto&

Done.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:423
>> +            if (session->waitingForPendingPrefetch(url)) {
> 
> I would rename waitingForPendingPrefetch to hasOngoingPrefetch.

Done.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:936
>> +void NetworkConnectionToWebProcess::prefetchFinished(const PAL::SessionID& sessionID, const URL& url, WebCore::ResourceResponse&& response, RefPtr<WebCore::SharedBuffer>&& buffer)
> 
> We are not consistent in const PAL::SessionID& vs. PAL::SessionID.
> I slightly prefer PAL::SessionID.

Done.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:942
>> +                m_networkResourceLoaders.add(crossOriginPrefetch->identifier(), crossOriginPrefetch.releaseNonNull());
> 
> I am not clear why we need this.
> If the crossOriginPrefetch is finished, why are we keeping the NetworkResourceLoader in the map? Is it for being able to clear it shortly after?
> 
> If session->hasPendingNavigation(url) is true, session->takeCrossOriginPrefetchLoad(url) should be true as well in most cases but maybe not all?

Yes, we need it because it will be cleared in NetworkConnectionToWebProcess::didCleanupResourceLoader. I did not like that code though, so I solved it differently, a lot like keep alive code.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:943
>> +            auto loadParameters = session->takePendingNavigation(url);
> 
> We are doing two HashMap queries hasPendingNavigation and takePendingNavigation. With find, we could do just one.

Done.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:946
>> +            loader->start();
> 
> Could be rewritten as something like m_networkResourceLoaders.add(loadParameters.identifier, WTFMove(loader)).iterator->value->start();

Done.

>> Source/WebKit/NetworkProcess/NetworkSession.cpp:237
>> +    if (auto ret = m_pendingCrossOriginPrefetchLoads.take(url))
> 
> s/ret/result

Done.

>> LayoutTests/http/wpt/prefetch/resources/navigate-redirect.html:9
>> +  document.body.appendChild(link);
> 
> You can probably use .sub.html to write it directly as
> <link rel="prefetch" href=""http://{{host[alt][]}}:{{ports[http][0]}}/WebKit/prefetch/resources/prefetched-main-resource-redirect.py"

Done.
Comment 15 Rob Buis 2019-07-01 08:10:52 PDT
Comment on attachment 373229 [details]
Patch

Note that I think we could/should make this work for same-origin prefetches as well, but it seems non trivial, so I am hoping this can go in first.
Comment 16 Ryosuke Niwa 2019-07-01 08:50:50 PDT
Comment on attachment 373229 [details]
Patch

This feature seems problematic from fingerprinting standpoint.

Wouldn't this feature allow domain A.com to prefetch tracker.com and then the user visits B.com, it can cycle through tracker.com to detect whether the user had visited A.com or not by detecting whether the page load had hit the prefetch cache or not?
Comment 17 Ryosuke Niwa 2019-07-01 08:51:38 PDT
+John, Maciej for assessing the fingerprinting issue I mention above.
Comment 18 youenn fablet 2019-07-02 11:04:23 PDT
> Wouldn't this feature allow domain A.com to prefetch tracker.com and then
> the user visits B.com, it can cycle through tracker.com to detect whether
> the user had visited A.com or not by detecting whether the page load had hit
> the prefetch cache or not?

Only the main frame is allowed to use the prefetch cache.
A.com can prefetch tracker.com so that A.com -> tracker.com -> B.com will be faster than it is now.
Comment 19 youenn fablet 2019-07-02 11:12:39 PDT
Comment on attachment 373229 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=373229&action=review

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:430
> +                session->addPendingNavigation(url, WTFMove(loadParameters));

It seems that load will hang in case the prefetch loading fails?
We should probably trigger the load with the load path.

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:946
> +        if (Optional<NetworkResourceLoadParameters> loadParameters = session->takePendingNavigation(url)) {

auto

> Source/WebKit/NetworkProcess/NetworkSession.cpp:250
> +        return m_pendingNavigations.take(url);

This does two search on the map, we should be able with find/remove to make just one.
Comment 20 Rob Buis 2019-07-03 00:10:33 PDT
Created attachment 373384 [details]
Patch
Comment 21 Ryosuke Niwa 2019-07-03 00:32:52 PDT
(In reply to youenn fablet from comment #18)
> > Wouldn't this feature allow domain A.com to prefetch tracker.com and then
> > the user visits B.com, it can cycle through tracker.com to detect whether
> > the user had visited A.com or not by detecting whether the page load had hit
> > the prefetch cache or not?
> 
> Only the main frame is allowed to use the prefetch cache.
> A.com can prefetch tracker.com so that A.com -> tracker.com -> B.com will be
> faster than it is now.

Yeah, so this will create a new tracking mechanism because you can detect that it was fast (and therefore prefetch'ed). Basically, A.com can contain a tracking script which inserts <link rel=prefetch href=tracker.com?url=a.com> to the main page. When B.com is loaded, it can immediately navigate to tracker.com?url=a.com to detect that the user had visited A.com based on how fast tracker.com loads.

I'm pretty certain there is a clever trick to make this tracking mechanism more powerful. In general, this feature seems to circumvent the whole partitioning of our disk cache based on top-level domain so there could be other privacy holes.
Comment 22 Rob Buis 2019-07-03 06:10:29 PDT
Created attachment 373388 [details]
Patch
Comment 23 Maciej Stachowiak 2019-07-03 14:35:14 PDT
Is a page limited in how many pages it can prefetch? Does <link rel=prefetch> have any signal for whether the prefetch succeeded or how long it took, such as load or error events?
Comment 24 Maciej Stachowiak 2019-07-03 14:38:14 PDT
Also, just to confirm, is `<link rel=prefetch>` supposed to bypass cache partitioning, and is it supposed to use the first party cookies for the prefetched resource?
Comment 25 Maciej Stachowiak 2019-07-03 14:42:01 PDT
Hmmm, if `<link rel=prefetch>` is allowed to load with first party cookies and gets Referer, then it could be used a tracking pixel that bypasses ITP, even without redirect tricks.

BTW, we know motivated trackers *will* use redirect tricks to get a user ID that then gets boosted into first party storage if they are able to. They did it with HSTS.

I'd rather not see this land until there is a clear privacy story (unless there's a WebKit port that really wants to ship prefetch regardless of the privacy problems).
Comment 26 youenn fablet 2019-07-03 14:42:25 PDT
(In reply to Maciej Stachowiak from comment #23)
> Is a page limited in how many pages it can prefetch? Does <link
> rel=prefetch> have any signal for whether the prefetch succeeded or how long
> it took, such as load or error events?

Currently, there is no such limitation in the implementation.
This seems reasonable to me as well as the possibility to scope which pages can get access to which prefetches.

As for load/error events, this is under discussion on GitHub.

(In reply to Maciej Stachowiak from comment #24)
> Also, just to confirm, is `<link rel=prefetch>` supposed to bypass cache
> partitioning, and is it supposed to use the first party cookies for the
> prefetched resource?

The idea is that it is supposed to bypass cache partitioning in a specific way, no data persistency, short lifetime and restricted access to specific loads.
The prefetch load is no credentials and manual redirect mode.
Comment 27 youenn fablet 2019-07-03 14:46:16 PDT
GitHub links: https://github.com/whatwg/fetch/pull/881 and https://github.com/whatwg/html/pull/4115
Comment 28 Maciej Stachowiak 2019-07-03 18:01:10 PDT
The Resource Hints spec that is linked by the HTML spec currently does not say anything about loads being without credentials, and even seems to allow CORS to read back the contents of a load with-credentials. And it also doesn't say to suppress load and error events, which happen for other link types that load a resource.

Is that Resource Hints spec obsolete and planned to be replaced with material inline in Fetch and HTML?

https://w3c.github.io/resource-hints/
Comment 29 youenn fablet 2019-07-03 18:07:24 PDT
The prefetch part is being specced as PRs that are not merged right now.
These PRs require the notion of partitioning, a new kind of load maybe so they are not straightforward.
Yoav knows more about the current state of the PRs.

For instance, I think the fetch PR states that credentials more is same origin.
Comment 30 Maciej Stachowiak 2019-07-03 18:13:34 PDT
If prefetching is without credentials, does that mean it can not safely be used for main resources in general? Only for sub resources known not to depend on cookies?
Comment 31 Maciej Stachowiak 2019-07-03 18:14:31 PDT
BTW doing a privacy review of something only described by PRs to multiple specs is pretty hard. it would be nice to have an explainer of how the whole thing is supposed to work.
Comment 32 Rob Buis 2019-07-04 10:08:32 PDT
Comment on attachment 373229 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=373229&action=review

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:430
>> +                session->addPendingNavigation(url, WTFMove(loadParameters));
> 
> It seems that load will hang in case the prefetch loading fails?
> We should probably trigger the load with the load path.

I think it depends on the load fail. It seems 4xx and 5xx responses go through  NetworkResourceLoader::didFinishLoading so they are on the right track. I found out however that the code to re-use the prefetch in NetworkResourceLoader::retrieveCacheEntry does not work for non-cacheable responses. To try to fix this I created https://bugs.webkit.org/show_bug.cgi?id=199499.

The rest of the failures I expect to go through NetworkResourceLoader::didFailLoading, where we could add a hook similar to NetworkConnectionToWebProcess::prefetchFinished, but for load failures (authentication, bad port etc.).

I would be curious to know if there are more failure code paths though.

>> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:946
>> +        if (Optional<NetworkResourceLoadParameters> loadParameters = session->takePendingNavigation(url)) {
> 
> auto

Done.

>> Source/WebKit/NetworkProcess/NetworkSession.cpp:250
>> +        return m_pendingNavigations.take(url);
> 
> This does two search on the map, we should be able with find/remove to make just one.

Done.
Comment 33 Yoav Weiss 2019-07-08 02:30:00 PDT
(In reply to youenn fablet from comment #29)
> The prefetch part is being specced as PRs that are not merged right now.
> These PRs require the notion of partitioning, a new kind of load maybe so
> they are not straightforward.
> Yoav knows more about the current state of the PRs.

The intent is indeed to integrate Prefetch's processing model into HTML and obsolete the parts of the Resource Hints spec which refer to it.


> 
> For instance, I think the fetch PR states that credentials more is same
> origin.
Comment 34 Rob Buis 2019-07-27 11:07:12 PDT
Created attachment 375031 [details]
Patch
Comment 35 Dominic Farolino 2019-07-30 21:21:52 PDT
Just an update to this thread: Chromium is interested in implementing the changes to prefetch requests proposed by Youenn in [1]. I've filed a request for a TAG review [2] as a part of our Intent to Implement, but ultimately having an explainer for these changes and their suspected effects would be ideal, as Maciej mentioned.

[1]: https://github.com/w3c/resource-hints/issues/82
[2]: https://github.com/w3ctag/design-reviews/issues/398
Comment 36 Rob Buis 2019-08-22 08:24:51 PDT
Created attachment 377007 [details]
Patch
Comment 37 Rob Buis 2019-08-22 08:39:41 PDT
Created attachment 377008 [details]
Patch
Comment 38 Rob Buis 2019-08-22 08:55:53 PDT
Created attachment 377011 [details]
Patch
Comment 39 Rob Buis 2019-08-23 00:18:46 PDT
Created attachment 377106 [details]
Patch
Comment 40 Darin Adler 2019-08-23 10:43:53 PDT
Comment on attachment 377106 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377106&action=review

> Source/WebCore/platform/network/ResourceRequestBase.cpp:275
> +    return httpHeaderField(HTTPHeaderName::Purpose) == "prefetch";

Is this case sensitive, or should be it equalLettersIgnoringASCIICase?
Comment 41 Rob Buis 2019-08-23 12:45:41 PDT
Created attachment 377150 [details]
Patch
Comment 42 Rob Buis 2019-08-23 14:22:13 PDT
(In reply to Darin Adler from comment #40)
> Comment on attachment 377106 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377106&action=review
> 
> > Source/WebCore/platform/network/ResourceRequestBase.cpp:275
> > +    return httpHeaderField(HTTPHeaderName::Purpose) == "prefetch";
> 
> Is this case sensitive, or should be it equalLettersIgnoringASCIICase?

Unfortunately AFAIK it is not specified (yet), but I am fine with case insensitive, so I updated the patch.
Comment 43 Frédéric Wang (:fredw) 2019-08-29 03:49:49 PDT
(In reply to Rob Buis from comment #42)
> Unfortunately AFAIK it is not specified (yet), but I am fine with case
> insensitive, so I updated the patch.

Do you know if there is any plan to specify it? I think this should be reported to the spec authors and we should have tests for it.
Comment 44 Dominic Farolino 2019-10-01 16:00:24 PDT
It is specified. This isn't a prefetch-specific thing; HTML Standard notes that link types are specified by link "keywords", and all keywords are ASCII case-insensitive [1].

[1]: https://html.spec.whatwg.org/multipage/links.html#linkTypes
Comment 45 Rob Buis 2019-10-10 01:45:39 PDT
Created attachment 380610 [details]
Patch
Comment 46 Rob Buis 2019-10-10 02:43:48 PDT
Created attachment 380617 [details]
Patch
Comment 47 EWS Watchlist 2019-10-10 18:28:59 PDT
Comment on attachment 380617 [details]
Patch

Attachment 380617 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13116694

New failing tests:
http/tests/preload/picture-type.html
Comment 48 EWS Watchlist 2019-10-10 18:29:02 PDT
Created attachment 380711 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 49 Rob Buis 2020-02-25 06:24:00 PST
Created attachment 391646 [details]
Patch
Comment 50 youenn fablet 2020-03-02 06:38:20 PST
Comment on attachment 391646 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391646&action=review

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:452
> +            if (session->hasOngoingCrossOriginPrefetch(url)) {

What if navigation is POST form?

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:1126
> +            m_networkResourceLoaders.add(identifier, WTFMove(loader)).iterator->value->start();

I do not think we want to wait for prefetchFinished since we want to parse data as fast as possible.
Ideally we would just migrate the loader.
Maybe we could wait for the response though?
Comment 51 Rob Buis 2020-09-09 05:54:54 PDT
Created attachment 408325 [details]
Patch
Comment 52 John Wilander 2020-09-09 07:53:40 PDT
How is the matching between cross-origin prefetch and navigation done? Does this prevent us from stopping link decoration tracking down the road? Would we have to apply any link decoration prevention to these prefetch requests “just in case?”
Comment 53 Dima Voytenko 2021-07-21 09:34:34 PDT
(In reply to John Wilander from comment #52)
> How is the matching between cross-origin prefetch and navigation done? Does
> this prevent us from stopping link decoration tracking down the road? Would
> we have to apply any link decoration prevention to these prefetch requests
> “just in case?”

If the caching is done with top origin as a key then no link decoration prevention or similar tactics are needed.
Comment 54 John Wilander 2021-07-21 10:02:53 PDT
(In reply to Dima Voytenko from comment #53)
> (In reply to John Wilander from comment #52)
> > How is the matching between cross-origin prefetch and navigation done? Does
> > this prevent us from stopping link decoration tracking down the road? Would
> > we have to apply any link decoration prevention to these prefetch requests
> > “just in case?”
> 
> If the caching is done with top origin as a key then no link decoration
> prevention or similar tactics are needed.

Is the cache partitioned in that fashion? That would mean that the prefetch doesn't apply to cross-origin navigations, right?
Comment 55 John Wilander 2021-07-21 10:07:07 PDT
(In reply to John Wilander from comment #54)
> (In reply to Dima Voytenko from comment #53)
> > (In reply to John Wilander from comment #52)
> > > How is the matching between cross-origin prefetch and navigation done? Does
> > > this prevent us from stopping link decoration tracking down the road? Would
> > > we have to apply any link decoration prevention to these prefetch requests
> > > “just in case?”
> > 
> > If the caching is done with top origin as a key then no link decoration
> > prevention or similar tactics are needed.
> 
> Is the cache partitioned in that fashion? That would mean that the prefetch
> doesn't apply to cross-origin navigations, right?

What I'm talking about here is this scenario:

1. The user is on social.example
2. social.example prefetches news.example/article?trackingID=abcd1234
3. The user clicks to navigate to what looks like news.example/article
4. social.example actually navigates to news.example/article?trackingID=abcd1234 in an attempt to track the user cross-site
5. Tracking prevention has code to detect link decoration tracking and remove it but no network request is made since there's a cache hit and the user is navigated to news.example/article?trackingID=abcd1234 and tracked
Comment 56 Dima Voytenko 2021-07-21 11:13:46 PDT
(In reply to John Wilander from comment #55)
> (In reply to John Wilander from comment #54)
> > (In reply to Dima Voytenko from comment #53)
> > > (In reply to John Wilander from comment #52)
> > > > How is the matching between cross-origin prefetch and navigation done? Does
> > > > this prevent us from stopping link decoration tracking down the road? Would
> > > > we have to apply any link decoration prevention to these prefetch requests
> > > > “just in case?”
> > > 
> > > If the caching is done with top origin as a key then no link decoration
> > > prevention or similar tactics are needed.
> > 
> > Is the cache partitioned in that fashion? That would mean that the prefetch
> > doesn't apply to cross-origin navigations, right?
> 
> What I'm talking about here is this scenario:
> 
> 1. The user is on social.example
> 2. social.example prefetches news.example/article?trackingID=abcd1234
> 3. The user clicks to navigate to what looks like news.example/article
> 4. social.example actually navigates to
> news.example/article?trackingID=abcd1234 in an attempt to track the user
> cross-site
> 5. Tracking prevention has code to detect link decoration tracking and
> remove it but no network request is made since there's a cache hit and the
> user is navigated to news.example/article?trackingID=abcd1234 and tracked

Got it. I misunderstood this issue as having to do with the prefetch of the cross-origin resources (such as hero image) on a same-origin page. E.g. a scenario where my.example/one.html uses this prefetch:

```
<link rel="prefetch" href="//cdn.example/images/big.jpeg">
```

in hope to use this resource soon and reduce its perceived network time. This kind of scenario is described in https://developer.mozilla.org/en-US/docs/Web/HTTP/Link_prefetching_FAQ.

This kind of prefetch has to be done in the context of the "my.example" origin's cache, otherwise it'd be a cache miss later. Right?

If so, I don't think `<link rel="prefetch" href="//news.example/article?trackingID=abcd">` will make any sense. If follow the rule above, this prefetch happens in the context of "my.example" origin, then the top-level navigation to this URL would be always a cache miss.
Comment 57 John Wilander 2021-07-22 14:32:41 PDT
(In reply to Dima Voytenko from comment #56)
> (In reply to John Wilander from comment #55)
> > (In reply to John Wilander from comment #54)
> > > (In reply to Dima Voytenko from comment #53)
> > > > (In reply to John Wilander from comment #52)
> > > > > How is the matching between cross-origin prefetch and navigation done? Does
> > > > > this prevent us from stopping link decoration tracking down the road? Would
> > > > > we have to apply any link decoration prevention to these prefetch requests
> > > > > “just in case?”
> > > > 
> > > > If the caching is done with top origin as a key then no link decoration
> > > > prevention or similar tactics are needed.
> > > 
> > > Is the cache partitioned in that fashion? That would mean that the prefetch
> > > doesn't apply to cross-origin navigations, right?
> > 
> > What I'm talking about here is this scenario:
> > 
> > 1. The user is on social.example
> > 2. social.example prefetches news.example/article?trackingID=abcd1234
> > 3. The user clicks to navigate to what looks like news.example/article
> > 4. social.example actually navigates to
> > news.example/article?trackingID=abcd1234 in an attempt to track the user
> > cross-site
> > 5. Tracking prevention has code to detect link decoration tracking and
> > remove it but no network request is made since there's a cache hit and the
> > user is navigated to news.example/article?trackingID=abcd1234 and tracked
> 
> Got it. I misunderstood this issue as having to do with the prefetch of the
> cross-origin resources (such as hero image) on a same-origin page. E.g. a
> scenario where my.example/one.html uses this prefetch:
> 
> ```
> <link rel="prefetch" href="//cdn.example/images/big.jpeg">
> ```
> 
> in hope to use this resource soon and reduce its perceived network time.
> This kind of scenario is described in
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Link_prefetching_FAQ.
> 
> This kind of prefetch has to be done in the context of the "my.example"
> origin's cache, otherwise it'd be a cache miss later. Right?
> 
> If so, I don't think `<link rel="prefetch"
> href="//news.example/article?trackingID=abcd">` will make any sense. If
> follow the rule above, this prefetch happens in the context of "my.example"
> origin, then the top-level navigation to this URL would be always a cache
> miss.

Then I'm confused by the title of this bug: "Cross-origin ongoing prefetches should be reused for top-level navigations"
Comment 58 Rob Buis 2021-07-23 14:14:40 PDT
(In reply to John Wilander from comment #57)
> > Got it. I misunderstood this issue as having to do with the prefetch of the
> > cross-origin resources (such as hero image) on a same-origin page. E.g. a
> > scenario where my.example/one.html uses this prefetch:
> > 
> > ```
> > <link rel="prefetch" href="//cdn.example/images/big.jpeg">
> > ```
> > 
> > in hope to use this resource soon and reduce its perceived network time.
> > This kind of scenario is described in
> > https://developer.mozilla.org/en-US/docs/Web/HTTP/Link_prefetching_FAQ.
> > 
> > This kind of prefetch has to be done in the context of the "my.example"
> > origin's cache, otherwise it'd be a cache miss later. Right?
> > 
> > If so, I don't think `<link rel="prefetch"
> > href="//news.example/article?trackingID=abcd">` will make any sense. If
> > follow the rule above, this prefetch happens in the context of "my.example"
> > origin, then the top-level navigation to this URL would be always a cache
> > miss.
> 
> Then I'm confused by the title of this bug: "Cross-origin ongoing prefetches
> should be reused for top-level navigations"

This bug was a way to address a question Youenn posted in an internal discussion:
- If the navigation load kicks in before the prefetch is finished, the prefetch will not be used. Can we improve this?

So really this bug is to optimise above scenario.
Since my work was related to the cross origin prefetch cache, it seems to me the title is accurate given the context?
Comment 59 John Wilander 2021-07-23 16:14:01 PDT
(In reply to Rob Buis from comment #58)
> (In reply to John Wilander from comment #57)
> > > Got it. I misunderstood this issue as having to do with the prefetch of the
> > > cross-origin resources (such as hero image) on a same-origin page. E.g. a
> > > scenario where my.example/one.html uses this prefetch:
> > > 
> > > ```
> > > <link rel="prefetch" href="//cdn.example/images/big.jpeg">
> > > ```
> > > 
> > > in hope to use this resource soon and reduce its perceived network time.
> > > This kind of scenario is described in
> > > https://developer.mozilla.org/en-US/docs/Web/HTTP/Link_prefetching_FAQ.
> > > 
> > > This kind of prefetch has to be done in the context of the "my.example"
> > > origin's cache, otherwise it'd be a cache miss later. Right?
> > > 
> > > If so, I don't think `<link rel="prefetch"
> > > href="//news.example/article?trackingID=abcd">` will make any sense. If
> > > follow the rule above, this prefetch happens in the context of "my.example"
> > > origin, then the top-level navigation to this URL would be always a cache
> > > miss.
> > 
> > Then I'm confused by the title of this bug: "Cross-origin ongoing prefetches
> > should be reused for top-level navigations"
> 
> This bug was a way to address a question Youenn posted in an internal
> discussion:
> - If the navigation load kicks in before the prefetch is finished, the
> prefetch will not be used. Can we improve this?
> 
> So really this bug is to optimise above scenario.
> Since my work was related to the cross origin prefetch cache, it seems to me
> the title is accurate given the context?

With the proposed change, can SiteA prefetch anything from SiteB that will be used in a navigation to SiteB?

The title "Cross-origin ongoing prefetches should be reused for top-level navigations" reads to me as "If SiteA has started to pre-fetch a URL from SiteB, and the user navigates to that SiteB URL, use the pre-fetch instead of starting a new network load."

If my understanding is correct, then there are implications for link decoration tracking.
Comment 60 Dima Voytenko 2021-07-26 09:39:31 PDT
(In reply to John Wilander from comment #59)
> (In reply to Rob Buis from comment #58)
> > (In reply to John Wilander from comment #57)
> > > Then I'm confused by the title of this bug: "Cross-origin ongoing prefetches
> > > should be reused for top-level navigations"
> > 
> > This bug was a way to address a question Youenn posted in an internal
> > discussion:
> > - If the navigation load kicks in before the prefetch is finished, the
> > prefetch will not be used. Can we improve this?
> > 
> > So really this bug is to optimise above scenario.
> > Since my work was related to the cross origin prefetch cache, it seems to me
> > the title is accurate given the context?
> 
> With the proposed change, can SiteA prefetch anything from SiteB that will
> be used in a navigation to SiteB?
> 
> The title "Cross-origin ongoing prefetches should be reused for top-level
> navigations" reads to me as "If SiteA has started to pre-fetch a URL from
> SiteB, and the user navigates to that SiteB URL, use the pre-fetch instead
> of starting a new network load."
> 
> If my understanding is correct, then there are implications for link
> decoration tracking.

From my side I'm pointing out that there might be a spec problem. I don't quite see how we could support `<link rel="prefetch" href="resource-in-next-page">` and `<link rel="prefetch" href="document-for-top-nav">` using the same format for the cross-origin case. Is it possible to confirm this with the spec owners?