Attachment 369345 Details for Bug 197674 – Patch

[patch] Patch

bug-197674-20190507180158.patch (text/plain), 6.00 KB, created by Chris Dumez on 2019-05-07 18:02:00 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-05-07 18:02:00 PDT

Size: 6.00 KB

patch

obsolete

>Subversion Revision: 245045
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index b1f6771bb34609313eab602e5bdd2994d96216df..c7108080e1d89a48dd26f6fd421c5ae4b869887a 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 Geoff Garen.
>+
>+        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  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r245038.
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 58ac38bd937b6273cf31e8e3ccf9a255b08587c9..8b26395a06092034187b8e6f508608f52d243f2a 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -416,9 +416,7 @@ 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_userActivity("App nap disabled for page due to user activity")
>     , m_userInterfaceLayoutDirection(parameters.userInterfaceLayoutDirection)
>     , m_overrideContentSecurityPolicy { parameters.overrideContentSecurityPolicy }
>     , m_cpuLimit(parameters.cpuLimit)
>@@ -765,22 +763,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 shouldAllowAppNap = 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 (shouldAllowAppNap)
>         m_userActivity.stop();
>+    else
>+        m_userActivity.start();
> }
> 
> WebPage::~WebPage()
>@@ -2662,7 +2653,7 @@ void WebPage::mouseEvent(const WebMouseEvent& mouseEvent)
> {
>     SetForScope<bool> userIsInteractingChange { m_userIsInteracting, true };
> 
>-    m_userActivityHysteresis.impulse();
>+    m_userActivity.impulse();
> 
>     bool shouldHandleEvent = true;
> 
>@@ -2710,7 +2701,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 +2724,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 +3510,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..90e408f9b131bbda0d0a3a3a946b9bb4fd43a493 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 { true };
>     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