Bug 198450 - TestWebKitAPI.WKWebView.LocalStorageProcessSuspends is flaky
Summary: TestWebKitAPI.WKWebView.LocalStorageProcessSuspends is flaky
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-31 20:59 PDT by Sihui Liu
Modified: 2019-07-29 16:40 PDT (History)
5 users (show)

See Also:


Attachments
Patch for landing (4.00 KB, patch)
2019-05-31 21:17 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2019-05-31 20:59:43 PDT
We used two WKWebViews in this test, one for setting local storage item and the other for checking that local storage item is correctly set.
The test goes like this:
1. Set item value to be "value" in webView1
2. Suspend network process
3. Set item value to be "newValue" in webView1
4. Get item value from webView2. The value should be "value" because network process cannot notify webView2 about new value during its suspension.
5. Wake up network process
6. Get item value from webView. The value should be "newValue" as network process handles the update and makes notification.

In step 6, we relies on web page of webView2 to send back a message that has the item value. To implement this, we let that web page periodically check the item value and send back a message when it detects the value has changed or *the limit of checks has been reached*.

Because web process is not suspended, it is possible that that web page in webView2 finishes the check and sends a message earlier than step 6. Therefore, we may not get the expected message during the wait in step 6 and the test times out. Also, the message content may be incorrect if the check finishes before network process resumes.

To make the test more robust, we can use storage event instead of the periodic check.
Comment 1 Sihui Liu 2019-05-31 21:17:41 PDT
Created attachment 371107 [details]
Patch for landing
Comment 2 Sihui Liu 2019-05-31 21:19:49 PDT
This was the correct bug for WKWebView.LocalStorageProcessSuspends... I wrongly uploaded the patch to https://bugs.webkit.org/show_bug.cgi?id=198423, which is for WKWebView.LocalStorageProcessCrashes
Comment 3 WebKit Commit Bot 2019-05-31 21:33:09 PDT
Comment on attachment 371107 [details]
Patch for landing

Clearing flags on attachment: 371107

Committed r246012: <https://trac.webkit.org/changeset/246012>
Comment 4 WebKit Commit Bot 2019-05-31 21:33:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2019-05-31 21:34:16 PDT
<rdar://problem/51325191>
Comment 6 Aakash Jain 2019-07-24 10:14:18 PDT
The test still seems to be flaky, filed https://bugs.webkit.org/show_bug.cgi?id=200086

Also curious, is there a particular reason to not run the patch through EWS? This seems especially important when fixing a test, which is covered by EWS.

(The patch was uploaded to bugzilla and marked cq+ directly. Note that EWS automatically runs on patches with r? flag, and can be manually invoked by pressing 'Submit to new EWS' button)
Comment 7 Sihui Liu 2019-07-29 16:40:52 PDT
(In reply to Aakash Jain from comment #6)
> The test still seems to be flaky, filed
> https://bugs.webkit.org/show_bug.cgi?id=200086
> 
> Also curious, is there a particular reason to not run the patch through EWS?
> This seems especially important when fixing a test, which is covered by EWS.
> 
> (The patch was uploaded to bugzilla and marked cq+ directly. Note that EWS
> automatically runs on patches with r? flag, and can be manually invoked by
> pressing 'Submit to new EWS' button)

This was run on EWS. See comments above, I wrongly uploaded the patch to https://bugs.webkit.org/show_bug.cgi?id=198423 (patch at https://bugs.webkit.org/attachment.cgi?id=371087&action=review), it got reviewed there.