Title: [197178] trunk
Revision
197178
Author
[email protected]
Date
2016-02-26 10:20:07 -0800 (Fri, 26 Feb 2016)

Log Message

Source/WebCore:
RefCounter value changed callback should be called on all changes (not just zero edge).
https://bugs.webkit.org/show_bug.cgi?id=154699

Reviewed by Anders Carlsson.

RefCounter currently only triggers a callback when the count goes from zero
to non-zero and vice-versa. Change that, to be useful to more clients.

* page/PageThrottler.cpp:
(WebCore::PageThrottler::PageThrottler):
    - Updated for change in RefCounter callback siganture.
* platform/VNodeTracker.cpp:
(WebCore::VNodeTracker::VNodeTracker):
    - Can now use RefCounter callback to trigger checkPressureState().
(WebCore::VNodeTracker::pressureWarningTimerFired):
    - RefCounter count is now a size_t (%d -> %ul).
* platform/VNodeTracker.h:
    - simplified VNodeTracker::token() [no longer needs to call checkPressureState()].

Source/WebKit2:
RefCounter value changed callback should be called on all changes (not just zero edge).
https://bugs.webkit.org/show_bug.cgi?id=154699

Reviewed by Anders Carlsson.

RefCounter currently only triggers a callback when the count goes from zero
to non-zero and vice-versa. Change that, to be useful to more clients.

* UIProcess/Plugins/PluginProcessManager.cpp:
(WebKit::PluginProcessManager::PluginProcessManager):
    - Updated for change in RefCounter callback siganture.
* UIProcess/Plugins/PluginProcessManager.h:
    - Updated for change in RefCounter callback siganture.
* UIProcess/Plugins/mac/PluginProcessManagerMac.mm:
(WebKit::PluginProcessManager::updateProcessSuppressionDisabled):
    - updated logic for enabling process supression.
* UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::ProcessThrottler):
    - Updated for change in RefCounter callback siganture.
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::WebProcessPool):
    - Updated for change in RefCounter callback siganture.

Source/WTF:
Unreviewed, rolling out r197168.
https://bugs.webkit.org/show_bug.cgi?id=154728

crashing on some devices (Requested by kling on #webkit).

Reverted changeset:

"[Darwin] Use vm_kernel_page_size for WTF::pageSize()."
https://bugs.webkit.org/show_bug.cgi?id=154726
http://trac.webkit.org/changeset/197168

Patch by Commit Queue <[email protected]> on 2016-02-26

Tools:
RefCounter value changed callback should be called on all changes (not just zero edge).
https://bugs.webkit.org/show_bug.cgi?id=154699

Reviewed by Geoff Garen.

RefCounter currently only triggers a callback when the count goes from zero
to non-zero and vice-versa. Change that, to be useful to more clients.

* TestWebKitAPI/Tests/WTF/RefCounter.cpp:
(TestWebKitAPI::TEST):
    - Updated for change in RefCounter callback siganture & behaviour.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (197177 => 197178)


--- trunk/Source/WTF/ChangeLog	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WTF/ChangeLog	2016-02-26 18:20:07 UTC (rev 197178)
@@ -27,6 +27,28 @@
 
 2016-02-25  Gavin Barraclough  <[email protected]>
 
+        RefCounter value changed callback should be called on all changes (not just zero edge).
+        https://bugs.webkit.org/show_bug.cgi?id=154699
+
+        Reviewed by Anders Carlsson.
+
+        RefCounter currently only triggers a callback when the count goes from zero
+        to non-zero and vice-versa. Change that, to be useful to more clients.
+
+        * wtf/RefCounter.h:
+        (WTF::RefCounter::Count::Count):
+        (WTF::RefCounter::RefCounter):
+            - Removed superfluous WTF_EXPORT_PRIVATE.
+        (WTF::RefCounter::value):
+            - Changed value() to a size_t.
+        (WTF::RefCounter<T>::Count::ref):
+        (WTF::RefCounter<T>::Count::deref):
+            - Trigger the callback on all increments/decrements.
+        (WTF::RefCounter<T>::RefCounter):
+            - Changed siganture of callback.
+
+2016-02-25  Gavin Barraclough  <[email protected]>
+
         Replace RefCounter::Token implementation with RefPtr
         https://bugs.webkit.org/show_bug.cgi?id=154698
 

Modified: trunk/Source/WTF/wtf/RefCounter.h (197177 => 197178)


--- trunk/Source/WTF/wtf/RefCounter.h	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WTF/wtf/RefCounter.h	2016-02-26 18:20:07 UTC (rev 197178)
@@ -52,13 +52,15 @@
         }
 
         RefCounter* m_refCounter;
