REGRESSION (r247486?): Flaky API Test TestWebKitAPI.WKWebView.LocalStorageProcessSuspends
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Jul 2019 17:37:51 +0000 (17:37 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Jul 2019 17:37:51 +0000 (17:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200086
<rdar://problem/53501721>

Reviewed by Alex Christensen.

Source/WebKit:

The test would first send a ProcessWillSuspendImminently IPC to the NetworkProcess and then
run JS in the WebContent process, which would in turn send IPC to the NetworkProcess. The
test was flaky because it expected the network process to receive the IPC from the UIProcess
*before* the one from the WebContent process. However, there is no guarantee about ordering
from IPC messages coming from different connections.

To address the flakiness, this patch introduces a new ProcessWillSuspendImminentlyForTesting
synchronous IPC and uses this instead. As a result, it is now guaranteed that the network
process processes this IPC *before* receiving any IPC from the WebContent process that is
the result of IPC from the UIProcess.

* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::processWillSuspendImminentlyForTestingSync):
* NetworkProcess/NetworkProcess.h:
* NetworkProcess/NetworkProcess.messages.in:
* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _sendNetworkProcessWillSuspendImminently]):
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::sendProcessWillSuspendImminentlyForTesting):
* UIProcess/Network/NetworkProcessProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::sendNetworkProcessWillSuspendImminentlyForTesting):
(WebKit::WebProcessPool::sendNetworkProcessWillSuspendImminently): Deleted.
* UIProcess/WebProcessPool.h:

Tools:

re-enable the API test.

* TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm:
(TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248047 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkProcess.cpp
Source/WebKit/NetworkProcess/NetworkProcess.h
Source/WebKit/NetworkProcess/NetworkProcess.messages.in
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm

index 4c2c4da..75b3814 100644 (file)
@@ -1,3 +1,36 @@
+2019-07-31  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r247486?): Flaky API Test TestWebKitAPI.WKWebView.LocalStorageProcessSuspends
+        https://bugs.webkit.org/show_bug.cgi?id=200086
+        <rdar://problem/53501721>
+
+        Reviewed by Alex Christensen.
+
+        The test would first send a ProcessWillSuspendImminently IPC to the NetworkProcess and then
+        run JS in the WebContent process, which would in turn send IPC to the NetworkProcess. The
+        test was flaky because it expected the network process to receive the IPC from the UIProcess
+        *before* the one from the WebContent process. However, there is no guarantee about ordering
+        from IPC messages coming from different connections.
+
+        To address the flakiness, this patch introduces a new ProcessWillSuspendImminentlyForTesting
+        synchronous IPC and uses this instead. As a result, it is now guaranteed that the network
+        process processes this IPC *before* receiving any IPC from the WebContent process that is
+        the result of IPC from the UIProcess.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::processWillSuspendImminentlyForTestingSync):
+        * NetworkProcess/NetworkProcess.h:
+        * NetworkProcess/NetworkProcess.messages.in:
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _sendNetworkProcessWillSuspendImminently]):
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::sendProcessWillSuspendImminentlyForTesting):
+        * UIProcess/Network/NetworkProcessProxy.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::sendNetworkProcessWillSuspendImminentlyForTesting):
+        (WebKit::WebProcessPool::sendNetworkProcessWillSuspendImminently): Deleted.
+        * UIProcess/WebProcessPool.h:
+
 2019-07-31  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS 13] Safari crashes when closing a tab with a focused element if the unified field has focus
index 08033d6..4a6c98d 100644 (file)
@@ -2110,6 +2110,12 @@ void NetworkProcess::processWillSuspendImminently()
     RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::processWillSuspendImminently() END", this);
 }
 
