[webkit-reviews] review granted: [Bug 197371] Store prefetch redirects in the prefetch cache : [Attachment 371728] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 10 08:26:12 PDT 2019


youenn fablet <youennf at gmail.com> has granted  review:
Bug 197371: Store prefetch redirects in the prefetch cache
https://bugs.webkit.org/show_bug.cgi?id=197371

Attachment 371728: Patch

https://bugs.webkit.org/attachment.cgi?id=371728&action=review




--- Comment #31 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 371728
  --> https://bugs.webkit.org/attachment.cgi?id=371728
Patch

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

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:701
> +    if (isCrossOriginPrefetch()) {

Maybe move the code below as an else to if(!isCrossOriginPrefetch())...

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:703
> +	       redirectRequest.clearPurpose();

I would tend to move this to prefetch cache implementation.

> Source/WebKit/NetworkProcess/cache/PrefetchCache.cpp:90
> +    m_sessionPrefetches->set(requestUrl,
std::make_unique<PrefetchCache::Entry>(WTFMove(redirectResponse),
WTFMove(redirectRequest)));

PrefetchCache::store uses add, here it is using set.
I guess set makes more sense so that we only keep the freshest one.
A test would be good around there for redirections and other responses.

> LayoutTests/http/tests/cache/link-prefetch-main-resource-redirect.html:3
> +if (window.testRunner) {

Other browsers are interested in that functionality.
It would be nice as a testharness based test so that we can easily upstream it
to WPT.


More information about the webkit-reviews mailing list