Change HysteresisActivity to use a lambda
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Dec 2014 19:37:33 +0000 (19:37 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Dec 2014 19:37:33 +0000 (19:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139636

Reviewed by Darin Adler.

The current implementation provides notifications via callbacks to a delegate. Using a delegate
with callbacks is limiting a number of ways. The callback names are very ambiguous, the callbacks
must either be on a separate object (more boilerplate), or the callback must be public for
HysteresisActivity to call them, or friends get involved. Without the boilerplate of extra
delegate objects it's hard to scale use of these objects – a single object can't serve as a
delegate for multiple HysteresisActivity members.

Instead, make HysteresisActivity take a lambda to callback on state change. To simplify, changed
HysteresisState to only track Started/Stopped states (removed WillStopPendingTimeout).

Source/WebCore:

* WebCore.exp.in:
    - removed exports of deleted functions.
* page/PageThrottler.cpp:
(WebCore::PageThrottler::PageThrottler):
    - m_hysteresis lambda calls updateUserActivity.
(WebCore::PageThrottler::pageActivityCounterValueDidChange):
    - ASSERT updated due to removal of WillStopPendingTimeout state.
(WebCore::PageThrottler::started): Deleted.
(WebCore::PageThrottler::stopped): Deleted.
    - functionality replaced by lambda.
* page/PageThrottler.h:
    - HysteresisActivity is no longer templated on delegate type, removed function declarations & friend.
* platform/HysteresisActivity.h:
(WebCore::HysteresisActivity::HysteresisActivity):
    - HysteresisActivity takes a lambda, not a delegate.
(WebCore::HysteresisActivity::start):
    - delegate call -> callback.
(WebCore::HysteresisActivity::state):
    - simplified to remove WillStopPendingTimeout.
(WebCore::HysteresisActivity::hysteresisTimerFired):
    - delegate call -> callback.
* platform/UserActivity.cpp:
(WebCore::UserActivity::UserActivity):
    - HysteresisActivity lambda calls hysteresisUpdated.
(WebCore::UserActivity::hysteresisUpdated):
(WebCore::UserActivity::started): Deleted.
(WebCore::UserActivity::stopped): Deleted.
    - started/stopped -> hysteresisUpdated.
* platform/UserActivity.h:
    - started/stopped -> hysteresisUpdated, removed friend.

Source/WebKit2:

* WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.cpp:
(WebKit::WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker):
    - HysteresisActivity now takes a lambda, not a delegate.
(WebKit::WebSQLiteDatabaseTracker::hysteresisUpdated):
(WebKit::WebSQLiteDatabaseTracker::started): Deleted.
(WebKit::WebSQLiteDatabaseTracker::stopped): Deleted.
    - started/stopped merged into hysteresisUpdated
* WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.h:
    - HysteresisActivity is no longer templated on delegate type, changed function declarations.

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

Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/page/PageThrottler.cpp
Source/WebCore/page/PageThrottler.h
Source/WebCore/platform/HysteresisActivity.h
Source/WebCore/platform/UserActivity.cpp
Source/WebCore/platform/UserActivity.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.h

index 792de4481e4a73ab079a07b35efdb2bba8bee9fd..8a4d290940e1d7a64988b77b8ebf64f5ca20b29a 100644 (file)
@@ -1,3 +1,51 @@
+2014-12-15  Gavin Barraclough  <barraclough@apple.com>
+
+        Change HysteresisActivity to use a lambda
+        https://bugs.webkit.org/show_bug.cgi?id=139636
+
+        Reviewed by Darin Adler.
+
+        The current implementation provides notifications via callbacks to a delegate. Using a delegate
+        with callbacks is limiting a number of ways. The callback names are very ambiguous, the callbacks
+        must either be on a separate object (more boilerplate), or the callback must be public for
+        HysteresisActivity to call them, or friends get involved. Without the boilerplate of extra
+        delegate objects it's hard to scale use of these objects – a single object can't serve as a
+        delegate for multiple HysteresisActivity members.
+
+        Instead, make HysteresisActivity take a lambda to callback on state change. To simplify, changed
+        HysteresisState to only track Started/Stopped states (removed WillStopPendingTimeout).
+
+        * WebCore.exp.in:
+            - removed exports of deleted functions.
+        * page/PageThrottler.cpp:
+        (WebCore::PageThrottler::PageThrottler):
+            - m_hysteresis lambda calls updateUserActivity.
+        (WebCore::PageThrottler::pageActivityCounterValueDidChange):
+            - ASSERT updated due to removal of WillStopPendingTimeout state.
+        (WebCore::PageThrottler::started): Deleted.
+        (WebCore::PageThrottler::stopped): Deleted.
+            - functionality replaced by lambda.
+        * page/PageThrottler.h:
+            - HysteresisActivity is no longer templated on delegate type, removed function declarations & friend.
+        * platform/HysteresisActivity.h:
+        (WebCore::HysteresisActivity::HysteresisActivity):
+            - HysteresisActivity takes a lambda, not a delegate.
+        (WebCore::HysteresisActivity::start):
+            - delegate call -> callback.
+        (WebCore::HysteresisActivity::state):
+            - simplified to remove WillStopPendingTimeout.
+        (WebCore::HysteresisActivity::hysteresisTimerFired):
+            - delegate call -> callback.
+        * platform/UserActivity.cpp:
+        (WebCore::UserActivity::UserActivity):
+            - HysteresisActivity lambda calls hysteresisUpdated.
+        (WebCore::UserActivity::hysteresisUpdated):
+        (WebCore::UserActivity::started): Deleted.
+        (WebCore::UserActivity::stopped): Deleted.
+            - started/stopped -> hysteresisUpdated.
+        * platform/UserActivity.h:
+            - started/stopped -> hysteresisUpdated, removed friend.
+
 2014-12-15  Antti Koivisto  <antti@apple.com>
 
         WebKit level persistent caching
index 44c2eef73ee0e3b6984bc7e62b7425baaa9ce408..606be94d20d82d5bf4317574afcabbce582465f9 100644 (file)
@@ -270,7 +270,6 @@ __ZN7WebCore12TextIterator8subrangeEPNS_5RangeEii
 __ZN7WebCore12TextIteratorC1EPKNS_5RangeEt
 __ZN7WebCore12TextIteratorD1Ev
 __ZN7WebCore12UTF8EncodingEv
-__ZN7WebCore12UserActivity7startedEv
 __ZN7WebCore12UserActivityC1EPKc
 __ZN7WebCore12WorkerThread17workerThreadCountEv
 __ZN7WebCore12blobRegistryEv
@@ -329,7 +328,6 @@ __ZN7WebCore13KeyboardEventC1Ev
 __ZN7WebCore13NodeTraversal13deepLastChildEPNS_4NodeE
 __ZN7WebCore13NodeTraversal19nextAncestorSiblingEPKNS_4NodeE
 __ZN7WebCore13NodeTraversal19nextAncestorSiblingEPKNS_4NodeES3_
-__ZN7WebCore13PageThrottler7startedEv
 __ZN7WebCore13ResourceErrorC1EP7NSError
 __ZN7WebCore13ResourceErrorC1EP9__CFError
 __ZN7WebCore13SQLResultDoneE
index 1106ee64e18ca9f51828c75fe6cf74b353330f1c..dd210181e85b1f1ac0bb125a7e30058ec39be4d1 100644 (file)
@@ -30,7 +30,7 @@ namespace WebCore {
 
 PageThrottler::PageThrottler(ViewState::Flags viewState)
     : m_viewState(viewState)
-    , m_hysteresis(*this)
+    , m_hysteresis([this](HysteresisState) { updateUserActivity(); })
     , m_pageActivityCounter([this]() { pageActivityCounterValueDidChange(); })
 {
     updateUserActivity();
@@ -60,8 +60,8 @@ void PageThrottler::pageActivityCounterValueDidChange()
     else
         m_hysteresis.stop();
 
-    // If the counter is nonzero, state must be Started; if the counter is zero, state may be Waiting or Stopped.
-    ASSERT(!!m_pageActivityCounter.value() == (m_hysteresis.state() == HysteresisState::Started));
+    // If the counter is nonzero, state cannot be Stopped.
+    ASSERT(!(m_pageActivityCounter.value() && m_hysteresis.state() == HysteresisState::Stopped));
 }
 
 void PageThrottler::updateUserActivity()
@@ -85,14 +85,4 @@ void PageThrottler::setViewState(ViewState::Flags viewState)
         updateUserActivity();
 }
 
-void PageThrottler::started()
-{
-    updateUserActivity();
-}
-
-void PageThrottler::stopped()
-{
-    updateUserActivity();
-}
-
 }
index d6aa3f786cfea8119772a61abd3d5443fb9fb0d8..0fca46beec84dbc51b5ecc80b22e61b08dbae8dd 100644 (file)
@@ -51,15 +51,10 @@ public:
 
 private:
     void pageActivityCounterValueDidChange();
-
     void updateUserActivity();
 
-    friend class HysteresisActivity<PageThrottler>;
-    WEBCORE_EXPORT void started();
-    void stopped();
-
     ViewState::Flags m_viewState;
-    HysteresisActivity<PageThrottler> m_hysteresis;
+    HysteresisActivity m_hysteresis;
     std::unique_ptr<UserActivity::Impl> m_activity;
     RefCounter m_pageActivityCounter;
 };
