Title: [260966] trunk/Source/WebKit
Revision
260966
Author
cdu...@apple.com
Date
2020-04-30 14:19:15 -0700 (Thu, 30 Apr 2020)

Log Message

[iOS][WK2] Add timeout for "Client navigation" foreground assertion
https://bugs.webkit.org/show_bug.cgi?id=211202

Reviewed by Alex Christensen.

Add a 30s timeout for "Client navigation" foreground assertion. Foreground assertions do not timeout
at RunningBoard level and we thus need to make sure we release them to avoid power leaks.

* UIProcess/API/APINavigation.cpp:
(API::Navigation::Navigation):
* UIProcess/API/APINavigation.h:
(API::Navigation::setClientNavigationActivity):
(API::Navigation::setForegroundActivity): Deleted.
* UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::TimedActivity::TimedActivity):
(WebKit::ProcessThrottler::TimedActivity::operator=):
(WebKit::ProcessThrottler::TimedActivity::activityTimedOut):
(WebKit::ProcessThrottler::TimedActivity::updateTimer):
* UIProcess/ProcessThrottler.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::loadRequest):
(WebKit::WebPageProxy::loadFile):
(WebKit::WebPageProxy::loadData):
(WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
(WebKit::WebPageProxy::didFinishLoadForFrame):
(WebKit::WebPageProxy::didFailLoadForFrame):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (260965 => 260966)


--- trunk/Source/WebKit/ChangeLog	2020-04-30 21:15:19 UTC (rev 260965)
+++ trunk/Source/WebKit/ChangeLog	2020-04-30 21:19:15 UTC (rev 260966)
@@ -1,3 +1,32 @@
+2020-04-30  Chris Dumez  <cdu...@apple.com>
+
+        [iOS][WK2] Add timeout for "Client navigation" foreground assertion
+        https://bugs.webkit.org/show_bug.cgi?id=211202
+
+        Reviewed by Alex Christensen.
+
+        Add a 30s timeout for "Client navigation" foreground assertion. Foreground assertions do not timeout
+        at RunningBoard level and we thus need to make sure we release them to avoid power leaks.
+
+        * UIProcess/API/APINavigation.cpp:
+        (API::Navigation::Navigation):
+        * UIProcess/API/APINavigation.h:
+        (API::Navigation::setClientNavigationActivity):
+        (API::Navigation::setForegroundActivity): Deleted.
+        * UIProcess/ProcessThrottler.cpp:
+        (WebKit::ProcessThrottler::TimedActivity::TimedActivity):
+        (WebKit::ProcessThrottler::TimedActivity::operator=):
+        (WebKit::ProcessThrottler::TimedActivity::activityTimedOut):
+        (WebKit::ProcessThrottler::TimedActivity::updateTimer):
+        * UIProcess/ProcessThrottler.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::loadRequest):
+        (WebKit::WebPageProxy::loadFile):
+        (WebKit::WebPageProxy::loadData):
+        (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+        (WebKit::WebPageProxy::didFinishLoadForFrame):
+        (WebKit::WebPageProxy::didFailLoadForFrame):
+
 2020-04-30  Alex Christensen  <achristen...@webkit.org>
 
         Add SPI to change a WKWebView's CORS disabling pattern after initialization

Modified: trunk/Source/WebKit/UIProcess/API/APINavigation.cpp (260965 => 260966)


--- trunk/Source/WebKit/UIProcess/API/APINavigation.cpp	2020-04-30 21:15:19 UTC (rev 260965)
+++ trunk/Source/WebKit/UIProcess/API/APINavigation.cpp	2020-04-30 21:19:15 UTC (rev 260966)
@@ -35,8 +35,11 @@
 using namespace WebCore;
 using namespace WebKit;
 
+static constexpr Seconds navigationActivityTimeout { 30_s };
+
 Navigation::Navigation(WebNavigationState& state)
     : m_navigationID(state.generateNavigationID())
+    , m_clientNavigationActivity(navigationActivityTimeout)
 {
 }
 
@@ -43,6 +46,7 @@
 Navigation::Navigation(WebNavigationState& state, WebBackForwardListItem* currentAndTargetItem)
     : m_navigationID(state.generateNavigationID())
     , m_reloadItem(currentAndTargetItem)
+    , m_clientNavigationActivity(navigationActivityTimeout)
 {
 }
 
@@ -51,6 +55,7 @@
     , m_originalRequest(WTFMove(request))
     , m_currentRequest(m_originalRequest)
     , m_fromItem(fromItem)
+    , m_clientNavigationActivity(navigationActivityTimeout)
 {
     m_redirectChain.append(m_originalRequest.url());
 }
@@ -62,6 +67,7 @@
     , m_targetItem(&targetItem)
     , m_fromItem(fromItem)
     , m_backForwardFrameLoadType(backForwardFrameLoadType)
+    , m_clientNavigationActivity(navigationActivityTimeout)
 {
 }
 

Modified: trunk/Source/WebKit/UIProcess/API/APINavigation.h (260965 => 260966)


--- trunk/Source/WebKit/UIProcess/API/APINavigation.h	2020-04-30 21:15:19 UTC (rev 260965)
+++ trunk/Source/WebKit/UIProcess/API/APINavigation.h	2020-04-30 21:19:15 UTC (rev 260966)
@@ -155,7 +155,7 @@
 
     const Optional<WebCore::AdClickAttribution>& adClickAttribution() const { return m_lastNavigationAction.adClickAttribution; }
 
-    void setForegroundActivity(std::unique_ptr<WebKit::ProcessThrottler::ForegroundActivity>&& activity) { m_foregroundActivity = WTFMove(activity); }
+    void setClientNavigationActivity(WebKit::ProcessThrottler::ActivityVariant&& activity) { m_clientNavigationActivity = WTFMove(activity); }
 
     void setIsLoadedWithNavigationShared(bool value) { m_isLoadedWithNavigationShared = value; }
     bool isLoadedWithNavigationShared() const { return m_isLoadedWithNavigationShared; }
@@ -183,7 +183,7 @@
     WebCore::SecurityOriginData m_destinationFrameSecurityOrigin;
     bool m_userContentExtensionsEnabled { true };
     WebKit::WebContentMode m_effectiveContentMode { WebKit::WebContentMode::Recommended };
-    std::unique_ptr<WebKit::ProcessThrottler::ForegroundActivity> m_foregroundActivity;
+    WebKit::ProcessThrottler::TimedActivity m_clientNavigationActivity;
     bool m_isLoadedWithNavigationShared { false };
 };
 

