WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load.
<rdar://problem/32723779>
Created attachment 312920 [details] WIP Patch
Created attachment 312922 [details] Patch
Comment on attachment 312922 [details] Patch Clearing flags on attachment: 312922 Committed r218295: <http://trac.webkit.org/changeset/218295>
All reviewed patches have been landed. Closing bug.
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 @@ >+2017-06-14 Chris Dumez <cdumez@apple.com> >+ >+ 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 <jbedard@apple.com> > > 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 @@ >+2017-06-14 Chris Dumez <cdumez@apple.com> >+ >+ 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 <timothy_horton@apple.com> > > 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
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)
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.
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