Diff
Modified: trunk/Source/WebKit2/ChangeLog (148311 => 148312)
--- trunk/Source/WebKit2/ChangeLog 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Source/WebKit2/ChangeLog 2013-04-12 23:29:13 UTC (rev 148312)
@@ -1,3 +1,42 @@
+2013-04-12 Adenilson Cavalcanti <cavalcan...@gmail.com>
+
+ [WK2] WebPageProxy loadURL() won't work when called just after terminateProcess()
+ https://bugs.webkit.org/show_bug.cgi?id=110743
+
+ Reviewed by Benjamin Poulain.
+
+ A call to loadURL() just after terminating WebProcess will fail thanks to
+ WebPageProxy being in an undefined state since it is in the middle of its own
+ cleanup after process termination.
+
+ To properly fix this, not only WebPageProxy cleanup should be made
+ at WebProcess termination request, but also WebProcessProxy needs
+ to only return to its caller after terminating the process and
+ closing connections. Otherwise, WebPageProxy can even be able to
+ detect that WebProcess is no longer running, but a call to respawn
+ the process will fail.
+
+ To fix these issues, this patch moves the cleanup code to a shared private function
+ that is used for both the cases i.e. user termination and real crash. WebProcess
+ shutdown is done using a new method that ensures that all cleanup was done before
+ returning.
+
+ A last change introduced in this patch is that for user requested termination,
+ clients are no longer notified of a crash (since it is not a crash).
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::terminateProcess):
+ (WebKit::WebPageProxy::processDidCrash):
+ (WebKit):
+ (WebKit::WebPageProxy::resetStateAfterProcessExited):
+ * UIProcess/WebPageProxy.h:
+ (WebPageProxy):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::userRequestedTerminate):
+ (WebKit):
+ * UIProcess/WebProcessProxy.h:
+ (WebProcessProxy):
+
2013-04-12 Gavin Barraclough <barraclo...@apple.com>
Add contentAnchor to WKView
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (148311 => 148312)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2013-04-12 23:29:13 UTC (rev 148312)
@@ -1567,7 +1567,8 @@
if (!m_isValid)
return;
- m_process->terminate();
+ m_process->requestTermination();
+ resetStateAfterProcessExited();
}
#if !USE(CF) || defined(BUILDING_QT__)
@@ -3798,8 +3799,17 @@
void WebPageProxy::processDidCrash()
{
+ ASSERT(m_isValid);
+
+ resetStateAfterProcessExited();
+
+ m_pageClient->processDidCrash();
+ m_loaderClient.processDidCrash(this);
+}
+
+void WebPageProxy::resetStateAfterProcessExited()
+{
ASSERT(m_pageClient);
-
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
m_isValid = false;
@@ -3886,17 +3896,12 @@
m_pendingLearnOrIgnoreWordMessageCount = 0;
- m_pageClient->processDidCrash();
- m_loaderClient.processDidCrash(this);
+ // If the call out to the loader client didn't cause the web process to be relaunched,
+ // we'll call setNeedsDisplay on the view so that we won't have the old contents showing.
+ // If the call did cause the web process to be relaunched, we'll keep the old page contents showing
+ // until the new web process has painted its contents.
+ setViewNeedsDisplay(IntRect(IntPoint(), viewSize()));
- if (!m_isValid) {
- // If the call out to the loader client didn't cause the web process to be relaunched,
- // we'll call setNeedsDisplay on the view so that we won't have the old contents showing.
- // If the call did cause the web process to be relaunched, we'll keep the old page contents showing
- // until the new web process has painted its contents.
- setViewNeedsDisplay(IntRect(IntPoint(), viewSize()));
- }
-
// Can't expect DidReceiveEvent notifications from a crashed web process.
m_keyEventQueue.clear();
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (148311 => 148312)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2013-04-12 23:29:13 UTC (rev 148312)
@@ -777,6 +777,8 @@
private:
WebPageProxy(PageClient*, PassRefPtr<WebProcessProxy>, WebPageGroup*, uint64_t pageID);
+ void resetStateAfterProcessExited();
+
// CoreIPC::MessageReceiver
virtual void didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&) OVERRIDE;
virtual void didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&, OwnPtr<CoreIPC::MessageEncoder>&) OVERRIDE;
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (148311 => 148312)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2013-04-12 23:29:13 UTC (rev 148312)
@@ -626,4 +626,11 @@
#endif
}
+void WebProcessProxy::requestTermination()
+{
+ ChildProcessProxy::terminate();
+ webConnection()->didClose();
+ disconnect();
+}
+
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.h (148311 => 148312)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.h 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.h 2013-04-12 23:29:13 UTC (rev 148312)
@@ -115,6 +115,8 @@
void updateProcessSuppressionState();
#endif
+ void requestTermination();
+
private:
explicit WebProcessProxy(PassRefPtr<WebContext>);
Modified: trunk/Tools/ChangeLog (148311 => 148312)
--- trunk/Tools/ChangeLog 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Tools/ChangeLog 2013-04-12 23:29:13 UTC (rev 148312)
@@ -1,3 +1,31 @@
+2013-04-12 Adenilson Cavalcanti <cavalcan...@gmail.com>
+
+ [WK2] WebPageProxy loadURL() won't work when called just after terminateProcess()
+ https://bugs.webkit.org/show_bug.cgi?id=110743
+
+ Reviewed by Benjamin Poulain.
+
+ Adding a new test file to check if loading a page just after WebProcess
+ has crashed (or was terminated) works. The test executes the
+ following steps (Load, Crash, Load), thus stressing WebProcess
+ reattach and process termination code path.
+
+ * TestWebKitAPI/GNUmakefile.am:
+ * TestWebKitAPI/PlatformEfl.cmake:
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKit2/MouseMoveAfterCrash.cpp:
+ (TestWebKitAPI::setPageLoaderClient):
+ (TestWebKitAPI::TEST):
+ * TestWebKitAPI/Tests/WebKit2/LoadPageOnCrash.cpp: Added.
+ (TestWebKitAPI):
+ (WebKit2CrashLoader):
+ (TestWebKitAPI::WebKit2CrashLoader::WebKit2CrashLoader):
+ (TestWebKitAPI::WebKit2CrashLoader::loadUrl):
+ (TestWebKitAPI::WebKit2CrashLoader::crashWebProcess):
+ (TestWebKitAPI::didFinishLoad):
+ (TestWebKitAPI::TEST):
+ * TestWebKitAPI/Tests/WebKit2/WebKit2.pro:
+
2013-04-12 Ryosuke Niwa <rn...@webkit.org>
[Mac] REGRESSION: Auto substitution strips new lines
Modified: trunk/Tools/TestWebKitAPI/GNUmakefile.am (148311 => 148312)
--- trunk/Tools/TestWebKitAPI/GNUmakefile.am 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Tools/TestWebKitAPI/GNUmakefile.am 2013-04-12 23:29:13 UTC (rev 148312)
@@ -155,6 +155,7 @@
Tools/TestWebKitAPI/Tests/WebKit2/InjectedBundleInitializationUserDataCallbackWins.cpp \
Tools/TestWebKitAPI/Tests/WebKit2/LoadAlternateHTMLStringWithNonDirectoryURL.cpp \
Tools/TestWebKitAPI/Tests/WebKit2/LoadCanceledNoServerRedirectCallback.cpp \
+ Tools/TestWebKitAPI/Tests/WebKit2/LoadPageOnCrash.cpp \
Tools/TestWebKitAPI/Tests/WebKit2/MouseMoveAfterCrash.cpp \
Tools/TestWebKitAPI/Tests/WebKit2/ReloadPageAfterCrash.cpp \
Tools/TestWebKitAPI/Tests/WebKit2/ResizeWindowAfterCrash.cpp \
Modified: trunk/Tools/TestWebKitAPI/PlatformEfl.cmake (148311 => 148312)
--- trunk/Tools/TestWebKitAPI/PlatformEfl.cmake 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Tools/TestWebKitAPI/PlatformEfl.cmake 2013-04-12 23:29:13 UTC (rev 148312)
@@ -68,6 +68,7 @@
InjectedBundleInitializationUserDataCallbackWins
LoadAlternateHTMLStringWithNonDirectoryURL
LoadCanceledNoServerRedirectCallback
+ LoadPageOnCrash
MouseMoveAfterCrash
ReloadPageAfterCrash
ResizeWindowAfterCrash
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (148311 => 148312)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2013-04-12 23:29:13 UTC (rev 148312)
@@ -91,6 +91,7 @@
81B50193140F232300D9EB58 /* StringBuilder.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 81B50192140F232300D9EB58 /* StringBuilder.cpp */; };
8A2C750E16CED9550024F352 /* ResizeWindowAfterCrash.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */; };
8A3AF93B16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */; };
+ 8AA28C1A16D2FA7B002FF4DB /* LoadPageOnCrash.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8AA28C1916D2FA7B002FF4DB /* LoadPageOnCrash.cpp */; };
930AD402150698D00067970F /* lots-of-text.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 930AD401150698B30067970F /* lots-of-text.html */; };
9318778915EEC57700A9CCE3 /* NewFirstVisuallyNonEmptyLayoutForImages.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 93AF4ECA1506F035007FD57E /* NewFirstVisuallyNonEmptyLayoutForImages.cpp */; };
9361002914DC95A70061379D /* lots-of-iframes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9361002814DC957B0061379D /* lots-of-iframes.html */; };
@@ -359,6 +360,7 @@
81B50192140F232300D9EB58 /* StringBuilder.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = StringBuilder.cpp; path = WTF/StringBuilder.cpp; sourceTree = "<group>"; };
8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ResizeWindowAfterCrash.cpp; sourceTree = "<group>"; };
8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ReloadPageAfterCrash.cpp; sourceTree = "<group>"; };
+ 8AA28C1916D2FA7B002FF4DB /* LoadPageOnCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LoadPageOnCrash.cpp; sourceTree = "<group>"; };
8DD76FA10486AA7600D96B5E /* TestWebKitAPI */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = TestWebKitAPI; sourceTree = BUILT_PRODUCTS_DIR; };
930AD401150698B30067970F /* lots-of-text.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "lots-of-text.html"; sourceTree = "<group>"; };
9361002814DC957B0061379D /* lots-of-iframes.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "lots-of-iframes.html"; sourceTree = "<group>"; };
@@ -662,6 +664,7 @@
52CB47401448FB9300873995 /* LoadAlternateHTMLStringWithNonDirectoryURL.cpp */,
33DC8910141953A300747EF7 /* LoadCanceledNoServerRedirectCallback.cpp */,
33DC89131419579F00747EF7 /* LoadCanceledNoServerRedirectCallback_Bundle.cpp */,
+ 8AA28C1916D2FA7B002FF4DB /* LoadPageOnCrash.cpp */,
33BE5AF4137B5A6C00705813 /* MouseMoveAfterCrash.cpp */,
33BE5AF8137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp */,
93F1DB3014DA20760024C362 /* NewFirstVisuallyNonEmptyLayout.cpp */,
@@ -1041,6 +1044,7 @@
26300B1816755CD90066886D /* ListHashSet.cpp in Sources */,
52CB47411448FB9300873995 /* LoadAlternateHTMLStringWithNonDirectoryURL.cpp in Sources */,
33DC8911141953A300747EF7 /* LoadCanceledNoServerRedirectCallback.cpp in Sources */,
+ 8AA28C1A16D2FA7B002FF4DB /* LoadPageOnCrash.cpp in Sources */,
B4039F9D15E6D8B3007255D6 /* MathExtras.cpp in Sources */,
CD5497B415857F0C00B5BC30 /* MediaTime.cpp in Sources */,
E1220DA0155B25480013E2FC /* MemoryCacheDisableWithinResourceLoadDelegate.mm in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2/LoadPageOnCrash.cpp (0 => 148312)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/LoadPageOnCrash.cpp (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/LoadPageOnCrash.cpp 2013-04-12 23:29:13 UTC (rev 148312)
@@ -0,0 +1,102 @@
+/*
+ * Copyright (C) 2013 Adenilson Cavalcanti <cavalcan...@gmail.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "PlatformUtilities.h"
+#include "PlatformWebView.h"
+#include "Test.h"
+#include <WebKit2/WKRetainPtr.h>
+
+namespace TestWebKitAPI {
+
+static void didFinishLoad(WKPageRef, WKFrameRef, WKTypeRef, const void*);
+
+class WebKit2CrashLoader {
+public:
+ WebKit2CrashLoader()
+ : context(AdoptWK, WKContextCreate())
+ , webView(context.get())
+ , url(adoptWK(WKURLCreateWithUTF8CString("about:blank")))
+ , firstSuccessfulLoad(false)
+ , secondSuccessfulLoad(false)
+ {
+ memset(&loaderClient, 0, sizeof(loaderClient));
+ loaderClient.clientInfo = this;
+ loaderClient.didFinishLoadForFrame = didFinishLoad;
+ WKPageSetPageLoaderClient(webView.page(), &loaderClient);
+ }
+
+ void loadUrl()
+ {
+ WKPageLoadURL(webView.page(), url.get());
+ }
+
+ void terminateWebProcess()
+ {
+ WKPageTerminate(webView.page());
+ }
+
+ WKRetainPtr<WKContextRef> context;
+ WKPageLoaderClient loaderClient;
+ PlatformWebView webView;
+ WKRetainPtr<WKURLRef> url;
+
+ bool firstSuccessfulLoad;
+ bool secondSuccessfulLoad;
+};
+
+// We are going to have 2 load events intertwined by a simulated crash
+// (i.e. Load -> Crash -> Load).
+void didFinishLoad(WKPageRef page, WKFrameRef frame, WKTypeRef userData, const void* clientInfo)
+{
+ WebKit2CrashLoader* testHelper = const_cast<WebKit2CrashLoader*>(static_cast<const WebKit2CrashLoader*>(clientInfo));
+
+ // First load worked, let's crash WebProcess.
+ if (!testHelper->firstSuccessfulLoad) {
+ testHelper->firstSuccessfulLoad = true;
+ return;
+ }
+
+ // Second load worked, we are done.
+ EXPECT_TRUE(testHelper->firstSuccessfulLoad);
+ if (!testHelper->secondSuccessfulLoad) {
+ testHelper->secondSuccessfulLoad = true;
+ return;
+ }
+}
+
+// This test will load a blank page and next kill WebProcess, the expected
+// result is that a call to page load should spawn a new WebProcess.
+TEST(WebKit2, LoadPageAfterCrash)
+{
+ WebKit2CrashLoader helper;
+ helper.loadUrl();
+ Util::run(&helper.firstSuccessfulLoad);
+ helper.terminateWebProcess();
+ helper.loadUrl();
+ Util::run(&helper.secondSuccessfulLoad);
+}
+
+} // namespace TestWebKitAPI
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2/MouseMoveAfterCrash.cpp (148311 => 148312)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/MouseMoveAfterCrash.cpp 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/MouseMoveAfterCrash.cpp 2013-04-12 23:29:13 UTC (rev 148312)
@@ -37,11 +37,6 @@
didFinishLoad = true;
}
-static void processDidCrash(WKPageRef page, const void*)
-{
- WKPageReload(page);
-}
-
static void setPageLoaderClient(WKPageRef page)
{
WKPageLoaderClient loaderClient;
@@ -49,7 +44,6 @@
loaderClient.version = 0;
loaderClient.clientInfo = 0;
loaderClient.didFinishLoadForFrame = didFinishLoadForFrame;
- loaderClient.processDidCrash = processDidCrash;
WKPageSetPageLoaderClient(page, &loaderClient);
}
@@ -79,11 +73,11 @@
webView.simulateMouseMove(10, 10);
webView.simulateMouseMove(20, 20);
- // After moving the mouse (while the web process was hung on the Pause message), kill the web process. It is restarted in
- // processDidCrash by reloading the page.
+ // After moving the mouse (while the web process was hung on the Pause message), kill the web process. It is restarted by reloading the page.
WKPageTerminate(webView.page());
+ WKPageReload(webView.page());
- // Wait until we load the page a second time (via reloading the page in processDidCrash).
+ // Wait until we load the page a second time.
Util::run(&didFinishLoad);
EXPECT_JS_FALSE(webView.page(), "didMoveMouse()");
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2/WebKit2.pro (148311 => 148312)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/WebKit2.pro 2013-04-12 23:18:50 UTC (rev 148311)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/WebKit2.pro 2013-04-12 23:29:13 UTC (rev 148312)
@@ -19,6 +19,7 @@
InjectedBundleInitializationUserDataCallbackWins.cpp \
LoadAlternateHTMLStringWithNonDirectoryURL.cpp \
LoadCanceledNoServerRedirectCallback.cpp \
+ LoadPageOnCrash.cpp \
MouseMoveAfterCrash.cpp \
PageLoadBasic.cpp \
PageLoadDidChangeLocationWithinPageForFrame.cpp \
