193323 – Add a new SPI to request for cache storage quota increase

RESOLVED FIXED 193323

Add a new SPI to request for cache storage quota increase

https://bugs.webkit.org/show_bug.cgi?id=193323

Summary Add a new SPI to request for cache storage quota increase

youenn fablet

Reported 2019-01-10 09:39:08 PST

Add a new SPI to request for cache storage quota increase

Attachments
Patch (34.42 KB, patch) 2019-01-10 11:22 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (18.08 KB, patch) 2019-01-16 12:41 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (32.97 KB, patch) 2019-01-16 12:43 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (30.00 KB, patch) 2019-01-17 10:04 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (29.96 KB, patch) 2019-01-17 11:17 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (32.29 KB, patch) 2019-01-17 12:03 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (37.20 KB, patch) 2019-01-17 14:29 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch (39.33 KB, patch) 2019-01-17 16:59 PST, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (39.38 KB, patch) 2019-01-18 09:04 PST, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2019-01-10 11:22:43 PST

Created attachment 358810 [details] Patch

youenn fablet

Comment 2 2019-01-16 12:41:18 PST

Created attachment 359292 [details] Patch

youenn fablet

Comment 3 2019-01-16 12:43:14 PST

Created attachment 359293 [details] Patch

EWS Watchlist

Comment 4 2019-01-16 17:21:10 PST

Attachment 359293 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:74: The parameter name "quota" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 5 2019-01-17 10:04:59 PST

Created attachment 359386 [details] Patch

EWS Watchlist

Comment 6 2019-01-17 10:07:27 PST

Attachment 359386 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:74: The parameter name "quota" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 7 2019-01-17 11:17:21 PST

Created attachment 359394 [details] Patch

EWS Watchlist

Comment 8 2019-01-17 11:20:54 PST

Attachment 359394 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:74: The parameter name "quota" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.

Alex Christensen

Comment 9 2019-01-17 11:23:41 PST

Comment on attachment 359394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359394&action=review > Source/WebKit/UIProcess/API/Cocoa/WKStorageQuotaDelegatePrivate.h:32 > +@protocol WKStorageQuotaDelegatePrivate <NSObject> _WKWebsiteDataStoreDelegate since this is not public API, then the callback selectors don't need underscores. Also, needs availability macros. > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStoreInternal.h:44 > + RetainPtr<id <WKStorageQuotaDelegatePrivate> > _quotaDelegate; Other delegates use a WeakObjCPtr. I see no compelling reason to not do the same here, especially since the ObjC property is labeled weak. > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreQuotaManager.h:36 > +class WebsiteDataStoreQuotaManager { Why not make this more general so we can add other callbacks here? WebsiteDataStoreDelegate > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreQuotaManager.h:40 > + virtual void requestCacheStorageSpace(const WebCore::SecurityOriginData& topOrigin, const WebCore::SecurityOriginData& frameOrigin, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, WTF::CompletionHandler<void(Optional<uint64_t>)>&& completionHandler) WTF:: is unnecessary here. > LayoutTests/http/wpt/cache-storage/cache-quota.any.js:157 > }).then(() => { You could also add a space here.

youenn fablet

Comment 10 2019-01-17 12:03:16 PST

Created attachment 359400 [details] Patch

EWS Watchlist

Comment 11 2019-01-17 12:05:05 PST

Attachment 359400 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:74: The parameter name "quota" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.

Alex Christensen

Comment 12 2019-01-17 13:05:21 PST