Modified: trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp (260965 => 260966)


--- trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp	2020-04-30 21:15:19 UTC (rev 260965)
+++ trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp	2020-04-30 21:19:15 UTC (rev 260966)
@@ -239,4 +239,33 @@
     return WTF::get<UniqueRef<ProcessThrottler::ForegroundActivity>>(activity)->isValid();
 }
 
+ProcessThrottler::TimedActivity::TimedActivity(Seconds timeout, ProcessThrottler::ActivityVariant&& activity)
+    : m_timer(RunLoop::main(), this, &TimedActivity::activityTimedOut)
+    , m_timeout(timeout)
+    , m_activity(WTFMove(activity))
+{
+    updateTimer();
+}
+
+auto ProcessThrottler::TimedActivity::operator=(ProcessThrottler::ActivityVariant&& activity) -> TimedActivity&
+{
+    m_activity = WTFMove(activity);
+    updateTimer();
+    return *this;
+}
+
+void ProcessThrottler::TimedActivity::activityTimedOut()
+{
+    RELEASE_LOG_ERROR(ProcessSuspension, "%p - TimedActivity::activityTimedOut:", this);
+    m_activity = nullptr;
+}
+
+void ProcessThrottler::TimedActivity::updateTimer()
+{
+    if (WTF::holds_alternative<std::nullptr_t>(m_activity))
+        m_timer.stop();
+    else
+        m_timer.startOneShot(m_timeout);
+}
+
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/ProcessThrottler.h (260965 => 260966)


