<!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>[195479] 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/195479">195479</a></dd>
<dt>Author</dt> <dd>timothy_horton@apple.com</dd>
<dt>Date</dt> <dd>2016-01-22 14:25:56 -0800 (Fri, 22 Jan 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Reproducible "Unhanded web process message 'WebUserContentController:AddUserScripts'" and friends
https://bugs.webkit.org/show_bug.cgi?id=153193
<rdar://problem/24222034>
Reviewed by Darin Adler.
The WebPageProxy constructor assumes that if its WebProcess is already running,
it can add itself to the existing WebUserContentController(Proxy) and all will be well.
However, if the API client constructs a different WKUserContentController for two views,
and forces them both into the same process, WebPageProxy's constructor sends a message
with a WebUserContentController ID that doesn't exist yet on the WebProcess side
(because createWebPage, which usually brings it up, hasn't happened yet), and we
drop the message on the floor (and get the aforementioned logging).
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy):
(WebKit::WebPageProxy::initializeWebPageAfterProcessLaunch):
Instead of connecting our WebUserContentControllerProxy and WebVisitedLinkStoreProxy
to our WebProcessProxy in the constructor, when doing so might send messages
to the WebProcess that arrive before their corresponding WebProcess objects have
been created, do this in initializeWebPageAfterProcessLaunch, which always runs
during-or-after inititalizeWebPage (and always after the CreateWebPage message goes out).
(WebKit::WebPageProxy::initializeWebPage):
Mark us as needing initializeWebPageAfterProcessLaunch run, and run it right now
if the WebProcess is already up.
(WebKit::WebPageProxy::processDidFinishLaunching):
If the WebProcess wasn't up at the end of initializeWebPage, we'll eventually end up here
after it is launched, and will initializeWebPageAfterProcessLaunch.
(WebKit::WebPageProxy::resetStateAfterProcessExited):
If the WebProcess dies, we don't want to initializeWebPageAfterProcessLaunch
until after initializeWebPage runs again, so make sure the flag isn't set.
* UIProcess/WebPageProxy.h:
* TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm:
(webViewForScriptMessageHandlerMultipleHandlerRemovalTest):
(TEST):
Add a test that exhibits the problems we're fixing here.
Before, it would both log and assert in debug, and crash in release.
Now it runs happily to completion.</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="#trunkSourceWebKit2UIProcessWebPageProxyh">trunk/Source/WebKit2/UIProcess/WebPageProxy.h</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKit2CocoaUserContentControllermm">trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (195478 => 195479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2016-01-22 22:25:02 UTC (rev 195478)
+++ trunk/Source/WebKit2/ChangeLog 2016-01-22 22:25:56 UTC (rev 195479)
</span><span class="lines">@@ -1,3 +1,42 @@
</span><ins>+2016-01-22 Tim Horton <timothy_horton@apple.com>
+
+ Reproducible "Unhanded web process message 'WebUserContentController:AddUserScripts'" and friends
+ https://bugs.webkit.org/show_bug.cgi?id=153193
+ <rdar://problem/24222034>
+
+ Reviewed by Darin Adler.
+
+ The WebPageProxy constructor assumes that if its WebProcess is already running,
+ it can add itself to the existing WebUserContentController(Proxy) and all will be well.
+
+ However, if the API client constructs a different WKUserContentController for two views,
+ and forces them both into the same process, WebPageProxy's constructor sends a message
+ with a WebUserContentController ID that doesn't exist yet on the WebProcess side
+ (because createWebPage, which usually brings it up, hasn't happened yet), and we
+ drop the message on the floor (and get the aforementioned logging).
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::WebPageProxy):
+ (WebKit::WebPageProxy::initializeWebPageAfterProcessLaunch):
+ Instead of connecting our WebUserContentControllerProxy and WebVisitedLinkStoreProxy
+ to our WebProcessProxy in the constructor, when doing so might send messages
+ to the WebProcess that arrive before their corresponding WebProcess objects have
+ been created, do this in initializeWebPageAfterProcessLaunch, which always runs
+ during-or-after inititalizeWebPage (and always after the CreateWebPage message goes out).
+
+ (WebKit::WebPageProxy::initializeWebPage):
+ Mark us as needing initializeWebPageAfterProcessLaunch run, and run it right now
+ if the WebProcess is already up.
+ (WebKit::WebPageProxy::processDidFinishLaunching):
+ If the WebProcess wasn't up at the end of initializeWebPage, we'll eventually end up here
+ after it is launched, and will initializeWebPageAfterProcessLaunch.
+
+ (WebKit::WebPageProxy::resetStateAfterProcessExited):
+ If the WebProcess dies, we don't want to initializeWebPageAfterProcessLaunch
+ until after initializeWebPage runs again, so make sure the flag isn't set.
+
+ * UIProcess/WebPageProxy.h:
+
</ins><span class="cx"> 2016-01-22 Darin Adler <darin@apple.com>
</span><span class="cx">
</span><span class="cx"> Reduce use of equalIgnoringCase to just ignore ASCII case
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessWebPageProxycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (195478 => 195479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2016-01-22 22:25:02 UTC (rev 195478)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2016-01-22 22:25:56 UTC (rev 195479)
</span><span class="lines">@@ -443,12 +443,6 @@
</span><span class="cx"> m_webProcessLifetimeTracker.addObserver(m_visitedLinkStore);
</span><span class="cx"> m_webProcessLifetimeTracker.addObserver(m_websiteDataStore);
</span><span class="cx">
</span><del>- if (m_process->state() == WebProcessProxy::State::Running) {
- if (m_userContentController)
- m_process->addWebUserContentControllerProxy(*m_userContentController);
- m_process->addVisitedLinkStore(m_visitedLinkStore);
- }
-
</del><span class="cx"> updateViewState();
</span><span class="cx"> updateActivityToken();
</span><span class="cx"> updateProccessSuppressionState();
</span><span class="lines">@@ -798,8 +792,25 @@
</span><span class="cx"> #if PLATFORM(COCOA)
</span><span class="cx"> send(Messages::WebPage::SetSmartInsertDeleteEnabled(m_isSmartInsertDeleteEnabled));
</span><span class="cx"> #endif
</span><ins>+
+ m_needsToFinishInitializingWebPageAfterProcessLaunch = true;
+ finishInitializingWebPageAfterProcessLaunch();
</ins><span class="cx"> }
</span><span class="cx">
</span><ins>+void WebPageProxy::finishInitializingWebPageAfterProcessLaunch()
+{
+ if (!m_needsToFinishInitializingWebPageAfterProcessLaunch)
+ return;
+ if (m_process->state() != WebProcessProxy::State::Running)
+ return;
+
+ m_needsToFinishInitializingWebPageAfterProcessLaunch = false;
+
+ if (m_userContentController)
+ m_process->addWebUserContentControllerProxy(*m_userContentController);
+ m_process->addVisitedLinkStore(m_visitedLinkStore);
+}
+
</ins><span class="cx"> void WebPageProxy::close()
</span><span class="cx"> {
</span><span class="cx"> if (m_isClosed)
</span><span class="lines">@@ -3615,10 +3626,7 @@
</span><span class="cx"> void WebPageProxy::processDidFinishLaunching()
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_process->state() == WebProcessProxy::State::Running);
</span><del>-
- if (m_userContentController)
- m_process->addWebUserContentControllerProxy(*m_userContentController);
- m_process->addVisitedLinkStore(m_visitedLinkStore);
</del><ins>+ finishInitializingWebPageAfterProcessLaunch();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if ENABLE(NETSCAPE_PLUGIN_API)
</span><span class="lines">@@ -5064,6 +5072,8 @@
</span><span class="cx"> m_isValid = false;
</span><span class="cx"> m_isPageSuspended = false;
</span><span class="cx">
</span><ins>+ m_needsToFinishInitializingWebPageAfterProcessLaunch = false;
+
</ins><span class="cx"> m_editorState = EditorState();
</span><span class="cx">
</span><span class="cx"> m_pageClient.processDidExit();
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessWebPageProxyh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (195478 => 195479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2016-01-22 22:25:02 UTC (rev 195478)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2016-01-22 22:25:56 UTC (rev 195479)
</span><span class="lines">@@ -1463,6 +1463,8 @@
</span><span class="cx">
</span><span class="cx"> void handleAutoFillButtonClick(const UserData&);
</span><span class="cx">
</span><ins>+ void finishInitializingWebPageAfterProcessLaunch();
+
</ins><span class="cx"> void handleMessage(IPC::Connection&, const String& messageName, const UserData& messageBody);
</span><span class="cx"> void handleSynchronousMessage(IPC::Connection&, const String& messageName, const UserData& messageBody, UserData& returnUserData);
</span><span class="cx">
</span><span class="lines">@@ -1613,6 +1615,8 @@
</span><span class="cx"> // Whether it can run modal child web pages.
</span><span class="cx"> bool m_canRunModal;
</span><span class="cx">
</span><ins>+ bool m_needsToFinishInitializingWebPageAfterProcessLaunch { false };
+
</ins><span class="cx"> bool m_isInPrintingMode;
</span><span class="cx"> bool m_isPerformingDOMPrintOperation;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (195478 => 195479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2016-01-22 22:25:02 UTC (rev 195478)
+++ trunk/Tools/ChangeLog 2016-01-22 22:25:56 UTC (rev 195479)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2016-01-22 Tim Horton <timothy_horton@apple.com>
+
+ Reproducible "Unhanded web process message 'WebUserContentController:AddUserScripts'" and friends
+ https://bugs.webkit.org/show_bug.cgi?id=153193
+ <rdar://problem/24222034>
+
+ Reviewed by Darin Adler.
+
+ * TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm:
+ (webViewForScriptMessageHandlerMultipleHandlerRemovalTest):
+ (TEST):
+ Add a test that exhibits the problems we're fixing here.
+ Before, it would both log and assert in debug, and crash in release.
+ Now it runs happily to completion.
+
</ins><span class="cx"> 2016-01-22 Enrica Casucci <enrica@apple.com>
</span><span class="cx">
</span><span class="cx"> Add support for testing data detection.
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKit2CocoaUserContentControllermm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm (195478 => 195479)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm 2016-01-22 22:25:02 UTC (rev 195478)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm 2016-01-22 22:25:56 UTC (rev 195479)
</span><span class="lines">@@ -28,7 +28,9 @@
</span><span class="cx"> #import "PlatformUtilities.h"
</span><span class="cx"> #import "Test.h"
</span><span class="cx"> #import <WebKit/WebKit.h>
</span><ins>+#import <WebKit/WKProcessPoolPrivate.h>
</ins><span class="cx"> #import <WebKit/WKUserContentControllerPrivate.h>
</span><ins>+#import <WebKit/_WKProcessPoolConfiguration.h>
</ins><span class="cx"> #import <WebKit/_WKUserStyleSheet.h>
</span><span class="cx"> #import <wtf/RetainPtr.h>
</span><span class="cx">
</span><span class="lines">@@ -174,6 +176,50 @@
</span><span class="cx"> EXPECT_WK_STREQ(@"PASS", (NSString *)[lastScriptMessage body]);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+static RetainPtr<WKWebView> webViewForScriptMessageHandlerMultipleHandlerRemovalTest(WKWebViewConfiguration *configuration)
+{
+ RetainPtr<ScriptMessageHandler> handler = adoptNS([[ScriptMessageHandler alloc] init]);
+ RetainPtr<WKWebViewConfiguration> configurationCopy = adoptNS([configuration copy]);
+ [configurationCopy setUserContentController:[[[WKUserContentController alloc] init] autorelease]];
+ [[configurationCopy userContentController] addScriptMessageHandler:handler.get() name:@"handlerToRemove"];
+ [[configurationCopy userContentController] addScriptMessageHandler:handler.get() name:@"handlerToPost"];
+ RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configurationCopy.get()]);
+ RetainPtr<SimpleNavigationDelegate> delegate = adoptNS([[SimpleNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+ NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+ [webView loadRequest:request];
+ TestWebKitAPI::Util::run(&isDoneWithNavigation);
+ isDoneWithNavigation = false;
+
+ return webView;
+}
+
+TEST(WKUserContentController, ScriptMessageHandlerMultipleHandlerRemoval)
+{
+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ RetainPtr<_WKProcessPoolConfiguration> processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ [processPoolConfiguration setMaximumProcessCount:1];
+ [configuration setProcessPool:[[[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()] autorelease]];
+
+ RetainPtr<WKWebView> webView = webViewForScriptMessageHandlerMultipleHandlerRemovalTest(configuration.get());
+ RetainPtr<WKWebView> webView2 = webViewForScriptMessageHandlerMultipleHandlerRemovalTest(configuration.get());
+
+ [[[webView configuration] userContentController] removeScriptMessageHandlerForName:@"handlerToRemove"];
+ [[[webView2 configuration] userContentController] removeScriptMessageHandlerForName:@"handlerToRemove"];
+
+ [webView evaluateJavaScript:
+ @"try {"
+ " handlerToRemove.postMessage('FAIL');"
+ "} catch (e) {"
+ " window.webkit.messageHandlers.handlerToPost.postMessage('PASS');"
+ "}" completionHandler:nil];
+
+ TestWebKitAPI::Util::run(&receivedScriptMessage);
+ receivedScriptMessage = false;
+
+ EXPECT_WK_STREQ(@"PASS", (NSString *)[lastScriptMessage body]);
+}
+
</ins><span class="cx"> #if !PLATFORM(IOS) // FIXME: hangs in the iOS simulator
</span><span class="cx"> TEST(WKUserContentController, ScriptMessageHandlerWithNavigation)
</span><span class="cx"> {
</span></span></pre>
</div>
</div>
</body>
</html>