Comment on attachment 359400 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359400&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:48 > +class WebsiteDataStoreManager : public WebKit::WebsiteDataStoreManager { I like "Client" more than "Manager" > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreManager.h:36 > +class WebsiteDataStoreManager { This doesn't really manage anything. It's more of a client. > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreManager.h:42 > + completionHandler({ }); WTF::nullopt > Source/WebKit/WebKit.xcodeproj/project.pbxproj:9838 > + 4118DC1F21E7BF5D00DE04C7 /* _WKWebsiteDataStoreDelegate.h in Headers */, > + 4118DC1F21E7BF5D00DE04C7 /* _WKWebsiteDataStoreDelegate.h in Headers */, This should probably not be duplicated. > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:58 > +@interface WebsiteDataStoreDelegateManager: NSObject <_WKWebsiteDataStoreDelegate> { This doesn't manage delegates. > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:60 > + BOOL shouldAllowRaisingQuota; ivars typically start with an underscore. > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:116 > + globalWebsiteDataStoreDelegateManager = [[WebsiteDataStoreDelegateManager alloc] init]; It seems like this should be owned by the TestController. > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:293 > + globalWebsiteDataStoreDelegateManager->shouldAllowRaisingQuota = false; This should call a setter. > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:430 > + globalWebsiteDataStoreDelegateManager->shouldAllowRaisingQuota = true; ditto

youenn fablet

Comment 13 2019-01-17 14:29:40 PST

Created attachment 359413 [details] Patch

EWS Watchlist

Comment 14 2019-01-17 14:31:06 PST

Attachment 359413 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/cocoa/TestWebsiteDataStoreDelegate.mm:40: The parameter name "quota" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.

youenn fablet

Comment 15 2019-01-17 14:32:09 PST

> > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:48 > > +class WebsiteDataStoreManager : public WebKit::WebsiteDataStoreManager { > > I like "Client" more than "Manager" Moved from Manager to Client. > > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreManager.h:42 > > + completionHandler({ }); > > WTF::nullopt I kept { }. This is equivalent, terse and is not WTF specific. > > Source/WebKit/WebKit.xcodeproj/project.pbxproj:9838 > > + 4118DC1F21E7BF5D00DE04C7 /* _WKWebsiteDataStoreDelegate.h in Headers */, > > + 4118DC1F21E7BF5D00DE04C7 /* _WKWebsiteDataStoreDelegate.h in Headers */, > > This should probably not be duplicated. Not sure what happened there. > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:60 > > + BOOL shouldAllowRaisingQuota; > > ivars typically start with an underscore. OK > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:116 > > + globalWebsiteDataStoreDelegateManager = [[WebsiteDataStoreDelegateManager alloc] init]; > > It seems like this should be owned by the TestController. TestController is fully C++ so I am not really sure how to handle the includes properly. I kept it as a global. > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:293 > > + globalWebsiteDataStoreDelegateManager->shouldAllowRaisingQuota = false; > > This should call a setter. OK

youenn fablet

Comment 16 2019-01-17 16:59:34 PST

Created attachment 359428 [details] Patch

EWS Watchlist

Comment 17 2019-01-17 17:02:21 PST

Attachment 359428 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/cocoa/TestWebsiteDataStoreDelegate.mm:40: The parameter name "quota" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.

Alex Christensen

Comment 18 2019-01-17 21:46:58 PST

Comment on attachment 359428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359428&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKWebsiteDataStoreDelegate.h:32 > +@protocol _WKWebsiteDataStoreDelegate <NSObject> This needs availability macros.

youenn fablet

Comment 19 2019-01-18 09:04:10 PST

Created attachment 359497 [details] Patch for landing

EWS Watchlist

Comment 20 2019-01-18 09:07:20 PST

Attachment 359497 [details] did not pass style-queue: ERROR: Tools/WebKitTestRunner/cocoa/TestWebsiteDataStoreDelegate.mm:40: The parameter name "quota" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 23 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Commit Bot

Comment 21 2019-01-18 10:19:47 PST

Comment on attachment 359497 [details] Patch for landing Clearing flags on attachment: 359497 Committed r240156: <https://trac.webkit.org/changeset/240156>

WebKit Commit Bot

Comment 22 2019-01-18 10:19:49 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 23 2019-01-18 10:20:36 PST

<rdar://problem/47388418>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebKit API

Assignee

youenn fablet

Reported

2019-01-10 09:39 PST

Modified

2023-12-10 10:26 PST History

CC List

6 users Show

URL

Keywords InRadar

Depends on

193296

Blocks

266189

Dependencies

tree graph