--- trunk/Source/WebKit/UIProcess/ProcessThrottler.h	2020-04-30 21:15:19 UTC (rev 260965)
+++ trunk/Source/WebKit/UIProcess/ProcessThrottler.h	2020-04-30 21:19:15 UTC (rev 260966)
@@ -102,6 +102,20 @@
     using ActivityVariant = Variant<std::nullptr_t, UniqueRef<BackgroundActivity>, UniqueRef<ForegroundActivity>>;
     static bool isValidBackgroundActivity(const ActivityVariant&);
     static bool isValidForegroundActivity(const ActivityVariant&);
+
+    class TimedActivity {
+    public:
+        TimedActivity(Seconds timeout, ActivityVariant&& = nullptr);
+        TimedActivity& operator=(ActivityVariant&&);
+
+    private:
+        void activityTimedOut();
+        void updateTimer();
+
+        RunLoop::Timer<TimedActivity> m_timer;
+        Seconds m_timeout;
+        ActivityVariant m_activity;
+    };
     
     void didConnectToProcess(ProcessID);
     bool shouldBeRunnable() const { return m_foregroundActivities.size() || m_backgroundActivities.size(); }

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (260965 => 260966)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2020-04-30 21:15:19 UTC (rev 260965)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2020-04-30 21:19:15 UTC (rev 260966)
@@ -1301,7 +1301,7 @@
     auto navigation = m_navigationState->createLoadRequestNavigation(ResourceRequest(request), m_backForwardList->currentItem());
 
     if (shouldForceForegroundPriorityForClientNavigation())
-        navigation->setForegroundActivity(process().throttler().foregroundActivity("Client navigation"_s).moveToUniquePtr());
+        navigation->setClientNavigationActivity(process().throttler().foregroundActivity("Client navigation"_s));
 
     loadRequestWithNavigationShared(m_process.copyRef(), m_webPageID, navigation.get(), WTFMove(request), shouldOpenExternalURLsPolicy, userData, ShouldTreatAsContinuingLoad::No, isNavigatingToAppBoundDomain(), hasNavigatedAwayFromAppBoundDomain());
     return navigation;
@@ -1382,7 +1382,7 @@
     auto navigation = m_navigationState->createLoadRequestNavigation(ResourceRequest(fileURL), m_backForwardList->currentItem());
 
     if (shouldForceForegroundPriorityForClientNavigation())
-        navigation->setForegroundActivity(process().throttler().foregroundActivity("Client navigation"_s).moveToUniquePtr());
+        navigation->setClientNavigationActivity(process().throttler().foregroundActivity("Client navigation"_s));
 
     auto transaction = m_pageLoadState.transaction();
 
@@ -1425,7 +1425,7 @@
     auto navigation = m_navigationState->createLoadDataNavigation(makeUnique<API::SubstituteData>(data.vector(), MIMEType, encoding, baseURL, userData));
 
     if (shouldForceForegroundPriorityForClientNavigation())
-        navigation->setForegroundActivity(process().throttler().foregroundActivity("Client navigation"_s).moveToUniquePtr());
+        navigation->setClientNavigationActivity(process().throttler().foregroundActivity("Client navigation"_s));
 
     loadDataWithNavigationShared(m_process.copyRef(), m_webPageID, navigation, data, MIMEType, encoding, baseURL, userData, ShouldTreatAsContinuingLoad::No, isNavigatingToAppBoundDomain(), hasNavigatedAwayFromAppBoundDomain(), WTF::nullopt, shouldOpenExternalURLsPolicy);
     return navigation;
@@ -4602,7 +4602,7 @@
         m_pageLoadState.didFailProvisionalLoad(transaction);
         pageClient().didFailProvisionalLoadForMainFrame();
         if (navigation)
-            navigation->setForegroundActivity(nullptr);
+            navigation->setClientNavigationActivity(nullptr);
     }
 
     frame->didFailProvisionalLoad();
@@ -4839,7 +4839,7 @@
         pageClient().didFinishLoadForMainFrame();
 
         if (navigation)
-            navigation->setForegroundActivity(nullptr);
+            navigation->setClientNavigationActivity(nullptr);
 
         resetRecentCrashCountSoon();
 
@@ -4892,7 +4892,7 @@
         reportPageLoadResult(error);
         pageClient().didFailLoadForMainFrame();
         if (navigation)
-            navigation->setForegroundActivity(nullptr);
+            navigation->setClientNavigationActivity(nullptr);
     }
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to