index a3c922f6de973a762a02bb5de00f04cb49bf40b4..128ef8209e2a748309e062820edc823980c0b2e1 100644 (file)
@@ -34,18 +34,16 @@ static const double DefaultHysteresisSeconds = 5.0;
 
 enum class HysteresisState {
     Started,
-    WillStopPendingTimeout,
     Stopped
 };
 
-template<typename Delegate>
 class HysteresisActivity {
 public:
-    explicit HysteresisActivity(Delegate& delegate, double hysteresisSeconds = DefaultHysteresisSeconds)
-        : m_delegate(delegate)
+    explicit HysteresisActivity(std::function<void(HysteresisState)> callback = [](HysteresisState) { }, double hysteresisSeconds = DefaultHysteresisSeconds)
+        : m_callback(callback)
         , m_hysteresisSeconds(hysteresisSeconds)
         , m_active(false)
-        , m_timer(*this, &HysteresisActivity<Delegate>::hysteresisTimerFired)
+        , m_timer(*this, &HysteresisActivity::hysteresisTimerFired)
     {
     }
 
@@ -58,7 +56,7 @@ public:
         if (m_timer.isActive())
             m_timer.stop();
         else
-            m_delegate.started();
+            m_callback(HysteresisState::Started);
     }
 
     void stop()
