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

[bugzilla-daemon at webkit.org]

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
https://bugs.webkit.org/show_bug.cgi?id=178540

Attachment 324448: Patch

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




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

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,
ClientCredentialPolicy::MayAskClientForCredentials,
FetchOptions::Credentials::Include, DoSecurityCheck,
FetchOptions::Mode::NoCors, DoNotIncludeCertificateInfo,
ContentSecurityPolicyImposition::DoPolicyCheck,
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
> +	      
m_pendingEntries.remove(m_currentHandle->originalRequest()->url());

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