Changeset 248047 in webkit

Timestamp:

Jul 31, 2019 10:37:51 AM (5 years ago)

Author:

Chris Dumez

Message:

REGRESSION (r247486?): Flaky API Test TestWebKitAPI.WKWebView.LocalStorageProcessSuspends
​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):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcess.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcess.h (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcess.messages.in (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (modified) (1 diff)
• Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp

@@ -2111 +2111 @@
 }
 
+void NetworkProcess::processWillSuspendImminentlyForTestingSync(CompletionHandler<void()>&& completionHandler)
+{
+    processWillSuspendImminently();
+    completionHandler();
+}
+
 void NetworkProcess::prepareToSuspend()
 {

trunk/Source/WebKit/NetworkProcess/NetworkProcess.h

@@ -182 +182 @@
 
     void processWillSuspendImminently();
+    void processWillSuspendImminentlyForTestingSync(CompletionHandler<void()>&&);
     void prepareToSuspend();
     void cancelPrepareToSuspend();

trunk/Source/WebKit/NetworkProcess/NetworkProcess.messages.in

@@ -80 +80 @@
 
     ProcessWillSuspendImminently()
+    ProcessWillSuspendImminentlyForTestingSync() -> () Synchronous
     PrepareToSuspend()
     CancelPrepareToSuspend()

trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm

@@ -440 +440 @@
 - (void)_sendNetworkProcessWillSuspendImminently
 {
-    _processPool->sendNetworkProcessWillSuspendImminently();
+    _processPool->sendNetworkProcessWillSuspendImminentlyForTesting();
 }
 

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

@@ -982 +982 @@
         send(Messages::NetworkProcess::ProcessWillSuspendImminently(), 0);
 }
+
+void NetworkProcessProxy::sendProcessWillSuspendImminentlyForTesting()
+{
+    if (canSendMessage())
+        sendSync(Messages::NetworkProcess::ProcessWillSuspendImminentlyForTestingSync(), Messages::NetworkProcess::ProcessWillSuspendImminentlyForTestingSync::Reply(), 0);
+}
     
 void NetworkProcessProxy::sendPrepareToSuspend()

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h

@@ -186 +186 @@
     void sendProcessWillSuspendImminently() final;
     void sendProcessDidResume() final;
+    
+    void sendProcessWillSuspendImminentlyForTesting();
 
 private:

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -1777 +1777 @@
 }
 
-void WebProcessPool::sendNetworkProcessWillSuspendImminently()
+void WebProcessPool::sendNetworkProcessWillSuspendImminentlyForTesting()
 {
     if (m_networkProcess)
-        m_networkProcess->sendProcessWillSuspendImminently();
+        m_networkProcess->sendProcessWillSuspendImminentlyForTesting();
 }
 

trunk/Source/WebKit/UIProcess/WebProcessPool.h

@@ -313 +313 @@
     void clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace&&, CompletionHandler<void()>&&);
     void terminateNetworkProcess();
-    void sendNetworkProcessWillSuspendImminently();
+    void sendNetworkProcessWillSuspendImminentlyForTesting();
     void sendNetworkProcessDidResume();
     void terminateServiceWorkerProcesses();

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/LocalStoragePersistence.mm

@@ -113 +113 @@
 }
 
-TEST(WKWebView, DISABLED_LocalStorageProcessSuspends)
+TEST(WKWebView, LocalStorageProcessSuspends)
 {
     readyToContinue = false;