Title: [218598] tags/Safari-604.1.26.2

Diff

Modified: tags/Safari-604.1.26.2/Source/WebKit2/ChangeLog (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/ChangeLog	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/ChangeLog	2017-06-20 17:09:38 UTC (rev 218598)
@@ -1,3 +1,43 @@
+2017-06-20  Jason Marcell  <jmarc...@apple.com>
+
+        Cherry-pick r218295. rdar://problem/32723779
+
+    2017-06-14  Chris Dumez  <cdu...@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 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  Carlos Garcia Campos  <cgar...@igalia.com>
 
         [GTK] Use API::InjectedBundle::EditorClient in WebKitWebEditor

Modified: tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.cpp (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.cpp	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.cpp	2017-06-20 17:09:38 UTC (rev 218598)
@@ -4986,11 +4986,6 @@
     }
 }
 
-void WebPageProxy::stopResponsivenessTimer()
-{
-    m_process->responsivenessTimer().stop();
-}
-
 void WebPageProxy::voidCallback(uint64_t callbackID)
 {
     auto callback = m_callbacks.take<VoidCallback>(callbackID);

Modified: tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.h (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.h	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.h	2017-06-20 17:09:38 UTC (rev 218598)
@@ -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: tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.messages.in (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.messages.in	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebPageProxy.messages.in	2017-06-20 17:09:38 UTC (rev 218598)
@@ -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: tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.cpp (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.cpp	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.cpp	2017-06-20 17:09:38 UTC (rev 218598)
@@ -939,6 +939,11 @@
         pages[i]->processDidTerminate(reason);
 }
 
+void WebProcessProxy::stopResponsivenessTimer()
+{
+    responsivenessTimer().stop();
+}
+
 void WebProcessProxy::enableSuddenTermination()
 {
     if (state() != State::Running)

Modified: tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.h (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.h	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.h	2017-06-20 17:09:38 UTC (rev 218598)
@@ -151,6 +151,8 @@
 
     void requestTermination(ProcessTerminationReason);
 
+    void stopResponsivenessTimer();
+
     RefPtr<API::Object> transformHandlesToObjects(API::Object*);
     static RefPtr<API::Object> transformObjectsToHandles(API::Object*);
 

Modified: tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.messages.in (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.messages.in	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/UIProcess/WebProcessProxy.messages.in	2017-06-20 17:09:38 UTC (rev 218598)
@@ -49,6 +49,8 @@
     DidExceedInactiveMemoryLimit()
     DidExceedCPULimit()
 
+    StopResponsivenessTimer()
+
     RetainIconForPageURL(String pageURL)
     ReleaseIconForPageURL(String pageURL)
 

Modified: tags/Safari-604.1.26.2/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (218597 => 218598)


--- tags/Safari-604.1.26.2/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2017-06-20 17:09:38 UTC (rev 218598)
@@ -264,18 +264,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 {
@@ -1176,10 +1168,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;
     }
 
@@ -1208,7 +1200,7 @@
 
 void WebPage::loadRequest(const LoadParameters& loadParameters)
 {
-    SendStopResponsivenessTimer stopper(this);
+    SendStopResponsivenessTimer stopper;
 
     m_pendingNavigationID = loadParameters.navigationID;
 
@@ -1232,7 +1224,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;
 
@@ -1316,7 +1308,7 @@
 
 void WebPage::stopLoading()
 {
-    SendStopResponsivenessTimer stopper(this);
+    SendStopResponsivenessTimer stopper;
 
     corePage()->userInputBridge().stopLoadingFrame(m_mainFrame->coreFrame());
 }
@@ -1333,7 +1325,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;
@@ -1344,7 +1336,7 @@
 
 void WebPage::goForward(uint64_t navigationID, uint64_t backForwardItemID)
 {
-    SendStopResponsivenessTimer stopper(this);
+    SendStopResponsivenessTimer stopper;
 
     HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
     ASSERT(item);
@@ -1359,7 +1351,7 @@
 
 void WebPage::goBack(uint64_t navigationID, uint64_t backForwardItemID)
 {
-    SendStopResponsivenessTimer stopper(this);
+    SendStopResponsivenessTimer stopper;
 
     HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
     ASSERT(item);
@@ -1374,7 +1366,7 @@
 
 void WebPage::goToBackForwardItem(uint64_t navigationID, uint64_t backForwardItemID)
 {
-    SendStopResponsivenessTimer stopper(this);
+    SendStopResponsivenessTimer stopper;
 
     HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
     ASSERT(item);

Modified: tags/Safari-604.1.26.2/Tools/ChangeLog (218597 => 218598)


--- tags/Safari-604.1.26.2/Tools/ChangeLog	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Tools/ChangeLog	2017-06-20 17:09:38 UTC (rev 218598)
@@ -1,3 +1,24 @@
+2017-06-20  Jason Marcell  <jmarc...@apple.com>
+
+        Cherry-pick r218295. rdar://problem/32723779
+
+    2017-06-14  Chris Dumez  <cdu...@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 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  Nael Ouedraogo  <nael.ouedra...@crf.canon.fr>
 
         MediaSource duration attribute should not be equal to Infinity when set to a value greater than 2^64

Modified: tags/Safari-604.1.26.2/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (218597 => 218598)


--- tags/Safari-604.1.26.2/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2017-06-20 17:09:38 UTC (rev 218598)
@@ -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 */,

Added: tags/Safari-604.1.26.2/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp (0 => 218598)


--- tags/Safari-604.1.26.2/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp	                        (rev 0)
+++ tags/Safari-604.1.26.2/Tools/TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp	2017-06-20 17:09:38 UTC (rev 218598)
@@ -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: tags/Safari-604.1.26.2/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm (218597 => 218598)


--- tags/Safari-604.1.26.2/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm	2017-06-20 17:03:23 UTC (rev 218597)
+++ tags/Safari-604.1.26.2/Tools/TestWebKitAPI/cocoa/UtilitiesCocoa.mm	2017-06-20 17:09:38 UTC (rev 218598)
@@ -37,7 +37,7 @@
 
 void sleep(double seconds)
 {
-    usleep(seconds * 1000000);
+    [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:seconds]];
 }
 
 } // namespace Util
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to