Changeset 204243 in webkit

Timestamp:

Aug 7, 2016 12:08:01 PM (8 years ago)

Author:

Chris Dumez

Message:

Write API test to cover crash fix in r204135
​https://bugs.webkit.org/show_bug.cgi?id=160587

Reviewed by Darin Adler.

Source/WebKit2:

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::terminateProcess):
Stop calling resetStateAfterProcessExited() after calling
requestTermination() because requestTermination() now calls
didClose() which calls processDidCrash() which already calls
resetStateAfterProcessExited(). Because the processDidCrash()
delegates may start new loads, we really do not want to
reset the state again after calling the delegates.

• UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::requestTermination):

• Call didClose() in WebProcessProxy::requestTermination() so that the processDidCrash() delegates get called in API tests whenever a WebContent process is terminated to simulate a crash.
• Stop calling shutDown() and webConnection()->didClose() because didClose() already does this for us.

Tools:

Add API test to cover crash fix in r204135. This reproduces the crash
by destroying a related WKWebView in the webViewWebContentProcessDidTerminate
callback.

• TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm:

(-[NavigationDelegate webViewWebContentProcessDidTerminate:]):
(TEST):

Location:

trunk

Files:

• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Source/WebKit2/UIProcess/WebProcessProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm (modified) (4 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-08-07  Chris Dumez  <cdumez@apple.com>
+
+        Write API test to cover crash fix in r204135
+        https://bugs.webkit.org/show_bug.cgi?id=160587
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::terminateProcess):
+        Stop calling resetStateAfterProcessExited() after calling
+        requestTermination() because requestTermination() now calls
+        didClose() which calls processDidCrash() which already calls
+        resetStateAfterProcessExited(). Because the processDidCrash()
+        delegates may start new loads, we really do not want to
+        reset the state again after calling the delegates.
+
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::requestTermination):
+        - Call didClose() in WebProcessProxy::requestTermination() so that
+          the processDidCrash() delegates get called in API tests whenever
+          a WebContent process is terminated to simulate a crash.
+        - Stop calling shutDown() and webConnection()->didClose() because
+          didClose() already does this for us.
+
 2016-08-06  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

@@ -2354 +2354 @@
 
     m_process->requestTermination();
-    resetStateAfterProcessExited();
 }
 

trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp

@@ -805 +805 @@
     ChildProcessProxy::terminate();
 
-    if (webConnection())
-        webConnection()->didClose();
-
-    shutDown();
+    didClose(*connection());
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2016-08-07  Chris Dumez  <cdumez@apple.com>
+
+        Write API test to cover crash fix in r204135
+        https://bugs.webkit.org/show_bug.cgi?id=160587
+
+        Reviewed by Darin Adler.
+
+        Add API test to cover crash fix in r204135. This reproduces the crash
+        by destroying a related WKWebView in the webViewWebContentProcessDidTerminate
+        callback.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm:
+        (-[NavigationDelegate webViewWebContentProcessDidTerminate:]):
+        (TEST):
+
 2016-08-06  Chris Dumez  <cdumez@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm

@@ -28 +28 @@
 #import <WebKit/WKNavigationPrivate.h>
 #import <WebKit/WKNavigationDelegate.h>
+#import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WKWebView.h>
+#import <WebKit/WKWebViewConfigurationPrivate.h>
+#import <WebKit/WKWebViewPrivate.h>
+#import <WebKit/_WKProcessPoolConfiguration.h>
 #import <wtf/RetainPtr.h>
 #import "PlatformUtilities.h"
@@ -36 +40 @@
 
 static bool isDone;
+static std::function<void()> crashHandler;
 static RetainPtr<WKNavigation> currentNavigation;
 
@@ -59 +64 @@
 
     isDone = true;
+}
+
+- (void)webViewWebContentProcessDidTerminate:(WKWebView *)webView
+{
+    crashHandler();
 }
 
@@ -188 +198 @@
 }
 
+TEST(WKNavigation, WebContentProcessDidTerminate)
+{
+    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
+    RetainPtr<_WKProcessPoolConfiguration> poolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    poolConfiguration.get().maximumProcessCount = 1;
+    RetainPtr<WKProcessPool> processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:poolConfiguration.get()]);
+
+    RetainPtr<WKWebViewConfiguration> configuration1 = adoptNS([[WKWebViewConfiguration alloc] init]);
+    configuration1.get().processPool = processPool.get();
+    RetainPtr<WKWebView> webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration1.get()]);
+
+    RetainPtr<NavigationDelegate> delegate1 = adoptNS([[NavigationDelegate alloc] init]);
+    [webView1 setNavigationDelegate:delegate1.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"data:text/html,1"]];
+
+    isDone = false;
+    currentNavigation = [webView1 loadRequest:request];
+    TestWebKitAPI::Util::run(&isDone);
+
+    RetainPtr<WKWebViewConfiguration> configuration2 = adoptNS([[WKWebViewConfiguration alloc] init]);
+    configuration2.get().processPool = processPool.get();
+    configuration2.get()._relatedWebView = webView1.get();
+    RetainPtr<WKWebView> webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration2.get()]);
+
+    RetainPtr<NavigationDelegate> delegate2 = adoptNS([[NavigationDelegate alloc] init]);
+    [webView2 setNavigationDelegate:delegate2.get()];
+
+    isDone = false;
+    currentNavigation = [webView2 loadRequest:request];
+    TestWebKitAPI::Util::run(&isDone);
+
+    bool didTerminate = false;
+    crashHandler = [&] {
+        webView1 = nullptr;
+        webView2 = nullptr;
+        [pool drain]; // Make sure the views get deallocated.
+        didTerminate = true;
+    };
+
+    [webView2 _killWebContentProcessAndResetState];
+    TestWebKitAPI::Util::run(&didTerminate);
+}
+
 #endif