[webkit-reviews] review granted: [Bug 54683] WebKit2: The CFNetwork Cache should be shared between the UI Process and the Web Process on Windows : [Attachment 82852] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Jessie Berlin
<jberlin at webkit.org>'s request for review:
Bug 54683: WebKit2: The CFNetwork Cache should be shared between the UI Process
and the Web Process on Windows
https://bugs.webkit.org/show_bug.cgi?id=54683

Attachment 82852: Patch
https://bugs.webkit.org/attachment.cgi?id=82852&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=82852&action=review

> Source/WebKit2/ChangeLog:22
> +	   Add the parameters for the path to the cache, the disk capacity, and
the memory capacity:
> +	   * Shared/WebProcessCreationParameters.cpp:
> +	   (WebKit::WebProcessCreationParameters::encode):
> +	   (WebKit::WebProcessCreationParameters::decode):
> +	   * Shared/WebProcessCreationParameters.h:
> + 
> +	   * UIProcess/win/WebContextWin.cpp:
> +	   (WebKit::WebContext::platformInitializeWebProcess):
> +	   Make sure to remove the ending slash, as CFNetwork does not
recognize the directory with
> +	   that slash.
> +
> +	   * WebProcess/win/WebProcessWin.cpp:
> +	   (WebKit::WebProcess::platformInitializeWebProcess):
> +	   Create a cache using the path, disk capacity, and memory capacity
and set it as default.

Having some comments above the items they apply to and some below confuses me!

> Source/WebKit2/Shared/WebProcessCreationParameters.cpp:72
>      encoder->encode(shouldPaintNativeControls);
> +    encoder->encode(cfURLCachePath);
> +    encoder->encode(cfURLCacheMemoryCapacity);
> +    encoder->encode(cfURLCacheDiskCapacity);

It would be nice to match the order in which these are declared in the struct.

> Source/WebKit2/Shared/WebProcessCreationParameters.cpp:128
>      if (!decoder->decode(parameters.shouldPaintNativeControls))
>	   return false;
> +    if (!decoder->decode(parameters.cfURLCachePath))
> +	   return false;
> +    if (!decoder->decode(parameters.cfURLCacheMemoryCapacity))
> +	   return false;
> +    if (!decoder->decode(parameters.cfURLCacheDiskCapacity))
> +	   return false;

Ditto.

> Source/WebKit2/UIProcess/win/WebContextWin.cpp:64
> +    parameters.cfURLCachePath = wkCopyFoundationCacheDirectory();

Presumably wkCopyFoundationCacheDirectory returns a CFStringRef, which you're
leaking?