[webkit-changes] [177295] trunk/Source

Mon, 15 Dec 2014 11:38:09 -0800

Title: [177295] trunk/Source
Revision
177295
Author
[email protected]
Date
2014-12-15 11:37:33 -0800 (Mon, 15 Dec 2014)

Log Message

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).

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (177294 => 177295)


--- trunk/Source/WebCore/ChangeLog	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebCore/ChangeLog	2014-12-15 19:37:33 UTC (rev 177295)
@@ -1,3 +1,51 @@
+2014-12-15  Gavin Barraclough  <[email protected]>
+
+        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  <[email protected]>
 
         WebKit level persistent caching

Modified: trunk/Source/WebCore/WebCore.exp.in (177294 => 177295)


--- trunk/Source/WebCore/WebCore.exp.in	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebCore/WebCore.exp.in	2014-12-15 19:37:33 UTC (rev 177295)
@@ -270,7 +270,6 @@
 __ZN7WebCore12TextIteratorC1EPKNS_5RangeEt
 __ZN7WebCore12TextIteratorD1Ev
 __ZN7WebCore12UTF8EncodingEv
-__ZN7WebCore12UserActivity7startedEv
 __ZN7WebCore12UserActivityC1EPKc
 __ZN7WebCore12WorkerThread17workerThreadCountEv
 __ZN7WebCore12blobRegistryEv
@@ -329,7 +328,6 @@
 __ZN7WebCore13NodeTraversal13deepLastChildEPNS_4NodeE
 __ZN7WebCore13NodeTraversal19nextAncestorSiblingEPKNS_4NodeE
 __ZN7WebCore13NodeTraversal19nextAncestorSiblingEPKNS_4NodeES3_
-__ZN7WebCore13PageThrottler7startedEv
 __ZN7WebCore13ResourceErrorC1EP7NSError
 __ZN7WebCore13ResourceErrorC1EP9__CFError
 __ZN7WebCore13SQLResultDoneE

Modified: trunk/Source/WebCore/page/PageThrottler.cpp (177294 => 177295)


--- trunk/Source/WebCore/page/PageThrottler.cpp	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebCore/page/PageThrottler.cpp	2014-12-15 19:37:33 UTC (rev 177295)
@@ -30,7 +30,7 @@
 
 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 @@
     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 @@
         updateUserActivity();
 }
 
-void PageThrottler::started()
-{
-    updateUserActivity();
 }
-
-void PageThrottler::stopped()
-{
-    updateUserActivity();
-}
-
-}

Modified: trunk/Source/WebCore/page/PageThrottler.h (177294 => 177295)


--- trunk/Source/WebCore/page/PageThrottler.h	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebCore/page/PageThrottler.h	2014-12-15 19:37:33 UTC (rev 177295)
@@ -51,15 +51,10 @@
 
 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;
 };

Modified: trunk/Source/WebCore/platform/HysteresisActivity.h (177294 => 177295)


--- trunk/Source/WebCore/platform/HysteresisActivity.h	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebCore/platform/HysteresisActivity.h	2014-12-15 19:37:33 UTC (rev 177295)
@@ -34,18 +34,16 @@
 
 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 @@
         if (m_timer.isActive())
             m_timer.stop();
         else
-            m_delegate.started();
+            m_callback(HysteresisState::Started);
     }
 
     void stop()
@@ -80,21 +78,17 @@
 
     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;

Modified: trunk/Source/WebCore/platform/UserActivity.cpp (177294 => 177295)


--- trunk/Source/WebCore/platform/UserActivity.cpp	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebCore/platform/UserActivity.cpp	2014-12-15 19:37:33 UTC (rev 177295)
@@ -45,19 +45,17 @@
 #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();
+    if (state == HysteresisState::Started)
+        m_impl.beginActivity();
+    else
+        m_impl.endActivity();
 }
 
-void UserActivity::stopped()
-{
-    m_impl.endActivity();
-}
-
 } // namespace WebCore

Modified: trunk/Source/WebCore/platform/UserActivity.h (177294 => 177295)


--- trunk/Source/WebCore/platform/UserActivity.h	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebCore/platform/UserActivity.h	2014-12-15 19:37:33 UTC (rev 177295)
@@ -39,7 +39,7 @@
 // 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,11 +58,8 @@
     WEBCORE_EXPORT explicit UserActivity(const char* description);
 
 private:
-    friend class HysteresisActivity<UserActivity>;
+    void hysteresisUpdated(HysteresisState);
 
-    WEBCORE_EXPORT void started();
-    void stopped();
-
     Impl m_impl;
 };

Modified: trunk/Source/WebKit2/ChangeLog (177294 => 177295)


--- trunk/Source/WebKit2/ChangeLog	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebKit2/ChangeLog	2014-12-15 19:37:33 UTC (rev 177295)
@@ -1,3 +1,30 @@
+2014-12-15  Gavin Barraclough  <[email protected]>
+
+        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  <[email protected]>
 
         WebKit level persistent caching

Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.cpp (177294 => 177295)


--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.cpp	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.cpp	2014-12-15 19:37:33 UTC (rev 177295)
@@ -44,7 +44,7 @@
 
 WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker(WebProcess* process)
     : m_process(process)
-    , m_hysteresis(*this)
+    , m_hysteresis([this](HysteresisState state) { hysteresisUpdated(state); })
 {
 }
 
@@ -67,16 +67,11 @@
     });
 }
 
-void WebSQLiteDatabaseTracker::started()
+void WebSQLiteDatabaseTracker::hysteresisUpdated(HysteresisState state)
 {
-    m_process->parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(true), 0);
+    m_process->parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(state == HysteresisState::Started), 0);
 }
 
-void WebSQLiteDatabaseTracker::stopped()
-{
-    m_process->parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(false), 0);
-}
-
 } // namespace WebKit
 
 #endif // ENABLE(SQL_DATABASE)

Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.h (177294 => 177295)


--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.h	2014-12-15 19:25:23 UTC (rev 177294)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebSQLiteDatabaseTracker.h	2014-12-15 19:37:33 UTC (rev 177295)
@@ -52,13 +52,10 @@
     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

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to