@@ -80,21 +78,17 @@ public:
 
     HysteresisState state() const
     {
-        if (m_active)
-            return HysteresisState::Started;
-        if (m_timer.isActive())
-            return HysteresisState::WillStopPendingTimeout;
-        return HysteresisState::Stopped;
+        return m_active || m_timer.isActive() ? HysteresisState::Started : HysteresisState::Stopped;
     }
     
 private:
     void hysteresisTimerFired()
     {
-        m_delegate.stopped();
         m_timer.stop();
+        m_callback(HysteresisState::Stopped);
     }
 
-    Delegate& m_delegate;
+    std::function<void(HysteresisState)> m_callback;
     double m_hysteresisSeconds;
     bool m_active;
     Timer m_timer;
index ec42cdf20414a231f4b64b4fd755d7f516bb6d96..3bcb2f3da7609a3fe7388c6e8c9df6cfc37df87a 100644 (file)
@@ -45,19 +45,17 @@ void UserActivity::Impl::endActivity()
 #endif
 
 UserActivity::UserActivity(const char* description)
-    : HysteresisActivity<UserActivity>(*this)
+    : HysteresisActivity([this](HysteresisState state) { hysteresisUpdated(state); })
     , m_impl(description)
 {
 }
 
-void UserActivity::started()
+void UserActivity::hysteresisUpdated(HysteresisState state)
 {
-    m_impl.beginActivity();
-}
-
-void UserActivity::stopped()
-{
-    m_impl.endActivity();
+    if (state == HysteresisState::Started)
+        m_impl.beginActivity();
+    else
+        m_impl.endActivity();
 }
 
 } // namespace WebCore
