Bug 234988 – Disable CFURLCache in WebKit2

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sihui Liu]

...

[Sihui Liu]

Created attachment 448634 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/87619196>

[Sihui Liu]

Created attachment 449670 [details]
Patch

[Sihui Liu]

Created attachment 449681 [details]
Patch

[Geoffrey Garen]

Comment on attachment 449681 [details]
Patch

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

r=me

I added some style comments.

> Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h:362
> +void _CFURLStorageSessionDisableCache(CFURLStorageSessionRef);

Can we make this declaration conditional on HAVE(CFNETWORK_DISABLE_CACHE_SPI)? That way you get an easier to understand compiler error, rather than a linker error, if you try to use this function when you shouldn't.

> Source/WebCore/platform/network/NetworkStorageSession.h:123
> +    enum class ShouldDisableURLCache : bool { No, Yes };

I would call this ShouldDisableCFURLCache, since we still enable the WebCore URL cache even when this is true.

> Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp:49
> +    bool isCacheDisabled = false;

Can we just use the shouldDisableURLCache parameter instead of tracking this boolean separately?

> Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:184
> +    bool isCacheDisabled = false;

Can we just use the shouldDisableURLCache parameter instead of tracking this boolean separately?

[Sihui Liu]

(In reply to Geoffrey Garen from comment #5)
> Comment on attachment 449681 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=449681&action=review
> 
> r=me
> 
> I added some style comments.
> 
> > Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h:362
> > +void _CFURLStorageSessionDisableCache(CFURLStorageSessionRef);
> 
> Can we make this declaration conditional on
> HAVE(CFNETWORK_DISABLE_CACHE_SPI)? That way you get an easier to understand
> compiler error, rather than a linker error, if you try to use this function
> when you shouldn't.

Sure.

> 
> > Source/WebCore/platform/network/NetworkStorageSession.h:123
> > +    enum class ShouldDisableURLCache : bool { No, Yes };
> 
> I would call this ShouldDisableCFURLCache, since we still enable the WebCore
> URL cache even when this is true.

Sure, I didn't add 'CF' as it does not exist on other platforms. 

> > Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp:49
> > +    bool isCacheDisabled = false;
> 
> Can we just use the shouldDisableURLCache parameter instead of tracking this
> boolean separately?

Sure.

> 
> > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:184
> > +    bool isCacheDisabled = false;
> 
> Can we just use the shouldDisableURLCache parameter instead of tracking this
> boolean separately?

Sure.

[Sihui Liu]

Created attachment 449707 [details]
Patch for landing

[EWS]

Committed r288389 (246286@main): <https://commits.webkit.org/246286@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449707 [details].