[webkit-reviews] review denied: [Bug 181686] [Win] Use switch when converting from ResourceRequestCachePolicy to platform cache policy. : [Attachment 331393] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 181686: [Win] Use switch when converting from ResourceRequestCachePolicy to
platform cache policy.
https://bugs.webkit.org/show_bug.cgi?id=181686

Attachment 331393: Patch

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




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

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

Please add a better explanation of why this change is needed, and consider
whether more things are needed in the switch statement.

> Source/WebCore/ChangeLog:10
> +	   The current implementation of toPlatformRequestCachePolicy is not
correct for all cases. 

I don't think this is clear enough. Is the issue that the casting operation is
returning an incorrect kCFURL* constant? I think you should just say that.

Did this stuff break after the Fetch API changes?

> Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp:202
> +	   return kCFURLRequestCachePolicyReloadIgnoringCache

Do you need to do anything to support 'RefreshAnyCacheData'? It might be better
to explicitly call out 'ReloadIgnoringCacheData, and DoNotUseAnyCache' before
the 'default' just to show that they map to kCFURL*IgnoringCache.