+void NetworkProcess::processWillSuspendImminentlyForTestingSync(CompletionHandler<void()>&& completionHandler)
+{
+    processWillSuspendImminently();
+    completionHandler();
+}
+
 void NetworkProcess::prepareToSuspend()
 {
     RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::prepareToSuspend()", this);
index 2866857..d7cb7c6 100644 (file)
@@ -181,6 +181,7 @@ public:
     bool canHandleHTTPSServerTrustEvaluation() const { return m_canHandleHTTPSServerTrustEvaluation; }
 
     void processWillSuspendImminently();
+    void processWillSuspendImminentlyForTestingSync(CompletionHandler<void()>&&);
     void prepareToSuspend();
     void cancelPrepareToSuspend();
     void processDidResume();
index 9be16d8..f4de408 100644 (file)
@@ -79,6 +79,7 @@ messages -> NetworkProcess LegacyReceiver {
     ProcessDidTransitionToForeground()
 
     ProcessWillSuspendImminently()
+    ProcessWillSuspendImminentlyForTestingSync() -> () Synchronous
     PrepareToSuspend()
     CancelPrepareToSuspend()
     ProcessDidResume()
index 72aef9c..9b5f7b5 100644 (file)
@@ -439,7 +439,7 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
 
 - (void)_sendNetworkProcessWillSuspendImminently
 {
-    _processPool->sendNetworkProcessWillSuspendImminently();
+    _processPool->sendNetworkProcessWillSuspendImminentlyForTesting();
 }
 
 - (void)_sendNetworkProcessDidResume
index 6d666c9..0d3d03c 100644 (file)
@@ -981,6 +981,12 @@ void NetworkProcessProxy::sendProcessWillSuspendImminently()
     if (canSendMessage())
         send(Messages::NetworkProcess::ProcessWillSuspendImminently(), 0);
 }
+
+void NetworkProcessProxy::sendProcessWillSuspendImminentlyForTesting()
+{
+    if (canSendMessage())
+        sendSync(Messages::NetworkProcess::ProcessWillSuspendImminentlyForTestingSync(), Messages::NetworkProcess::ProcessWillSuspendImminentlyForTestingSync::Reply(), 0);
+}
     
 void NetworkProcessProxy::sendPrepareToSuspend()
 {
index 2d0be51..35b8213 100644 (file)
@@ -185,6 +185,8 @@ public:
     // ProcessThrottlerClient
     void sendProcessWillSuspendImminently() final;
     void sendProcessDidResume() final;
+    
+    void sendProcessWillSuspendImminentlyForTesting();
 
 private:
     // AuxiliaryProcessProxy
index 7cb4f11..afab3fd 100644 (file)
@@ -1776,10 +1776,10 @@ void WebProcessPool::terminateNetworkProcess()
     m_didNetworkProcessCrash = true;
 }
 
-void WebProcessPool::sendNetworkProcessWillSuspendImminently()
+void WebProcessPool::sendNetworkProcessWillSuspendImminentlyForTesting()
 {
     if (m_networkProcess)
-        m_networkProcess->sendProcessWillSuspendImminently();
+        m_networkProcess->sendProcessWillSuspendImminentlyForTesting();
 }
 
 void WebProcessPool::sendNetworkProcessDidResume()
index dae2591..226cf36 100644 (file)
@@ -312,7 +312,7 @@ public:
     void clearCachedCredentials();
     void clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace&&, CompletionHandler<void()>&&);
     void terminateNetworkProcess();
-    void sendNetworkProcessWillSuspendImminently();
+    void sendNetworkProcessWillSuspendImminentlyForTesting();
     void sendNetworkProcessDidResume();
     void terminateServiceWorkerProcesses();
     void disableServiceWorkerProcessTerminationDelay();
index 4be756c..a90091f 100644 (file)
@@ -1,3 +1,16 @@
+2019-07-31  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r247486?): Flaky API Test TestWebKitAPI.WKWebView.LocalStorageProcessSuspends
+        https://bugs.webkit.org/show_bug.cgi?id=200086
+        <rdar://problem/53501721>
+
+        Reviewed by Alex Christensen.
+
+        re-enable the API test.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm:
+        (TEST):
+
 2019-07-31  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS 13] Safari crashes when closing a tab with a focused element if the unified field has focus
index 2b2bf74..11562bb 100644 (file)
@@ -112,7 +112,7 @@ TEST(WKWebView, LocalStorageProcessCrashes)
     TestWebKitAPI::Util::run(&readyToContinue);
 }
 
-TEST(WKWebView, DISABLED_LocalStorageProcessSuspends)
+TEST(WKWebView, LocalStorageProcessSuspends)
 {
     readyToContinue = false;
     [[WKWebsiteDataStore defaultDataStore] removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^() {