[webkit-reviews] review denied: [Bug 178540] Move AppCache loading to the NetworkProcess : [Attachment 324448] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 28 14:18:51 PST 2017

Brent Fulgham <bfulgham at webkit.org> has denied Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 178540: Move AppCache loading to the NetworkProcess

Attachment 324448: Patch


--- Comment #11 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 324448
  --> https://bugs.webkit.org/attachment.cgi?id=324448

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

Bots aren't happy with this patch, so r-. I'm also unclear on the
firstRequest/originalRequest changes and what those mean. Maybe a comment in
the changelog would help.

> Source/WebCore/ChangeLog:3
> +	   Move AppCache loading to the NetworkProcess

Yes please!

> Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:484
> +    auto handleOrError =
m_cachedResourceLoader->requestRawResource({ResourceRequest(request), {
SendCallbacks, DoNotSniffContent, BufferData, StoredCredentialsPolicy::Use,
FetchOptions::Credentials::Include, DoSecurityCheck,
FetchOptions::Mode::NoCors, DoNotIncludeCertificateInfo,
DefersLoadingPolicy::AllowDefersLoading, CachingPolicy::DisallowCaching }});

Isn't 'request' already a ResourceRequest? Why does it need to be wrapped again
like this?

> Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:563
> +	      

Was using firstRequest always wrong in the original code? I'm a little confused
by the various 'firstRequest' versus 'originalRequest' code changes.

More information about the webkit-reviews mailing list