Bug 169776 - Update the WKHTTPCookieStore API to be simpler and add observers
Summary: Update the WKHTTPCookieStore API to be simpler and add observers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-03-16 12:49 PDT by Brady Eidson
Modified: 2017-03-16 16:30 PDT (History)
4 users (show)

See Also:


Attachments
Patch (46.05 KB, patch)
2017-03-16 12:57 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (54.23 KB, patch)
2017-03-16 15:37 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2017-03-16 12:49:43 PDT
Update the WKHTTPCookieStore API to be simpler and add observers
Comment 1 Radar WebKit Bug Importer 2017-03-16 12:50:07 PDT
<rdar://problem/31096000>
Comment 2 Brady Eidson 2017-03-16 12:57:05 PDT
Created attachment 304680 [details]
Patch
Comment 3 WebKit Commit Bot 2017-03-16 12:58:17 PDT
Attachment 304680 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKHTTPCookieStore.mm:58:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:74:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:94:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:99:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:96:  Missing space before {  [whitespace/braces] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:103:  Missing space before {  [whitespace/braces] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:139:  Missing space before {  [whitespace/braces] [5]
Total errors found: 7 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Alex Christensen 2017-03-16 15:09:05 PDT
Comment on attachment 304680 [details]
Patch

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

Fix GTK.
You might need to use _saveCookies sometimes, like before we read all cookies?

> Source/WebKit2/UIProcess/API/Cocoa/WKHTTPCookieStore.h:67
> + @discussion Your observer is not retained by the receiver. It is your responsibility

Who is the intended antecedent of "your"?

> Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h:126
> +    HashSet<RefPtr<WebProcessPool>> processPools(size_t count = std::numeric_limits<size_t>::max(), bool ensureOneExists = true) const;

One always exists.  It is the integer between zero and two.
And we don't need a process pool to exist to access the default cookie storage.
Comment 5 Brady Eidson 2017-03-16 15:20:11 PDT
(In reply to comment #4)
> Comment on attachment 304680 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304680&action=review
> 
> Fix GTK.
> You might need to use _saveCookies sometimes, like before we read all
> cookies?
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKHTTPCookieStore.h:67
> > + @discussion Your observer is not retained by the receiver. It is your responsibility
> 
> Who is the intended antecedent of "your"?
> 
> > Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h:126
> > +    HashSet<RefPtr<WebProcessPool>> processPools(size_t count = std::numeric_limits<size_t>::max(), bool ensureOneExists = true) const;
> 
> One always exists.  It is the integer between zero and two.
> And we don't need a process pool to exist to access the default cookie
> storage.

I'll come up with a better name for the bool, but the concern is not for needing a process pool for default cookie storage.
Comment 6 Brady Eidson 2017-03-16 15:37:46 PDT
Created attachment 304706 [details]
Patch
Comment 7 WebKit Commit Bot 2017-03-16 15:40:26 PDT
Attachment 304706 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:228:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKHTTPCookieStore.mm:58:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:74:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:94:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/API/APIHTTPCookieStore.cpp:99:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:141:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:96:  Missing space before {  [whitespace/braces] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:103:  Missing space before {  [whitespace/braces] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:139:  Missing space before {  [whitespace/braces] [5]
Total errors found: 9 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 WebKit Commit Bot 2017-03-16 16:30:13 PDT
Comment on attachment 304706 [details]
Patch

Clearing flags on attachment: 304706

Committed r214078: <http://trac.webkit.org/changeset/214078>
Comment 9 WebKit Commit Bot 2017-03-16 16:30:18 PDT
All reviewed patches have been landed.  Closing bug.