Diff
Modified: trunk/Source/WebKit2/ChangeLog (218294 => 218295)
--- trunk/Source/WebKit2/ChangeLog 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/ChangeLog 2017-06-14 22:49:42 UTC (rev 218295)
@@ -1,3 +1,39 @@
+2017-06-14 Chris Dumez <[email protected]>
+
+ WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load
+ https://bugs.webkit.org/show_bug.cgi?id=173384
+ <rdar://problem/32723779>
+
+ Reviewed by Dan Bernstein.
+
+ WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load.
+ This is because WebPageProxy::stopLoad() starts the responsiveness timer and expects a StopResponsinessTimer
+ IPC from the WebProcess to stop the timer so we don't report the process as unresponsive. However, if
+ WebPageProxy::close() is called before the StopResponsinessTimer IPC has been received, the page will remove
+ itself from the message receiver map and we would no longer be able to receive the StopResponsinessTimer
+ IPC and stop the timer, even if the WebProcess sent it to the UIProcess.
+
+ To address the issue, we now send the IPC Message to the WebProcessProxy instead of the WebPageProxy, so we
+ can stop the responsiveness timer, even after the WebPageProxy has been called.
+
+ * UIProcess/WebPageProxy.cpp:
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/WebPageProxy.messages.in:
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::stopResponsivenessTimer):
+ * UIProcess/WebProcessProxy.h:
+ * UIProcess/WebProcessProxy.messages.in:
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::SendStopResponsivenessTimer::~SendStopResponsivenessTimer):
+ (WebKit::WebPage::tryClose):
+ (WebKit::WebPage::loadRequest):
+ (WebKit::WebPage::loadDataImpl):
+ (WebKit::WebPage::stopLoading):
+ (WebKit::WebPage::reload):
+ (WebKit::WebPage::goForward):
+ (WebKit::WebPage::goBack):
+ (WebKit::WebPage::goToBackForwardItem):
+
2017-06-14 Commit Queue <[email protected]>
Unreviewed, rolling out r218263, r218265, and r218266.
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (218294 => 218295)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2017-06-14 22:49:42 UTC (rev 218295)
@@ -4996,11 +4996,6 @@
}
}
-void WebPageProxy::stopResponsivenessTimer()
-{
- m_process->responsivenessTimer().stop();
-}
-
void WebPageProxy::voidCallback(uint64_t callbackID)
{
auto callback = m_callbacks.take<VoidCallback>(callbackID);
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (218294 => 218295)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2017-06-14 22:49:42 UTC (rev 218295)
@@ -1440,7 +1440,6 @@
void setCursorHiddenUntilMouseMoves(bool);
void didReceiveEvent(uint32_t opaqueType, bool handled);
- void stopResponsivenessTimer();
void voidCallback(uint64_t);
void dataCallback(const IPC::DataReference&, uint64_t);
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in (218294 => 218295)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in 2017-06-14 22:49:42 UTC (rev 218295)
@@ -39,7 +39,6 @@
#endif // ENABLE(WEBGL)
DidChangeViewportProperties(struct WebCore::ViewportAttributes attributes)
DidReceiveEvent(uint32_t type, bool handled)
- StopResponsivenessTimer()
#if !PLATFORM(IOS)
SetCursor(WebCore::Cursor cursor)
SetCursorHiddenUntilMouseMoves(bool hiddenUntilMouseMoves)
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (218294 => 218295)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2017-06-14 22:49:42 UTC (rev 218295)
@@ -939,6 +939,11 @@
pages[i]->processDidTerminate(reason);
}
+void WebProcessProxy::stopResponsivenessTimer()
+{
+ responsivenessTimer().stop();
+}
+
void WebProcessProxy::enableSuddenTermination()
{
if (state() != State::Running)
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.h (218294 => 218295)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.h 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.h 2017-06-14 22:49:42 UTC (rev 218295)
@@ -151,6 +151,8 @@
void requestTermination(ProcessTerminationReason);
+ void stopResponsivenessTimer();
+
RefPtr<API::Object> transformHandlesToObjects(API::Object*);
static RefPtr<API::Object> transformObjectsToHandles(API::Object*);
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.messages.in (218294 => 218295)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.messages.in 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.messages.in 2017-06-14 22:49:42 UTC (rev 218295)
@@ -49,6 +49,8 @@
DidExceedInactiveMemoryLimit()
DidExceedCPULimit()
+ StopResponsivenessTimer()
+
RetainIconForPageURL(String pageURL)
ReleaseIconForPageURL(String pageURL)
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (218294 => 218295)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2017-06-14 22:49:42 UTC (rev 218295)
@@ -265,18 +265,10 @@
class SendStopResponsivenessTimer {
public:
- SendStopResponsivenessTimer(WebPage* page)
- : m_page(page)
- {
- }
-
~SendStopResponsivenessTimer()
{
- m_page->send(Messages::WebPageProxy::StopResponsivenessTimer());
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);
}
-
-private:
- WebPage* m_page;
};
class DeferredPageDestructor {
@@ -1177,10 +1169,10 @@
void WebPage::tryClose()
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
if (!corePage()->userInputBridge().tryClosePage()) {
- send(Messages::WebPageProxy::StopResponsivenessTimer());
+ WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);
return;
}
@@ -1209,7 +1201,7 @@
void WebPage::loadRequest(const LoadParameters& loadParameters)
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
m_pendingNavigationID = loadParameters.navigationID;
@@ -1233,7 +1225,7 @@
void WebPage::loadDataImpl(uint64_t navigationID, Ref<SharedBuffer>&& sharedBuffer, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& unreachableURL, const UserData& userData)
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
m_pendingNavigationID = navigationID;
@@ -1317,7 +1309,7 @@
void WebPage::stopLoading()
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
corePage()->userInputBridge().stopLoadingFrame(m_mainFrame->coreFrame());
}
@@ -1334,7 +1326,7 @@
void WebPage::reload(uint64_t navigationID, uint32_t reloadOptions, const SandboxExtension::Handle& sandboxExtensionHandle)
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
ASSERT(!m_mainFrame->coreFrame()->loader().frameHasLoaded() || !m_pendingNavigationID);
m_pendingNavigationID = navigationID;
@@ -1345,7 +1337,7 @@
void WebPage::goForward(uint64_t navigationID, uint64_t backForwardItemID)
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
ASSERT(item);
@@ -1360,7 +1352,7 @@
void WebPage::goBack(uint64_t navigationID, uint64_t backForwardItemID)
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
ASSERT(item);
@@ -1375,7 +1367,7 @@
void WebPage::goToBackForwardItem(uint64_t navigationID, uint64_t backForwardItemID)
{
- SendStopResponsivenessTimer stopper(this);
+ SendStopResponsivenessTimer stopper;
HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
ASSERT(item);
Modified: trunk/Tools/ChangeLog (218294 => 218295)
--- trunk/Tools/ChangeLog 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Tools/ChangeLog 2017-06-14 22:49:42 UTC (rev 218295)
@@ -1,3 +1,20 @@
+2017-06-14 Chris Dumez <[email protected]>
+
+ WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load
+ https://bugs.webkit.org/show_bug.cgi?id=173384
+ <rdar://problem/32723779>
+
+ Reviewed by Dan Bernstein.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp: Added.
+ Add API test coverage.
+
+ * TestWebKitAPI/cocoa/UtilitiesCocoa.mm:
+ (TestWebKitAPI::Util::sleep):
+ Update implementation of Util::sleep() so that we actually run the run loop.
+ Otherwise, we don't process events while sleeping.
+
2017-06-14 Alex Christensen <[email protected]>
Add SPI for immediate injection of user scripts
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (218294 => 218295)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-06-14 22:49:42 UTC (rev 218295)
@@ -484,6 +484,7 @@
8361F1781E610B4E00759B25 /* link-with-download-attribute-with-slashes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 8361F1771E610B2100759B25 /* link-with-download-attribute-with-slashes.html */; };
837A35F11D9A1E7D00663C57 /* DownloadRequestBlobURL.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */; };
83B6DE6F1EE75221001E792F /* RestoreSessionState.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */; };
+ 83DE134D1EF1C50800C1B355 /* ResponsivenessTimer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */; };
8E4A85371E1D1AB200F53B0F /* GridPosition.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8E4A85361E1D1AA100F53B0F /* GridPosition.cpp */; };
930AD402150698D00067970F /* lots-of-text.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 930AD401150698B30067970F /* lots-of-text.html */; };
9329AA291DE3F81E003ABD07 /* TextBreakIterator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9329AA281DE3F81E003ABD07 /* TextBreakIterator.cpp */; };
@@ -1293,6 +1294,7 @@
837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = DownloadRequestBlobURL.html; sourceTree = "<group>"; };
83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RestoreSessionState.cpp; sourceTree = "<group>"; };
83B88A331C80056D00BB2418 /* HTMLParserIdioms.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HTMLParserIdioms.cpp; sourceTree = "<group>"; };
+ 83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ResponsivenessTimer.cpp; sourceTree = "<group>"; };
86BD19971A2DB05B006DCF0A /* RefCounter.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RefCounter.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>"; };
@@ -2189,6 +2191,7 @@
8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */,
2DD7D3A9178205D00026E1E3 /* ResizeReversePaginatedWebView.cpp */,
8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */,
+ 83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */,
C0BD669C131D3CF700E18F2A /* ResponsivenessTimerDoesntFireEarly.cpp */,
C0BD669E131D3CFF00E18F2A /* ResponsivenessTimerDoesntFireEarly_Bundle.cpp */,
83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */,
@@ -2983,6 +2986,7 @@
7A95BDE11E9BEC5F00865498 /* InjectedBundleAppleEvent.cpp in Sources */,
510477781D29923B009747EB /* IDBDeleteRecovery.mm in Sources */,
5110FCFA1E01CDB8006F8D0B /* IDBIndexUpgradeToV2.mm in Sources */,
+ 83DE134D1EF1C50800C1B355 /* ResponsivenessTimer.cpp in Sources */,
51A587861D273AA9004BA9AF /* IndexedDBDatabaseProcessKill.mm in Sources */,
7C83E0BE1D0A651300FEBCF3 /* IndexedDBMultiProcess.mm in Sources */,
7C83E0BF1D0A652200FEBCF3 /* IndexedDBPersistence.mm in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp (0 => 218295)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp 2017-06-14 22:49:42 UTC (rev 218295)
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All rights reserved.
+ *
+ * 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"
+
+#if WK_HAVE_C_SPI
+
+#include "PlatformUtilities.h"
+#include "PlatformWebView.h"
+
+namespace TestWebKitAPI {
+
+static bool didFinishLoad { false };
+static bool didBecomeUnresponsive { false };
+
+static void didFinishLoadForFrame(WKPageRef, WKFrameRef, WKTypeRef, const void*)
+{
+ didFinishLoad = true;
+}
+
+static void processDidBecomeUnresponsive(WKPageRef page, const void*)
+{
+ didBecomeUnresponsive = true;
+}
+
+static void setPageLoaderClient(WKPageRef page)
+{
+ WKPageLoaderClientV0 loaderClient;
+ memset(&loaderClient, 0, sizeof(loaderClient));
+
+ loaderClient.base.version = 0;
+ loaderClient.didFinishLoadForFrame = didFinishLoadForFrame;
+ loaderClient.processDidBecomeUnresponsive = processDidBecomeUnresponsive;
+
+ WKPageSetPageLoaderClient(page, &loaderClient.base);
+}
+
+TEST(WebKit2, ResponsivenessTimerShouldNotFireAfterTearDown)
+{
+ WKRetainPtr<WKContextRef> context(AdoptWK, WKContextCreate());
+ // The two views need to share the same WebContent process.
+ WKContextSetMaximumNumberOfProcesses(context.get(), 1);
+
+ PlatformWebView webView1(context.get());
+ setPageLoaderClient(webView1.page());
+
+ WKPageLoadURL(webView1.page(), adoptWK(Util::createURLForResource("simple", "html")).get());
+ Util::run(&didFinishLoad);
+
+ EXPECT_FALSE(didBecomeUnresponsive);
+
+ PlatformWebView webView2(context.get());
+ setPageLoaderClient(webView2.page());
+
+ didFinishLoad = false;
+ WKPageLoadURL(webView2.page(), adoptWK(Util::createURLForResource("simple", "html")).get());
+ Util::run(&didFinishLoad);
+
+ EXPECT_FALSE(didBecomeUnresponsive);
+
+ // Call stopLoading() and close() on the first page in quick succession.
+ WKPageStopLoading(webView1.page());
+ WKPageClose(webView1.page());
+
+ // We need to wait here because it takes 3 seconds for a process to be recognized as unresponsive.
+ Util::sleep(4);
+
+ // We should not report the second page sharing the same process as unresponsive.
+ EXPECT_FALSE(didBecomeUnresponsive);
+}
+
+} // namespace TestWebKitAPI
+
+#endif
Modified: trunk/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm (218294 => 218295)
--- trunk/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm 2017-06-14 22:19:49 UTC (rev 218294)
+++ trunk/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm 2017-06-14 22:49:42 UTC (rev 218295)
@@ -37,7 +37,7 @@
void sleep(double seconds)
{
- usleep(seconds * 1000000);
+ [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:seconds]];
}
} // namespace Util
