<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[204243] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/204243">204243</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-08-07 12:08:01 -0700 (Sun, 07 Aug 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Write API test to cover crash fix in <a href="http://trac.webkit.org/projects/webkit/changeset/204135">r204135</a>
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 <a href="http://trac.webkit.org/projects/webkit/changeset/204135">r204135</a>. This reproduces the crash
by destroying a related WKWebView in the webViewWebContentProcessDidTerminate
callback.
* TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm:
(-[NavigationDelegate webViewWebContentProcessDidTerminate:]):
(TEST):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2UIProcessWebPageProxycpp">trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp</a></li>
<li><a href="#trunkSourceWebKit2UIProcessWebProcessProxycpp">trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKit2CocoaNavigationmm">trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (204242 => 204243)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2016-08-07 17:14:54 UTC (rev 204242)
+++ trunk/Source/WebKit2/ChangeLog 2016-08-07 19:08:01 UTC (rev 204243)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+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.
+
</ins><span class="cx"> 2016-08-06 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r204226.
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessWebPageProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (204242 => 204243)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2016-08-07 17:14:54 UTC (rev 204242)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2016-08-07 19:08:01 UTC (rev 204243)
</span><span class="lines">@@ -2353,7 +2353,6 @@
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> m_process->requestTermination();
</span><del>- resetStateAfterProcessExited();
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> SessionState WebPageProxy::sessionState(const std::function<bool (WebBackForwardListItem&)>& filter) const
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessWebProcessProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (204242 => 204243)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2016-08-07 17:14:54 UTC (rev 204242)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2016-08-07 19:08:01 UTC (rev 204243)
</span><span class="lines">@@ -804,10 +804,7 @@
</span><span class="cx">
</span><span class="cx"> ChildProcessProxy::terminate();
</span><span class="cx">
</span><del>- if (webConnection())
- webConnection()->didClose();
-
- shutDown();
</del><ins>+ didClose(*connection());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void WebProcessProxy::enableSuddenTermination()
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (204242 => 204243)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2016-08-07 17:14:54 UTC (rev 204242)
+++ trunk/Tools/ChangeLog 2016-08-07 19:08:01 UTC (rev 204243)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+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):
+
</ins><span class="cx"> 2016-08-06 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r204226.
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKit2CocoaNavigationmm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm (204242 => 204243)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm 2016-08-07 17:14:54 UTC (rev 204242)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/Navigation.mm 2016-08-07 19:08:01 UTC (rev 204243)
</span><span class="lines">@@ -27,7 +27,11 @@
</span><span class="cx">
</span><span class="cx"> #import <WebKit/WKNavigationPrivate.h>
</span><span class="cx"> #import <WebKit/WKNavigationDelegate.h>
</span><ins>+#import <WebKit/WKProcessPoolPrivate.h>
</ins><span class="cx"> #import <WebKit/WKWebView.h>
</span><ins>+#import <WebKit/WKWebViewConfigurationPrivate.h>
+#import <WebKit/WKWebViewPrivate.h>
+#import <WebKit/_WKProcessPoolConfiguration.h>
</ins><span class="cx"> #import <wtf/RetainPtr.h>
</span><span class="cx"> #import "PlatformUtilities.h"
</span><span class="cx"> #import "Test.h"
</span><span class="lines">@@ -35,6 +39,7 @@
</span><span class="cx"> #if WK_API_ENABLED
</span><span class="cx">
</span><span class="cx"> static bool isDone;
</span><ins>+static std::function<void()> crashHandler;
</ins><span class="cx"> static RetainPtr<WKNavigation> currentNavigation;
</span><span class="cx">
</span><span class="cx"> @interface NavigationDelegate : NSObject <WKNavigationDelegate>
</span><span class="lines">@@ -60,6 +65,11 @@
</span><span class="cx"> isDone = true;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+- (void)webViewWebContentProcessDidTerminate:(WKWebView *)webView
+{
+ crashHandler();
+}
+
</ins><span class="cx"> @end
</span><span class="cx">
</span><span class="cx"> TEST(WKNavigation, NavigationDelegate)
</span><span class="lines">@@ -187,4 +197,48 @@
</span><span class="cx"> ASSERT_TRUE([delegate decidedPolicyForBackForwardNavigation]);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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);
+}
+
</ins><span class="cx"> #endif
</span></span></pre>
</div>
</div>
</body>
</html>