Bug 196991 – -[WKWebsiteDataStore fetchDataRecordsOfTypes:completionHandler:] never returns _WKWebsiteDataTypeCredentials

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Per Arne Vollan]

-[WKWebsiteDataStore fetchDataRecordsOfTypes:completionHandler:] never returns WKWebsiteDataRecords with dataTypes that include _WKWebsiteDataTypeCredentials.

[Per Arne Vollan]

rdar://problem/45507423

[Per Arne Vollan]

Created attachment 367591 [details]
Patch

[Brent Fulgham]

Comment on attachment 367591 [details]
Patch

r=me

[Per Arne Vollan]

Comment on attachment 367591 [details]
Patch

Thanks for reviewing!

[John Wilander]

Comment on attachment 367591 [details]
Patch

We should refactor computeNetworkProcessAccessTypeForDataFetch() to make use of WebsiteData::ownerProcess(dataType) which I added in https://trac.webkit.org/changeset/243632/webkit. See computeNetworkProcessAccessTypeForDataRemoval() in WebKit::WebsiteDataStore.cpp for an example.

[WebKit Commit Bot]

Comment on attachment 367591 [details]
Patch

Clearing flags on attachment: 367591

Committed r244365: <https://trac.webkit.org/changeset/244365>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Per Arne Vollan]

(In reply to John Wilander from comment #5)
> Comment on attachment 367591 [details]
> Patch
> 
> We should refactor computeNetworkProcessAccessTypeForDataFetch() to make use
> of WebsiteData::ownerProcess(dataType) which I added in
> https://trac.webkit.org/changeset/243632/webkit. See
> computeNetworkProcessAccessTypeForDataRemoval() in
> WebKit::WebsiteDataStore.cpp for an example.

I see, that would probably simplify the implementation a bit!

[Per Arne Vollan]

Reopening to attach new patch.

[Per Arne Vollan]

Created attachment 367981 [details]
Patch

[Alex Christensen]

Comment on attachment 367981 [details]
Patch

Please add a test.  See https://bugs.webkit.org/show_bug.cgi?id=197093 for an example of a test for doing things with credentials.

[Per Arne Vollan]

(In reply to Alex Christensen from comment #11)
> Comment on attachment 367981 [details]
> Patch
> 
> Please add a test.  See https://bugs.webkit.org/show_bug.cgi?id=197093 for
> an example of a test for doing things with credentials.

Thanks for reviewing! I am looking into adding a test similar to the test in https://bugs.webkit.org/show_bug.cgi?id=197093, but it seems the credential is not being stored.

[Per Arne Vollan]

Created attachment 368085 [details]
Patch

[Per Arne Vollan]

Created attachment 368176 [details]
Patch

[Brent Fulgham]

It looks like GTK/WPE are unhappy because of ISOHeapInlines.h?

[Per Arne Vollan]

Created attachment 368188 [details]
Patch

[Brent Fulgham]

 I think this looks fine, but Alex should review this.

[Per Arne Vollan]

Created attachment 368240 [details]
Patch

[Per Arne Vollan]

Thanks for reviewing, Alex!

[EWS Watchlist]

Comment on attachment 368240 [details]
Patch

Attachment 368240 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11995934

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-abort.https.html

[EWS Watchlist]

Created attachment 368251 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Per Arne Vollan]

(In reply to Build Bot from comment #20)
> Comment on attachment 368240 [details]
> Patch
> 
> Attachment 368240 [details] did not pass ios-sim-ews (ios-simulator-wk2):
> Output: https://webkit-queues.webkit.org/results/11995934
> 
> New failing tests:
> imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-
> abort.https.html

I think this failure in unrelated.

[Per Arne Vollan]

Committed r244659: <https://trac.webkit.org/changeset/244659/webkit>