[webkit-reviews] review denied: [Bug 193323] Add a new SPI to request for cache storage quota increase : [Attachment 359400] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 17 13:05:21 PST 2019
Alex Christensen <achristensen at apple.com> has denied youenn fablet
<youennf at gmail.com>'s request for review:
Bug 193323: Add a new SPI to request for cache storage quota increase
https://bugs.webkit.org/show_bug.cgi?id=193323
Attachment 359400: Patch
https://bugs.webkit.org/attachment.cgi?id=359400&action=review
--- Comment #12 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 359400
--> https://bugs.webkit.org/attachment.cgi?id=359400
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
> + at 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
More information about the webkit-reviews
mailing list