Bug 230053

Summary: Regression (r282130): [ macOS iOS wk2 ] imported/w3c/web-platform-tests/storage/persisted.https.any.html is failing
Product: WebKit Reporter: ayumi_kojima
Component: New BugsAssignee: Sihui Liu <sihui_liu>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, darin, ehutchison, sihui_liu, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=229925
Bug Depends on:    
Bug Blocks: 229811    
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch for landing none

Description ayumi_kojima 2021-09-08 10:18:02 PDT
imported/w3c/web-platform-tests/storage/persisted.https.any.html 

Is failing on macOS wk2 and iOS.

History: https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fstorage%2Fpersisted.https.any.html

Diff:

--- /Volumes/Data/worker/bigsur-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/storage/persisted.https.any-expected.txt
+++ /Volumes/Data/worker/bigsur-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/storage/persisted.https.any-actual.txt
@@ -1,4 +1,4 @@
 
 PASS persisted() method exists and returns a Promise
-PASS persisted() returns a promise and resolves as boolean with false
+FAIL persisted() returns a promise and resolves as boolean with false assert_equals: expected false but got true
 
According to the history, it looks like the test started failing at https://trac.webkit.org/changeset/282130/webkit
Comment 1 Radar WebKit Bug Importer 2021-09-08 10:19:43 PDT
<rdar://problem/82879548>
Comment 2 Sihui Liu 2021-09-08 11:59:57 PDT
Created attachment 437653 [details]
Patch
Comment 3 Chris Dumez 2021-09-08 12:03:12 PDT
Comment on attachment 437653 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2424
> +    completionHandler();

This call the completion handler before the storage is actually cleared on the background thread.

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:2159
> +    auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));

Why are we using a callback aggregator for a single function call? Just pass the completion handler to clearStorage().

> Tools/ChangeLog:9
> +        Clear storage states between tests so test does not affect each other.

test does -> tests do
Comment 4 Sihui Liu 2021-09-08 12:21:40 PDT
Created attachment 437654 [details]
Patch
Comment 5 Sihui Liu 2021-09-08 12:30:45 PDT
Created attachment 437655 [details]
Patch
Comment 6 Sihui Liu 2021-09-08 12:44:19 PDT
Comment on attachment 437655 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +        Coverd by existing tests.

Typo: Covered
Comment 7 Sihui Liu 2021-09-08 13:24:38 PDT
Created attachment 437661 [details]
Patch for landing
Comment 8 EWS 2021-09-08 14:09:52 PDT
Committed r282170 (241462@main): <https://commits.webkit.org/241462@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437661 [details].
Comment 9 Eric Hutchison 2021-09-08 15:33:21 PDT
*** Bug 230068 has been marked as a duplicate of this bug. ***