[webkit-reviews] review granted: [Bug 40627] Limit ApplicationCache Total and Per-Origin Storage Capacity (Quotas) : [Attachment 59425] [PATCH] Part 1 - Add Total and Per-Origin Quota Preferences.

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 40627: Limit ApplicationCache Total and Per-Origin Storage Capacity
(Quotas)
https://bugs.webkit.org/show_bug.cgi?id=40627

Attachment 59425: [PATCH] Part 1 - Add Total and Per-Origin Quota Preferences.
https://bugs.webkit.org/attachment.cgi?id=59425&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
> diff --git a/WebCore/DerivedSources.make b/WebCore/DerivedSources.make
> +ifeq ($(findstring ENABLE_OFFLINE_WEB_APPLICATIONS,$(FEATURE_DEFINES)),
ENABLE_OFFLINE_WEB_APPLICATIONS)
> +    WEBCORE_EXPORT_DEPENDENCIES := $(WEBCORE_EXPORT_DEPENDENCIES)
WebCore.OfflineWebApplications.exp
> +endif
> diff --git a/WebCore/WebCore.OfflineWebApplications.exp
b/WebCore/WebCore.OfflineWebApplications.exp
> +__ZN7WebCore23ApplicationCacheStorage14setMaximumSizeEx
>
+__ZN7WebCore23ApplicationCacheStorage16storeCopyOfCacheERKNS_6StringEPNS_20App
licationCacheHostE
> +__ZN7WebCore23ApplicationCacheStorage17setCacheDirectoryERKNS_6StringE
> +__ZN7WebCore23ApplicationCacheStorage18vacuumDatabaseFileEv
> +__ZN7WebCore23ApplicationCacheStorage21setDefaultOriginQuotaEx
> +__ZN7WebCore23ApplicationCacheStorage5emptyEv
> diff --git a/WebCore/WebCore.base.exp b/WebCore/WebCore.base.exp
> -__ZN7WebCore23ApplicationCacheStorage14setMaximumSizeEx
>
-__ZN7WebCore23ApplicationCacheStorage16storeCopyOfCacheERKNS_6StringEPNS_20App
licationCacheHostE
> -__ZN7WebCore23ApplicationCacheStorage17setCacheDirectoryERKNS_6StringE
> -__ZN7WebCore23ApplicationCacheStorage18vacuumDatabaseFileEv
> -__ZN7WebCore23ApplicationCacheStorage5emptyEv
> diff --git a/WebCore/WebCore.xcodeproj/project.pbxproj
b/WebCore/WebCore.xcodeproj/project.pbxproj
> +		A57302A611CBD9F500484CC3 /* WebCore.Geolocation.exp */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.exports;
path = WebCore.Geolocation.exp; sourceTree = "<group>"; };
> +		A57302A711CBD9F600484CC3 /* WebCore.OfflineWebApplications.exp
*/ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType =
sourcecode.exports; path = WebCore.OfflineWebApplications.exp; sourceTree =
"<group>"; };
> @@ -11420,9 +11422,11 @@
> +				A57302A611CBD9F500484CC3 /*
WebCore.Geolocation.exp */,
> +				A57302A711CBD9F600484CC3 /*
WebCore.OfflineWebApplications.exp */,

This mechanism has changed on trunk since the patch was made (e.g. bit rot set
it).  However, this is the correct approach, so it just needs to be updated to
work with the new mechanism.

> diff --git a/WebCore/loader/appcache/ApplicationCacheStorage.h
b/WebCore/loader/appcache/ApplicationCacheStorage.h
> +    static const int64_t noQuota = INT64_MAX;

It's unfortunate that this constant isn't "-1" since it's a signed value.  Has
this ship already sailed for existing users?

> diff --git a/WebKit/mac/WebView/WebPreferences.mm
b/WebKit/mac/WebView/WebPreferences.mm
> +- (int64_t)applicationCacheTotalQuota
> +{
> +    return [self _longLongValueForKey:WebKitApplicationCacheTotalQuota];
> +}

I think an ASSERT() here would be good to make sure the dictionary and
WebApplicationCache don't get out of sync, since you set the values in to
places in -setApplicationCacheTotalQuota: below.

> +- (void)setApplicationCacheTotalQuota:(int64_t)quota
> +{
> +    [self _setLongLongValue:quota forKey:WebKitApplicationCacheTotalQuota];
> +
> +    // Application Cache Preferences are stored on the global cache storage
manager, not in Settings.
> +    [WebApplicationCache setMaximumSize:quota];
> +}

r=me after updating the WebCore.*.exp changes to use the new WebCore.exp.in
mechanism on trunk.