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

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 178540: Move AppCache loading to the NetworkProcess
https://bugs.webkit.org/show_bug.cgi?id=178540

Attachment 334404: Patch

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




--- Comment #14 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 334404
  --> https://bugs.webkit.org/attachment.cgi?id=334404
Patch

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

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:273
> +	   // FIXME: Add support for ApplicationCache in inspector.

This hopefully won't be worth it because ApplicationCache is on its way out.

> Source/WebCore/loader/SubstituteResource.h:52
> +    ResourceResponse& resourceResponse() { return m_response; }

It seems like there could be a cleaner way to set the source than a non-const
resourceResponse accessor.  Maybe just a setter for the source.

> Source/WebCore/loader/appcache/ApplicationCacheResourceLoader.cpp:33
> +RefPtr<ApplicationCacheResourceLoader>
ApplicationCacheResourceLoader::create(unsigned type, CachedResourceLoader&
loader, ResourceRequest&& request, CompletionHandler<void(ResourceOrError&&)>&&
callback)

I think type should be a ApplicationCacheResource::Type instead of an unsigned.

> Source/WebCore/loader/appcache/ApplicationCacheResourceLoader.h:62
> +    unsigned m_type;

ApplicationCacheResource::Type