index 0b35fe846da3a54e9389edb63841e1375a476e6f..35813b8727bcc2634cd1d60b1468946b93453fe1 100644 (file)
@@ -39,7 +39,7 @@ namespace WebCore {
 // The UserActivity type is used to indicate to the operating system that
 // a user initiated or visible action is taking place, and as such that
 // resources should be allocated to the process accordingly.
-class UserActivity : public HysteresisActivity<UserActivity> {
+class UserActivity : public HysteresisActivity {
 public:
     class Impl {
     public:
@@ -58,10 +58,7 @@ public:
     WEBCORE_EXPORT explicit UserActivity(const char* description);
 
 private:
-    friend class HysteresisActivity<UserActivity>;
-
-    WEBCORE_EXPORT void started();
-    void stopped();
+    void hysteresisUpdated(HysteresisState);
 
     Impl m_impl;
 };
index cf2c657a2f35e6d043b8f30be8809d43434cf403..5d6aec906ef539c55f49ccf8eeebea76daeae9f0 100644 (file)
@@ -1,3 +1,30 @@
+2014-12-15  Gavin Barraclough  <barraclough@apple.com>
+
+        Change HysteresisActivity to use a lambda
+        https://bugs.webkit.org/show_bug.cgi?id=139636
+
+        Reviewed by Darin Adler.
+
+        The current implementation provides notifications via callbacks to a delegate. Using a delegate
+        with callbacks is limiting a number of ways. The callback names are very ambiguous, the callbacks
+        must either be on a separate object (more boilerplate), or the callback must be public for
+        HysteresisActivity to call them, or friends get involved. Without the boilerplate of extra
+        delegate objects it's hard to scale use of these objects – a single object can't serve as a
+        delegate for multiple HysteresisActivity members.
+
+        Instead, make HysteresisActivity take a lambda to callback on state change. To simplify, changed
+        HysteresisState to only track Started/Stopped states (removed WillStopPendingTimeout).
+
+        * WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.cpp:
+        (WebKit::WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker):
+            - HysteresisActivity now takes a lambda, not a delegate.
+        (WebKit::WebSQLiteDatabaseTracker::hysteresisUpdated):
+        (WebKit::WebSQLiteDatabaseTracker::started): Deleted.
+        (WebKit::WebSQLiteDatabaseTracker::stopped): Deleted.
+            - started/stopped merged into hysteresisUpdated
+        * WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.h:
+            - HysteresisActivity is no longer templated on delegate type, changed function declarations.
+
 2014-12-15  Antti Koivisto  <antti@apple.com>
 
         WebKit level persistent caching
index 9e680b5d9b160f24b624d63456627ce821c7a586..8b4a6f00d0dc7e81058f45fd81e6a42f7e52294c 100644 (file)
@@ -44,7 +44,7 @@ const char* WebSQLiteDatabaseTracker::supplementName()
 
 WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker(WebProcess* process)
     : m_process(process)
-    , m_hysteresis(*this)
+    , m_hysteresis([this](HysteresisState state) { hysteresisUpdated(state); })
 {
 }
 
@@ -67,14 +67,9 @@ void WebSQLiteDatabaseTracker::didFinishLastTransaction()
     });
 }
 
-void WebSQLiteDatabaseTracker::started()
+void WebSQLiteDatabaseTracker::hysteresisUpdated(HysteresisState state)
 {
-    m_process->parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(true), 0);
-}
-
-void WebSQLiteDatabaseTracker::stopped()
-{
-    m_process->parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(false), 0);
+    m_process->parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(state == HysteresisState::Started), 0);
 }
 
 } // namespace WebKit
index 37dd8905bb4af1edbb2c6998c78e077eeb2771c8..fa0d904ae8d6aa759fdab30ee848422f28418519 100644 (file)
@@ -52,13 +52,10 @@ public:
     virtual void didFinishLastTransaction() override;
 
 private:
-    // WebCore::HysteresisActivity
-    friend class WebCore::HysteresisActivity<WebSQLiteDatabaseTracker>;
-    void started();
-    void stopped();
+    void hysteresisUpdated(WebCore::HysteresisState);
 
     WebProcess* m_process;
-    WebCore::HysteresisActivity<WebSQLiteDatabaseTracker> m_hysteresis;
+    WebCore::HysteresisActivity m_hysteresis;
 };
 
 } // namespace WebKit