...
Created attachment 448634 [details] Patch
<rdar://problem/87619196>
Created attachment 449670 [details] Patch
Created attachment 449681 [details] Patch
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?
(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.
Created attachment 449707 [details] Patch for landing
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].