Bug 173384 – WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load.

[Chris Dumez]

<rdar://problem/32723779>

[Chris Dumez]

Created attachment 312920 [details]
WIP Patch

[Chris Dumez]

Created attachment 312922 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 312922 [details]
Patch

Clearing flags on attachment: 312922

Committed r218295: <http://trac.webkit.org/changeset/218295>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Olivier Blin]

Comment on attachment 312922 [details]
Patch

>Subversion Revision: 218277
>diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
>index b7910c723e92c06e03b4528784ece3f1b7eede95..7ab5e2dd817a62e0ad1b90f7dee03ea269d04592 100644
>--- a/Source/WebKit2/ChangeLog
>+++ b/Source/WebKit2/ChangeLog
>@@ -1,3 +1,39 @@
>[phone]  Chris Dumez  <[e-mail]>
>+
>+        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 NOBODY (OOPS!).
>+
>+        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  Jonathan Bedard  <[e-mail]>
> 
>         Configure screen scale for running layout tests on plus devices
>diff --git a/Source/WebKit2/UIProcess/WebPageProxy.cpp b/Source/WebKit2/UIProcess/WebPageProxy.cpp
>index e446aea68e1202d05528c2e402737429cff93d00..cf069c3d35a49d8107ecebfcc204ef3c77dcdf2c 100644
>--- a/Source/WebKit2/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit2/UIProcess/WebPageProxy.cpp
>@@ -4996,11 +4996,6 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
>     }
> }
> 
>-void WebPageProxy::stopResponsivenessTimer()
>-{
>-    m_process->responsivenessTimer().stop();
>-}
>-
> void WebPageProxy::voidCallback(uint64_t callbackID)
> {
>     auto callback = m_callbacks.take<VoidCallback>(callbackID);
>diff --git a/Source/WebKit2/UIProcess/WebPageProxy.h b/Source/WebKit2/UIProcess/WebPageProxy.h
>index 6a45774baf68ed31420931715d050596cd170ea5..e2246413cf8f4151558c0bbf3ecaca5e235de701 100644
>--- a/Source/WebKit2/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit2/UIProcess/WebPageProxy.h
>@@ -1440,7 +1440,6 @@ private:
>     void setCursorHiddenUntilMouseMoves(bool);
> 
>     void didReceiveEvent(uint32_t opaqueType, bool handled);
>-    void stopResponsivenessTimer();
> 
>     void voidCallback(uint64_t);
>     void dataCallback(const IPC::DataReference&, uint64_t);
>diff --git a/Source/WebKit2/UIProcess/WebPageProxy.messages.in b/Source/WebKit2/UIProcess/WebPageProxy.messages.in
>index 60eaa52382b96ec456de3d95d4ddb73c8c0ac56d..2c9cac43be43b14955c65a03d290d937285035b3 100644
>--- a/Source/WebKit2/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit2/UIProcess/WebPageProxy.messages.in
>@@ -39,7 +39,6 @@ messages -> WebPageProxy {
> #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)
>diff --git a/Source/WebKit2/UIProcess/WebProcessProxy.cpp b/Source/WebKit2/UIProcess/WebProcessProxy.cpp
>index f93331bad8fbd515582ff18695ea8c49654f69e4..4019f2730888e21effa82f7c2fc0b070588bd77d 100644
>--- a/Source/WebKit2/UIProcess/WebProcessProxy.cpp
>+++ b/Source/WebKit2/UIProcess/WebProcessProxy.cpp
>@@ -939,6 +939,11 @@ void WebProcessProxy::requestTermination(ProcessTerminationReason reason)
>         pages[i]->processDidTerminate(reason);
> }
> 
>+void WebProcessProxy::stopResponsivenessTimer()
>+{
>+    responsivenessTimer().stop();
>+}
>+
> void WebProcessProxy::enableSuddenTermination()
> {
>     if (state() != State::Running)
>diff --git a/Source/WebKit2/UIProcess/WebProcessProxy.h b/Source/WebKit2/UIProcess/WebProcessProxy.h
>index fd9d6f3ea7f353a7b22d92025f4cd1bba3e58b75..d00b47494997439c2e8de345135c91a2531a921e 100644
>--- a/Source/WebKit2/UIProcess/WebProcessProxy.h
>+++ b/Source/WebKit2/UIProcess/WebProcessProxy.h
>@@ -151,6 +151,8 @@ public:
> 
>     void requestTermination(ProcessTerminationReason);
> 
>+    void stopResponsivenessTimer();
>+
>     RefPtr<API::Object> transformHandlesToObjects(API::Object*);
>     static RefPtr<API::Object> transformObjectsToHandles(API::Object*);
> 
>diff --git a/Source/WebKit2/UIProcess/WebProcessProxy.messages.in b/Source/WebKit2/UIProcess/WebProcessProxy.messages.in
>index 9337830c2715b489a582164d3fb56692e0ab500a..b2bb78120f55e645a6e2129ddd3f535dded485b3 100644
>--- a/Source/WebKit2/UIProcess/WebProcessProxy.messages.in
>+++ b/Source/WebKit2/UIProcess/WebProcessProxy.messages.in
>@@ -49,6 +49,8 @@ messages -> WebProcessProxy LegacyReceiver {
>     DidExceedInactiveMemoryLimit()
>     DidExceedCPULimit()
> 
>+    StopResponsivenessTimer()
>+
>     RetainIconForPageURL(String pageURL)
>     ReleaseIconForPageURL(String pageURL)
> 
>diff --git a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
>index f52da8e1f3f95997f512bfe7860a542a80806848..2eb675db1e9c9a0f52da70cc23e2cefcb24cfa75 100644
>--- a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
>@@ -264,18 +264,10 @@ static const Seconds maximumLayerVolatilityTimerInterval { 2_s };
> 
> 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 {
>@@ -1180,10 +1172,10 @@ void WebPage::close()
> 
> void WebPage::tryClose()
> {
>-    SendStopResponsivenessTimer stopper(this);
>+    SendStopResponsivenessTimer stopper;
> 
>     if (!corePage()->userInputBridge().tryClosePage()) {
>-        send(Messages::WebPageProxy::StopResponsivenessTimer());
>+        WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);

Isn't this call duplicate?
There is already a SendStopResponsivenessTimer object in the method scope, that will send the same message when destroyed.

>         return;
>     }
> 
>@@ -1212,7 +1204,7 @@ void WebPage::platformDidReceiveLoadParameters(const LoadParameters& loadParamet
> 
> void WebPage::loadRequest(const LoadParameters& loadParameters)
> {
>-    SendStopResponsivenessTimer stopper(this);
>+    SendStopResponsivenessTimer stopper;
> 
>     m_pendingNavigationID = loadParameters.navigationID;
> 
>@@ -1236,7 +1228,7 @@ void WebPage::loadRequest(const LoadParameters& loadParameters)
> 
> 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;
> 
>@@ -1320,7 +1312,7 @@ void WebPage::stopLoadingFrame(uint64_t frameID)
> 
> void WebPage::stopLoading()
> {
>-    SendStopResponsivenessTimer stopper(this);
>+    SendStopResponsivenessTimer stopper;
> 
>     corePage()->userInputBridge().stopLoadingFrame(m_mainFrame->coreFrame());
> }
>@@ -1337,7 +1329,7 @@ void WebPage::setDefersLoading(bool defersLoading)
> 
> 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;
>@@ -1348,7 +1340,7 @@ void WebPage::reload(uint64_t navigationID, uint32_t reloadOptions, const Sandbo
> 
> void WebPage::goForward(uint64_t navigationID, uint64_t backForwardItemID)
> {
>-    SendStopResponsivenessTimer stopper(this);
>+    SendStopResponsivenessTimer stopper;
> 
>     HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
>     ASSERT(item);
>@@ -1363,7 +1355,7 @@ void WebPage::goForward(uint64_t navigationID, uint64_t backForwardItemID)
> 
> void WebPage::goBack(uint64_t navigationID, uint64_t backForwardItemID)
> {
>-    SendStopResponsivenessTimer stopper(this);
>+    SendStopResponsivenessTimer stopper;
> 
>     HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
>     ASSERT(item);
>@@ -1378,7 +1370,7 @@ void WebPage::goBack(uint64_t navigationID, uint64_t backForwardItemID)
> 
> void WebPage::goToBackForwardItem(uint64_t navigationID, uint64_t backForwardItemID)
> {
>-    SendStopResponsivenessTimer stopper(this);
>+    SendStopResponsivenessTimer stopper;
> 
>     HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
>     ASSERT(item);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index ecfa4ee447ea46ae2bbb53fdf350a3b97b2528c4..ada5afbca58c9b9b1fb78f3f4c6e413913ac17ff 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,20 @@
>[phone]  Chris Dumez  <[e-mail]>
>+
>+        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 NOBODY (OOPS!).
>+
>+        * 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  Tim Horton  <[e-mail]>
> 
>         WKContentViewEditingActions API test always fails
>diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>index 99c0c09247d03aceee633b6961036a4084d42e49..95f961c089fb5ee905528388ba6e4980c4354395 100644
>--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>@@ -483,6 +483,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 */; };
>@@ -1291,6 +1292,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>"; };
>@@ -2186,6 +2188,7 @@
> 				8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */,
> 				2DD7D3A9178205D00026E1E3 /* ResizeReversePaginatedWebView.cpp */,
> 				8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */,
>+				83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */,
> 				C0BD669C131D3CF700E18F2A /* ResponsivenessTimerDoesntFireEarly.cpp */,
> 				C0BD669E131D3CFF00E18F2A /* ResponsivenessTimerDoesntFireEarly_Bundle.cpp */,
> 				83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */,
>@@ -2980,6 +2983,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 */,
>diff --git a/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp b/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..06f81d53f69f716a1a8f82bba4471e52f87eecd4
>--- /dev/null
>+++ b/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp
>@@ -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
>diff --git a/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm b/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm
>index 1f7538548530f278f47f70a16226da63ffb1fde6..ba5a52f9b6fa7787787c2c13f0ddad6d88547cd8 100644
>--- a/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm
>+++ b/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm
>@@ -37,7 +37,7 @@ void run(bool* done)
> 
> void sleep(double seconds)
> {
>-    usleep(seconds * 1000000);
>+    [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:seconds]];
> }
> 
> } // namespace Util

[Olivier Blin]

Comment on attachment 312922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312922&action=review

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:270
>      ~SendStopResponsivenessTimer()
>      {
> -        m_page->send(Messages::WebPageProxy::StopResponsivenessTimer());
> +        WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);
>      }

.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1178
>  void WebPage::tryClose()
>  {
> -    SendStopResponsivenessTimer stopper(this);
> +    SendStopResponsivenessTimer stopper;
>  
>      if (!corePage()->userInputBridge().tryClosePage()) {
> -        send(Messages::WebPageProxy::StopResponsivenessTimer());
> +        WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);

Isn't this call duplicate?
There is already a SendStopResponsivenessTimer object in the method scope, that will send the same message when destroyed.

(sorry for the previous comment)

[Chris Dumez]

Comment on attachment 312922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312922&action=review

>> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1178
>> +        WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);
> 
> Isn't this call duplicate?
> There is already a SendStopResponsivenessTimer object in the method scope, that will send the same message when destroyed.
> 
> (sorry for the previous comment)

Yes, it looks like you're right. I'll drop it in a follow-up.

[Chris Dumez]

Comment on attachment 312922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312922&action=review

>>> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1178
>>> +        WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);
>> 
>> Isn't this call duplicate?
>> There is already a SendStopResponsivenessTimer object in the method scope, that will send the same message when destroyed.
>> 
>> (sorry for the previous comment)
> 
> Yes, it looks like you're right. I'll drop it in a follow-up.

https://bugs.webkit.org/show_bug.cgi?id=173533