-        unsigned m_value;
+        size_t m_value;
     };
 
 public:
     using Token = RefPtr<Count>;
+    enum class Event { Decrement, Increment };
+    using ValueChangeFunction = std::function<void (Event)>;
 
-    RefCounter(std::function<void(bool)> = [](bool) { });
+    RefCounter(ValueChangeFunction = nullptr);
     ~RefCounter();
 
     Token count() const
@@ -66,47 +68,43 @@
         return m_count;
     }
 
-    unsigned value() const
+    size_t value() const
     {
         return m_count->m_value;
     }
 
 private:
-    std::function<void(bool)> m_valueDidChange;
+    ValueChangeFunction m_valueDidChange;
     Count* m_count;
 };
 
 template<typename T>
 inline void RefCounter<T>::Count::ref()
 {
-    bool valueWasZero = !m_value;
     ++m_value;
-    
-    if (valueWasZero && m_refCounter)
-        m_refCounter->m_valueDidChange(true);
+    if (m_refCounter && m_refCounter->m_valueDidChange)
+        m_refCounter->m_valueDidChange(Event::Increment);
 }
 
 template<typename T>
 inline void RefCounter<T>::Count::deref()
 {
     ASSERT(m_value);
+
     --m_value;
+    if (m_refCounter && m_refCounter->m_valueDidChange)
+        m_refCounter->m_valueDidChange(Event::Decrement);
 
-    if (m_value)
-        return;
-
     // The Count object is kept alive so long as either the RefCounter that created it remains
     // allocated, or so long as its reference count is non-zero.
     // If the RefCounter has already been deallocted then delete the Count when its reference
     // count reaches zero.
-    if (m_refCounter)
-        m_refCounter->m_valueDidChange(false);
-    else
+    if (!m_refCounter && !m_value)
         delete this;
 }
 
 template<typename T>
-inline RefCounter<T>::RefCounter(std::function<void(bool)> valueDidChange)
+inline RefCounter<T>::RefCounter(ValueChangeFunction valueDidChange)
     : m_valueDidChange(valueDidChange)
     , m_count(new Count(*this))
 {

Modified: trunk/Source/WebCore/ChangeLog (197177 => 197178)


--- trunk/Source/WebCore/ChangeLog	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebCore/ChangeLog	2016-02-26 18:20:07 UTC (rev 197178)
@@ -1,3 +1,24 @@
+2016-02-25  Gavin Barraclough  <[email protected]>
+
+        RefCounter value changed callback should be called on all changes (not just zero edge).
+        https://bugs.webkit.org/show_bug.cgi?id=154699
+
+        Reviewed by Anders Carlsson.
+
+        RefCounter currently only triggers a callback when the count goes from zero
+        to non-zero and vice-versa. Change that, to be useful to more clients.
+
+        * page/PageThrottler.cpp:
+        (WebCore::PageThrottler::PageThrottler):
+            - Updated for change in RefCounter callback siganture.
+        * platform/VNodeTracker.cpp:
+        (WebCore::VNodeTracker::VNodeTracker):
+            - Can now use RefCounter callback to trigger checkPressureState().
+        (WebCore::VNodeTracker::pressureWarningTimerFired):
+            - RefCounter count is now a size_t (%d -> %ul).
+        * platform/VNodeTracker.h:
+            - simplified VNodeTracker::token() [no longer needs to call checkPressureState()].
+
 2016-02-26  Andreas Kling  <[email protected]>
 
         Remove unused CFNetwork disk cache mmap optimization in WebKit2.

Modified: trunk/Source/WebCore/page/PageThrottler.cpp (197177 => 197178)


--- trunk/Source/WebCore/page/PageThrottler.cpp	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebCore/page/PageThrottler.cpp	2016-02-26 18:20:07 UTC (rev 197178)
@@ -34,8 +34,8 @@
     : m_page(page)
     , m_userInputHysteresis([this](HysteresisState state) { setActivityFlag(PageActivityState::UserInputActivity, state == HysteresisState::Started); })
     , m_audiblePluginHysteresis([this](HysteresisState state) { setActivityFlag(PageActivityState::AudiblePlugin, state == HysteresisState::Started); })
-    , m_mediaActivityCounter([this](bool value) { setActivityFlag(PageActivityState::MediaActivity, value); })
-    , m_pageLoadActivityCounter([this](bool value) { setActivityFlag(PageActivityState::PageLoadActivity, value); })
+    , m_mediaActivityCounter([this](PageActivityCounter::Event) { setActivityFlag(PageActivityState::MediaActivity, m_mediaActivityCounter.value()); })
+    , m_pageLoadActivityCounter([this](PageActivityCounter::Event) { setActivityFlag(PageActivityState::PageLoadActivity, m_pageLoadActivityCounter.value()); })
 {
 }
 

Modified: trunk/Source/WebCore/platform/VNodeTracker.cpp (197177 => 197178)


--- trunk/Source/WebCore/platform/VNodeTracker.cpp	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebCore/platform/VNodeTracker.cpp	2016-02-26 18:20:07 UTC (rev 197178)
@@ -39,7 +39,8 @@
 }
 
 VNodeTracker::VNodeTracker()
