Exclude page visibility from PageThrottler's hysteresis
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Sep 2014 16:32:10 +0000 (16:32 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Sep 2014 16:32:10 +0000 (16:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136866

Reviewed by Geoff Garen.

Including visibility in the hysteresis mechanism has the effect of prolonging the visually
idle timeout, and causing the page hide event to be run at foreground priority. Neither of
these are particularly desirable. Instead separate visibility from the rest of the page
activities we track (and apply hysteresis to), and feed this directly into determination
of the UserActivity state.

* page/PageThrottler.cpp:
(WebCore::PageThrottler::PageThrottler):
    - when the PageThrottler is instantiated call updateUserActivity to set initial UserActivity.
(WebCore::PageThrottler::incrementActivityCount):
    - simplified - when m_activityCount becomes non-zero, start m_hysteresis.
(WebCore::PageThrottler::decrementActivityCount):
    - simplified - when m_activityCount becomes zero, stop m_hysteresis.
(WebCore::PageThrottler::updateUserActivity):
    - end the UserActivity (allow AppNap) if visually idle and no page activity is taking place.
(WebCore::PageThrottler::setViewState):
    - when the visually idle state changed call updateUserActivity to update the UserActivity.
(WebCore::PageThrottler::started):
(WebCore::PageThrottler::stopped):
    - when the hysteresis state changed call updateUserActivity to update the UserActivity.
(WebCore::PageThrottler::updateHysteresis): Deleted.
    - removed: simplified the hysteresis trigger, we now incorporate visually idle state in updateUserActivity.
* page/PageThrottler.h:
    - removed updateHysteresis, added updateUserActivity.
* platform/HysteresisActivity.h:
(WebCore::HysteresisActivity::state):
    - determine the curent state of the HysteresisActivity - started, waiting, or stopped.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173693 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/PageThrottler.cpp
Source/WebCore/page/PageThrottler.h
Source/WebCore/platform/HysteresisActivity.h

index 571495c..479379b 100644 (file)
@@ -1,3 +1,38 @@
+2014-09-16  Gavin Barraclough  <baraclough@apple.com>
+
+        Exclude page visibility from PageThrottler's hysteresis
+        https://bugs.webkit.org/show_bug.cgi?id=136866
+
+        Reviewed by Geoff Garen.
+
+        Including visibility in the hysteresis mechanism has the effect of prolonging the visually
+        idle timeout, and causing the page hide event to be run at foreground priority. Neither of
+        these are particularly desirable. Instead separate visibility from the rest of the page
+        activities we track (and apply hysteresis to), and feed this directly into determination
+        of the UserActivity state.
+
+        * page/PageThrottler.cpp:
+        (WebCore::PageThrottler::PageThrottler):
+            - when the PageThrottler is instantiated call updateUserActivity to set initial UserActivity.
+        (WebCore::PageThrottler::incrementActivityCount):
+            - simplified - when m_activityCount becomes non-zero, start m_hysteresis.
+        (WebCore::PageThrottler::decrementActivityCount):
+            - simplified - when m_activityCount becomes zero, stop m_hysteresis.
+        (WebCore::PageThrottler::updateUserActivity):
+            - end the UserActivity (allow AppNap) if visually idle and no page activity is taking place.
+        (WebCore::PageThrottler::setViewState):
+            - when the visually idle state changed call updateUserActivity to update the UserActivity.
+        (WebCore::PageThrottler::started):
+        (WebCore::PageThrottler::stopped):
+            - when the hysteresis state changed call updateUserActivity to update the UserActivity.
+        (WebCore::PageThrottler::updateHysteresis): Deleted.
+            - removed: simplified the hysteresis trigger, we now incorporate visually idle state in updateUserActivity.
+        * page/PageThrottler.h:
+            - removed updateHysteresis, added updateUserActivity.
+        * platform/HysteresisActivity.h:
+        (WebCore::HysteresisActivity::state):
+            - determine the curent state of the HysteresisActivity - started, waiting, or stopped.
+
 2014-09-17  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Fix runtime critical warnings when writing to the clipboard after r173687
index 8fda65b..f7892ac 100644 (file)
 #include "config.h"
 #include "PageThrottler.h"
 
-#include "Chrome.h"
-#include "ChromeClient.h"
-#include "MainFrame.h"
-#include "Page.h"
 #include "PageActivityAssertionToken.h"
-#include <wtf/StdLibExtras.h>
 
 namespace WebCore {
 
@@ -43,8 +38,7 @@ PageThrottler::PageThrottler(Page& page, ViewState::Flags viewState)
     , m_activity("Page is active.")
     , m_activityCount(0)
 {
-    if (!(m_viewState & ViewState::IsVisuallyIdle))
-        m_hysteresis.start();
+    updateUserActivity();
 }
 
 std::unique_ptr<PageActivityAssertionToken> PageThrottler::mediaActivityToken()
@@ -59,22 +53,33 @@ std::unique_ptr<PageActivityAssertionToken> PageThrottler::pageLoadActivityToken
 
 void PageThrottler::incrementActivityCount()
 {
-    ++m_activityCount;
-    updateHysteresis();
+    // If m_activityCount is nonzero, state must be Started; if m_activityCount is zero, state may be Waiting or Stopped.
+    ASSERT(!!m_activityCount == (m_hysteresis.state() == HysteresisState::Started));
+
+    if (!m_activityCount++)
+        m_hysteresis.start();
+
+    ASSERT(m_activityCount && m_hysteresis.state() == HysteresisState::Started);
 }
 
 void PageThrottler::decrementActivityCount()
 {
-    --m_activityCount;
-    updateHysteresis();
+    ASSERT(m_activityCount && m_hysteresis.state() == HysteresisState::Started);
+
+    if (!--m_activityCount)
+        m_hysteresis.stop();
+
+    // If m_activityCount is nonzero, state must be Started; if m_activityCount is zero, state may be Waiting or Stopped.
+    ASSERT(!!m_activityCount == (m_hysteresis.state() == HysteresisState::Started));
 }
 
-void PageThrottler::updateHysteresis()
+void PageThrottler::updateUserActivity()
 {
-    if (m_activityCount || !(m_viewState & ViewState::IsVisuallyIdle))
-        m_hysteresis.start();
+    // Allow throttling if there is no page activity, and the page is visually idle.
+    if (m_hysteresis.state() == HysteresisState::Stopped && m_viewState & ViewState::IsVisuallyIdle)
+        m_activity.endActivity();
     else
-        m_hysteresis.stop();
+        m_activity.beginActivity();
 }
 
 void PageThrottler::setViewState(ViewState::Flags viewState)
@@ -83,17 +88,17 @@ void PageThrottler::setViewState(ViewState::Flags viewState)
     m_viewState = viewState;
 
     if (changed & ViewState::IsVisuallyIdle)
-        updateHysteresis();
+        updateUserActivity();
 }
 
 void PageThrottler::started()
 {
-    m_activity.beginActivity();
+    updateUserActivity();
 }
 
 void PageThrottler::stopped()
 {
-    m_activity.endActivity();
+    updateUserActivity();
 }
 
 }
index 385afa0..bf387fe 100644 (file)
@@ -30,7 +30,6 @@
 
 #include "UserActivity.h"
 #include "ViewState.h"
-#include <wtf/HashSet.h>
 #include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -56,7 +55,7 @@ private:
     void incrementActivityCount();
     void decrementActivityCount();
 
-    void updateHysteresis();
+    void updateUserActivity();
 
     friend class HysteresisActivity<PageThrottler>;
     WEBCORE_EXPORT void started();
index c7a6841..b882636 100644 (file)
@@ -32,6 +32,12 @@ namespace WebCore {
 
 static const double DefaultHysteresisSeconds = 5.0;
 
+enum class HysteresisState {
+    Started,
+    WillStopPendingTimeout,
+    Stopped
+};
+
 template<typename Delegate>
 class HysteresisActivity {
 public:
@@ -72,6 +78,15 @@ public:
         }
     }
 
+    HysteresisState state() const
+    {
+        if (m_active)
+            return HysteresisState::Started;
+        if (m_timer.isActive())
+            return HysteresisState::WillStopPendingTimeout;
+        return HysteresisState::Stopped;
+    }
+    
 private:
     void hysteresisTimerFired(Timer<HysteresisActivity>&)
     {