Bug 180681 - Storage Access API: Make document.hasStorageAccess() retrieve current status from the network process
Summary: Storage Access API: Make document.hasStorageAccess() retrieve current status ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-11 16:37 PST by John Wilander
Modified: 2017-12-17 19:44 PST (History)
9 users (show)

See Also:

Attachments
Patch (42.21 KB, patch)
2017-12-15 14:56 PST, John Wilander 		no flags Details | Formatted Diff | Diff
Patch (46.19 KB, patch)
2017-12-15 16:17 PST, John Wilander 		no flags Details | Formatted Diff | Diff
Patch for landing (47.54 KB, patch)
2017-12-17 19:11 PST, John Wilander 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description John Wilander 2017-12-11 16:37:04 PST 
This is part 2 of making Storage Access API frame-specific.
Comment 1 Radar WebKit Bug Importer 2017-12-11 16:37:37 PST 
<rdar://problem/35982161>
Comment 2 John Wilander 2017-12-15 14:56:22 PST 
Created attachment 329522 [details]
Patch
Comment 3 John Wilander 2017-12-15 14:56:46 PST 
Style errors are all of the "Extra space before ( in function call" variety.
Comment 4 EWS Watchlist 2017-12-15 14:59:07 PST 
Attachment 329522 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1436:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:257:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:352:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/page/ChromeClient.h:470:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebPage/WebPage.cpp:5780:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h:150:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h:86:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebPage/WebPage.h:1027:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:1259:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 9 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Alex Christensen 2017-12-15 15:08:50 PST 
Comment on attachment 329522 [details]
Patch

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

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:268
> +});

indentation

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:81
> +    void hasStorageAccessForPrevalentDomains(PAL::SessionID, const String& resourceDomain, const String& firstPartyDomain, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void(bool)>&& callback);

WTF:: prefix should be unnecessary
Comment 6 Alex Christensen 2017-12-15 15:12:10 PST 
Comment on attachment 329522 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:7430
> -    RefPtr<DeferredPromise> promise(WTFMove(passedPromise));
> +    RefPtr<DeferredPromise>&& promise(WTFMove(passedPromise));

We shouldn't need this at all.  We should be able to just use a Ref and call the passedPromise promise.
Comment 7 John Wilander 2017-12-15 15:31:31 PST 
(In reply to Alex Christensen from comment #5)
> Comment on attachment 329522 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329522&action=review
> 
> > Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:268
> > +});
> 
> indentation

Fixed.

> > Source/WebKit/UIProcess/Network/NetworkProcessProxy.h:81
> > +    void hasStorageAccessForPrevalentDomains(PAL::SessionID, const String& resourceDomain, const String& firstPartyDomain, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void(bool)>&& callback);
> 
> WTF:: prefix should be unnecessary

OK.
Comment 8 John Wilander 2017-12-15 15:32:08 PST 
(In reply to Alex Christensen from comment #6)
> Comment on attachment 329522 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329522&action=review
> 
> > Source/WebCore/dom/Document.cpp:7430
> > -    RefPtr<DeferredPromise> promise(WTFMove(passedPromise));
> > +    RefPtr<DeferredPromise>&& promise(WTFMove(passedPromise));
> 
> We shouldn't need this at all.  We should be able to just use a Ref and call
> the passedPromise promise.

This was actually me trying to figure out weird promise behavior. Turned out to be something else. Fixed.
Comment 9 John Wilander 2017-12-15 16:11:37 PST 
I know what the WK2 test timeouts are about. New patch coming.
Comment 10 John Wilander 2017-12-15 16:17:08 PST 
Created attachment 329536 [details]
Patch
Comment 11 EWS Watchlist 2017-12-15 16:19:50 PST 
Attachment 329536 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1436:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:257:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:352:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/page/ChromeClient.h:470:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebPage/WebPage.cpp:5780:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h:150:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h:86:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebPage/WebPage.h:1027:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:1259:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 9 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Alex Christensen 2017-12-15 19:16:02 PST 
Comment on attachment 329536 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:7430
> +    RefPtr<DeferredPromise>&& promise(WTFMove(passedPromise));

Delete this line, change the parameter name from passedPromise to promise, and change the lambda capture from promise to promise = WTFMove(promise)
Comment 13 John Wilander 2017-12-17 19:11:45 PST 
Created attachment 329630 [details]
Patch for landing
Comment 14 John Wilander 2017-12-17 19:12:08 PST 
(In reply to Alex Christensen from comment #12)
> Comment on attachment 329536 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329536&action=review
> 
> > Source/WebCore/dom/Document.cpp:7430
> > +    RefPtr<DeferredPromise>&& promise(WTFMove(passedPromise));
> 
> Delete this line, change the parameter name from passedPromise to promise,
> and change the lambda capture from promise to promise = WTFMove(promise)

Fixed. Thanks for the review, Alex!
Comment 15 WebKit Commit Bot 2017-12-17 19:44:58 PST 
Comment on attachment 329630 [details]
Patch for landing

Clearing flags on attachment: 329630

Committed r226016: <https://trac.webkit.org/changeset/226016>
Comment 16 WebKit Commit Bot 2017-12-17 19:44:59 PST 
All reviewed patches have been landed.  Closing bug.