Attachment 369327 Details for Bug 197674 – Patch

[patch] Patch

bug-197674-20190507140950.patch (text/plain), 6.08 KB, created by Chris Dumez on 2019-05-07 14:09:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-05-07 14:09:51 PDT

Size: 6.08 KB

patch

obsolete

>Subversion Revision: 245028
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 8f8a1d25ed49a4a8faa0c0302220d21fb1e611f1..e6405410b0c8366cb7c11aad64cd1c72eaf9f608 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-05-07  Chris Dumez  <cdumez@apple.com>
>+
>+        Simplify logic to prevent App Nap in WebPage
>+        https://bugs.webkit.org/show_bug.cgi?id=197674
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Simplify logic to prevent App Nap in WebPage. We do not need both m_userActivityHysteresis and
>+        m_userActivity since UserActivity is already a HysteresisActivity. We had 2 levels of
>+        HysteresisActivity stacked on top of one another. Also rename "process suppression" to "app nap" as
>+        I find it clearer.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::updateThrottleState):
>+        (WebKit::WebPage::mouseEvent):
>+        (WebKit::WebPage::wheelEvent):
>+        (WebKit::WebPage::keyEvent):
>+        (WebKit::WebPage::updatePreferences):
>+        (WebKit::m_userActivityHysteresis): Deleted.
>+        (WebKit::WebPage::updateUserActivity): Deleted.
>+        * WebProcess/WebPage/WebPage.h:
>+
> 2019-05-07  Adrian Perez de Castro  <aperez@igalia.com>
> 
>         [GTK][WPE] Cannot build documentation with gtk-doc
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 58ac38bd937b6273cf31e8e3ccf9a255b08587c9..8e24e3d7334ce5c159e8abae1ca5bf4bff74ae61 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -416,9 +416,8 @@ WebPage::WebPage(uint64_t pageID, WebPageCreationParameters&& parameters)
> #endif
>     , m_layerVolatilityTimer(*this, &WebPage::layerVolatilityTimerFired)
>     , m_activityState(parameters.activityState)
>-    , m_processSuppressionEnabled(true)
>-    , m_userActivity("Process suppression disabled for page.")
>-    , m_userActivityHysteresis([this](PAL::HysteresisState) { updateUserActivity(); })
>+    , m_isAppNapEnabled(true)
>+    , m_userActivity("App nap disabled for page due to user activity")
>     , m_userInterfaceLayoutDirection(parameters.userInterfaceLayoutDirection)
>     , m_overrideContentSecurityPolicy { parameters.overrideContentSecurityPolicy }
>     , m_cpuLimit(parameters.cpuLimit)
>@@ -765,22 +764,15 @@ void WebPage::updateThrottleState()
> {
>     bool isActive = m_activityState.containsAny({ ActivityState::IsLoading, ActivityState::IsAudible, ActivityState::IsCapturingMedia, ActivityState::WindowIsActive });
>     bool isVisuallyIdle = m_activityState.contains(ActivityState::IsVisuallyIdle);
>-    bool pageSuppressed = m_processSuppressionEnabled && !isActive && isVisuallyIdle;
> 
>-    // The UserActivity keeps the processes runnable. So if the page should be suppressed, stop the activity.
>-    // If the page should not be supressed, start it.
>-    if (pageSuppressed)
>-        m_userActivityHysteresis.stop();
>-    else
>-        m_userActivityHysteresis.start();
>-}
>+    bool shouldAllowAppNapGivenPageState = m_isAppNapEnabled && !isActive && isVisuallyIdle;
> 
>-void WebPage::updateUserActivity()
>-{
>-    if (m_userActivityHysteresis.state() == PAL::HysteresisState::Started)
>-        m_userActivity.start();
>-    else
>+    // The UserActivity prevents App Nap. So if we want to allow App Nap of the page, stop the activity.
>+    // If the page should not be app nap'd, start it.
>+    if (shouldAllowAppNapGivenPageState)
>         m_userActivity.stop();
>+    else
>+        m_userActivity.start();
> }
> 
> WebPage::~WebPage()
>@@ -2662,7 +2654,7 @@ void WebPage::mouseEvent(const WebMouseEvent& mouseEvent)
> {
>     SetForScope<bool> userIsInteractingChange { m_userIsInteracting, true };
> 
>-    m_userActivityHysteresis.impulse();
>+    m_userActivity.impulse();
> 
>     bool shouldHandleEvent = true;
> 
>@@ -2710,7 +2702,7 @@ static bool handleWheelEvent(const WebWheelEvent& wheelEvent, Page* page)
> 
> void WebPage::wheelEvent(const WebWheelEvent& wheelEvent)
> {
>-    m_userActivityHysteresis.impulse();
>+    m_userActivity.impulse();
> 
>     CurrentEvent currentEvent(wheelEvent);
> 
>@@ -2733,7 +2725,7 @@ void WebPage::keyEvent(const WebKeyboardEvent& keyboardEvent)
> {
>     SetForScope<bool> userIsInteractingChange { m_userIsInteracting, true };
> 
>-    m_userActivityHysteresis.impulse();
>+    m_userActivity.impulse();
> 
>     PlatformKeyboardEvent::setCurrentModifierState(platform(keyboardEvent).modifiers());
> 
>@@ -3519,9 +3511,9 @@ void WebPage::updatePreferences(const WebPreferencesStore& store)
>     else if (!store.getBoolValueForKey(WebPreferencesKey::privateBrowsingEnabledKey()) && sessionID() == PAL::SessionID::legacyPrivateSessionID())
>         setSessionID(PAL::SessionID::defaultSessionID());
> 
>-    bool processSuppressionEnabled = store.getBoolValueForKey(WebPreferencesKey::pageVisibilityBasedProcessSuppressionEnabledKey());
>-    if (m_processSuppressionEnabled != processSuppressionEnabled) {
>-        m_processSuppressionEnabled = processSuppressionEnabled;
>+    bool isAppNapEnabled = store.getBoolValueForKey(WebPreferencesKey::pageVisibilityBasedProcessSuppressionEnabledKey());
>+    if (m_isAppNapEnabled != isAppNapEnabled) {
>+        m_isAppNapEnabled = isAppNapEnabled;
>         updateThrottleState();
>     }
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index c93903f7bb31f4c9390445c86806eed9ceb93cad..fbd6af27725da56bbf34b0d95d048d79ec317148 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -1191,7 +1191,6 @@ private:
>     WebPage(uint64_t pageID, WebPageCreationParameters&&);
> 
>     void updateThrottleState();
>-    void updateUserActivity();
> 
>     // IPC::MessageSender
>     IPC::Connection* messageSenderConnection() const override;
>@@ -1854,9 +1853,8 @@ private:
> 
>     OptionSet<WebCore::ActivityState::Flag> m_activityState;
> 
>-    bool m_processSuppressionEnabled;
>+    bool m_isAppNapEnabled;
>     UserActivity m_userActivity;
>-    PAL::HysteresisActivity m_userActivityHysteresis;
> 
>     uint64_t m_pendingNavigationID { 0 };
>     Optional<WebsitePoliciesData> m_pendingWebsitePolicies;

Actions: View | Formatted Diff | Diff

Attachments on bug 197674: 369326 | 369327 | 369329 | 369334 | 369345