-    : m_pressureWarningTimer(*this, &VNodeTracker::pressureWarningTimerFired)
+    : m_vnodeCounter([this](VNodeCounter::Event event) { if (event == VNodeCounter::Event::Increment) checkPressureState(); })
+    , m_pressureWarningTimer(*this, &VNodeTracker::pressureWarningTimerFired)
     , m_lastWarningTime(std::chrono::steady_clock::now())
 {
     platformInitialize();
@@ -67,7 +68,7 @@
     unsigned vnodeCount = m_vnodeCounter.value();
     auto critical = vnodeCount > m_hardVNodeLimit ? Critical::Yes : Critical::No;
     m_pressureHandler(critical);
-    LOG(MemoryPressure, "vnode pressure handler freed %d vnodes out of %u (critical pressure: %s)", vnodeCount - m_vnodeCounter.value(), vnodeCount, critical == Critical::Yes ? "Yes" : "No");
+    LOG(MemoryPressure, "vnode pressure handler freed %lu vnodes out of %u (critical pressure: %s)", vnodeCount - m_vnodeCounter.value(), vnodeCount, critical == Critical::Yes ? "Yes" : "No");
 }
 
 std::chrono::milliseconds VNodeTracker::nextPressureWarningInterval() const

Modified: trunk/Source/WebCore/platform/VNodeTracker.h (197177 => 197178)


--- trunk/Source/WebCore/platform/VNodeTracker.h	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebCore/platform/VNodeTracker.h	2016-02-26 18:20:07 UTC (rev 197178)
@@ -72,12 +72,7 @@
 
 inline auto VNodeTracker::token() -> Token
 {
-    if (!m_pressureHandler)
-        return Token();
-
-    Token token(m_vnodeCounter.count());
-    checkPressureState();
-    return token;
+    return m_pressureHandler ? m_vnodeCounter.count() : Token();
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebKit2/ChangeLog (197177 => 197178)


--- trunk/Source/WebKit2/ChangeLog	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebKit2/ChangeLog	2016-02-26 18:20:07 UTC (rev 197178)
@@ -1,3 +1,28 @@
+2016-02-25  Gavin Barraclough  <[email protected]>
+
+        RefCounter value changed callback should be called on all changes (not just zero edge).
+        https://bugs.webkit.org/show_bug.cgi?id=154699
+
+        Reviewed by Anders Carlsson.
+
+        RefCounter currently only triggers a callback when the count goes from zero
+        to non-zero and vice-versa. Change that, to be useful to more clients.
+
+        * UIProcess/Plugins/PluginProcessManager.cpp:
+        (WebKit::PluginProcessManager::PluginProcessManager):
+            - Updated for change in RefCounter callback siganture.
+        * UIProcess/Plugins/PluginProcessManager.h:
+            - Updated for change in RefCounter callback siganture.
+        * UIProcess/Plugins/mac/PluginProcessManagerMac.mm:
+        (WebKit::PluginProcessManager::updateProcessSuppressionDisabled):
+            - updated logic for enabling process supression.
+        * UIProcess/ProcessThrottler.cpp:
+        (WebKit::ProcessThrottler::ProcessThrottler):
+            - Updated for change in RefCounter callback siganture.
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::WebProcessPool):
+            - Updated for change in RefCounter callback siganture.
+
 2016-02-26  Andreas Kling  <[email protected]>
 
         Remove unused CFNetwork disk cache mmap optimization in WebKit2.

Modified: trunk/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp (197177 => 197178)


--- trunk/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp	2016-02-26 18:20:07 UTC (rev 197178)
@@ -43,7 +43,7 @@
 
 PluginProcessManager::PluginProcessManager()
 #if PLATFORM(COCOA)
-    : m_processSuppressionDisabledForPageCounter([this](bool value) { updateProcessSuppressionDisabled(value); })
+    : m_processSuppressionDisabledForPageCounter([this](ProcessSuppressionDisabledCounter::Event event) { updateProcessSuppressionDisabled(event); })
 #endif
 {
 }

Modified: trunk/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h (197177 => 197178)


--- trunk/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h	2016-02-26 18:20:07 UTC (rev 197178)
@@ -68,7 +68,7 @@
 #if PLATFORM(COCOA)
     inline ProcessSuppressionDisabledToken processSuppressionDisabledToken();
     inline bool processSuppressionDisabled() const;
-    void updateProcessSuppressionDisabled(bool);
+    void updateProcessSuppressionDisabled(ProcessSuppressionDisabledCounter::Event);
 #endif
 
 private:

Modified: trunk/Source/WebKit2/UIProcess/Plugins/mac/PluginProcessManagerMac.mm (197177 => 197178)


--- trunk/Source/WebKit2/UIProcess/Plugins/mac/PluginProcessManagerMac.mm	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebKit2/UIProcess/Plugins/mac/PluginProcessManagerMac.mm	2016-02-26 18:20:07 UTC (rev 197178)
@@ -32,10 +32,17 @@
 
 namespace WebKit {
 
-void PluginProcessManager::updateProcessSuppressionDisabled(bool disabled)
+void PluginProcessManager::updateProcessSuppressionDisabled(ProcessSuppressionDisabledCounter::Event event)
 {
-    bool enabled = !disabled;
+    size_t disableCount = m_processSuppressionDisabledForPageCounter.value();
 
+    // We only care about zero/non-zero edge changes; ignore cases where the count was previously non-zero, and still is.
+    if (disableCount >= 2 || (disableCount == 1 && event == ProcessSuppressionDisabledCounter::Event::Decrement))
+        return;
+    ASSERT((event == ProcessSuppressionDisabledCounter::Event::Increment && disableCount == 1)
+        || (event == ProcessSuppressionDisabledCounter::Event::Decrement && !disableCount));
+
+    bool enabled = !disableCount;
     for (auto& pluginProcess : m_pluginProcesses)
         pluginProcess->setProcessSuppressionEnabled(enabled);
 }

Modified: trunk/Source/WebKit2/UIProcess/ProcessThrottler.cpp (197177 => 197178)


--- trunk/Source/WebKit2/UIProcess/ProcessThrottler.cpp	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebKit2/UIProcess/ProcessThrottler.cpp	2016-02-26 18:20:07 UTC (rev 197178)
@@ -35,8 +35,8 @@
 ProcessThrottler::ProcessThrottler(ProcessThrottlerClient& process)
     : m_process(process)
     , m_suspendTimer(RunLoop::main(), this, &ProcessThrottler::suspendTimerFired)
-    , m_foregroundCounter([this](bool) { updateAssertion(); })
-    , m_backgroundCounter([this](bool) { updateAssertion(); })
+    , m_foregroundCounter([this](ForegroundActivityCounter::Event) { updateAssertion(); })
+    , m_backgroundCounter([this](BackgroundActivityCounter::Event) { updateAssertion(); })
     , m_suspendMessageCount(0)
 {
 }

Modified: trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp (197177 => 197178)


--- trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp	2016-02-26 18:20:07 UTC (rev 197178)
@@ -158,8 +158,8 @@
     , m_canHandleHTTPSServerTrustEvaluation(true)
     , m_didNetworkProcessCrash(false)
     , m_memoryCacheDisabled(false)
-    , m_userObservablePageCounter([this](bool) { updateProcessSuppressionState(); })
-    , m_processSuppressionDisabledForPageCounter([this](bool) { updateProcessSuppressionState(); })
+    , m_userObservablePageCounter([this](UserObservablePageCounter::Event) { updateProcessSuppressionState(); })
+    , m_processSuppressionDisabledForPageCounter([this](ProcessSuppressionDisabledCounter::Event) { updateProcessSuppressionState(); })
 {
     for (auto& scheme : m_configuration->alwaysRevalidatedURLSchemes())
         m_schemesToRegisterAsAlwaysRevalidated.add(scheme);

Modified: trunk/Tools/ChangeLog (197177 => 197178)


--- trunk/Tools/ChangeLog	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Tools/ChangeLog	2016-02-26 18:20:07 UTC (rev 197178)
@@ -1,3 +1,17 @@
+2016-02-25  Gavin Barraclough  <[email protected]>
+
+        RefCounter value changed callback should be called on all changes (not just zero edge).
+        https://bugs.webkit.org/show_bug.cgi?id=154699
+
+        Reviewed by Geoff Garen.
+
+        RefCounter currently only triggers a callback when the count goes from zero
+        to non-zero and vice-versa. Change that, to be useful to more clients.
+
+        * TestWebKitAPI/Tests/WTF/RefCounter.cpp:
+        (TestWebKitAPI::TEST):
+            - Updated for change in RefCounter callback siganture & behaviour.
+
 2016-02-25  Sam Weinig  <[email protected]>
 
         Allow WKUserScripts to be run in isolated worlds

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp (197177 => 197178)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp	2016-02-26 18:17:14 UTC (rev 197177)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/RefCounter.cpp	2016-02-26 18:20:07 UTC (rev 197178)
@@ -31,7 +31,8 @@
 
 namespace TestWebKitAPI {
 
-static const int CallbackExpected = 0xC0FFEE;
+static const int IncrementExpected = 0xC0FFEE1;
+static const int DecrementExpected = 0xC0FFEE2;
 static const int CallbackNotExpected = 0xDECAF;
 
 enum TestCounterType { };
@@ -79,54 +80,64 @@
     {
         // Testing (1a) - Construction with a callback.
         TestCounter* counterPtr = nullptr;
-        TestCounter counter([&](bool value) {
+        TestCounter counter([&](TestCounter::Event event) {
             // Check that the callback is called at the expected times, and the correct number of times.
-            EXPECT_EQ(callbackValue, CallbackExpected);
-            // Value provided should be equal to the counter value.
-            EXPECT_EQ(value, counterPtr->value());
+            if (TestCounter::Event::Increment == event)
+                EXPECT_EQ(callbackValue, IncrementExpected);
+            if (TestCounter::Event::Decrement == event)
+                EXPECT_EQ(callbackValue, DecrementExpected);
             // return the value of the counter in the callback.
-            callbackValue = value;
+            callbackValue = counterPtr->value();
         });
         counterPtr = &counter;
         // Testing (4a) - after construction value() is 0.
         EXPECT_EQ(0, static_cast<int>(counter.value()));
 
         // Testing (3a) - ref with callback from 0 -> 1.
-        callbackValue = CallbackExpected;
+        callbackValue = IncrementExpected;
         TokenType incTo1(counter.count());
         // Testing (4b) & (4c) - values within & after callback.
-        EXPECT_EQ(true, callbackValue);
+        EXPECT_EQ(1, callbackValue);
         EXPECT_EQ(1, static_cast<int>(counter.value()));
 
         // Testing (3b) - ref with callback from 1 -> 2.
+        callbackValue = IncrementExpected;
         TokenType incTo2(incTo1);
         // Testing (4b) & (4c) - values within & after callback.
+        EXPECT_EQ(2, callbackValue);
         EXPECT_EQ(2, static_cast<int>(counter.value()));
 
         // Testing (3c) - deref with callback from >1 -> 1.
+        callbackValue = DecrementExpected;
         incTo1 = nullptr;
         // Testing (4b) & (4c) - values within & after callback.
+        EXPECT_EQ(1, callbackValue);
         EXPECT_EQ(1, static_cast<int>(counter.value()));
 
         {
             // Testing (3j) - ref using a Ref rather than a RefPtr.
+            callbackValue = IncrementExpected;
             TokenType incTo2Again(counter.count());
             // Testing (4b) & (4c) - values within & after callback.
+            EXPECT_EQ(2, callbackValue);
             EXPECT_EQ(2, static_cast<int>(counter.value()));
             // Testing (3k) - deref using a Ref rather than a RefPtr.
+
+            callbackValue = DecrementExpected;
         }
+        EXPECT_EQ(1, callbackValue);
         EXPECT_EQ(1, static_cast<int>(counter.value()));
         // Testing (4b) & (4c) - values within & after callback.
 
         // Testing (3d) - deref with callback from 1 -> 0.
-        callbackValue = CallbackExpected;
+        callbackValue = DecrementExpected;
         incTo2 = nullptr;
         // Testing (4b) & (4c) - values within & after callback.
         EXPECT_EQ(0, callbackValue);
         EXPECT_EQ(0, static_cast<int>(counter.value()));
 
         // Testing (2a) - Destruction where the TestCounter::Count has a non-zero reference count.
-        callbackValue = CallbackExpected;
+        callbackValue = IncrementExpected;
         incTo1Again = counter.count();
         EXPECT_EQ(1, callbackValue);
         EXPECT_EQ(1, static_cast<int>(counter.value()));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to