[Oilpan] Remove a raw pointer to PlatformEventController class on Oilpan build

Source/core/frame/PlatformEventDispatcher.cpp

Index: Source/core/frame/PlatformEventDispatcher.cpp
diff --git a/Source/core/frame/PlatformEventDispatcher.cpp b/Source/core/frame/PlatformEventDispatcher.cpp
index b588df26190466b88d871cbc12c42ad46145b881..ef830041074bd43b1bae495015b1d10981c5ea2a 100644
--- a/Source/core/frame/PlatformEventDispatcher.cpp
+++ b/Source/core/frame/PlatformEventDispatcher.cpp
@@ -11,95 +11,62 @@
 namespace blink {
 
 PlatformEventDispatcher::PlatformEventDispatcher()
-    : m_needsPurge(false)
-    , m_isDispatching(false)
+    : m_isDispatching(false)
+    , m_isListening(false)
 {
 }
 
 void PlatformEventDispatcher::addController(PlatformEventController* controller)
 {
-    bool wasEmpty = m_controllers.isEmpty();
-    if (!m_controllers.contains(controller))
-        m_controllers.append(controller);
-    if (wasEmpty)
+    ASSERT(controller);
+    ASSERT(!m_controllers.contains(controller));

[timvolodine, 2015/08/27 12:31:53]

nit: looks like it's fine, because we add the controller upon initial promise
resolution, but was initially wondering if the assert could fail when we add the
same event listener twice (e.g. battery.addEventListener('levelchange',..)).

[peria, 2015/08/27 12:52:39]

Yes, this ASSERT assumes that one controller is not added twice.
PlatformEventController::startUpdating() seems to avoid such a case, though.

+
+    m_controllers.add(controller);
+    if (!m_isListening) {
         startListening();
+        m_isListening = true;
+    }
 }
 
 void PlatformEventDispatcher::removeController(PlatformEventController* controller)
 {
-    // Do not actually remove the controller from the vector, instead zero them out.
-    // The zeros are removed in these two cases:
-    // 1. either immediately if we are not dispatching any events,
-    // 2. or after events to all controllers have dispatched (see notifyControllers()).
-    // This is to correctly handle the re-entrancy case when a controller is destroyed
-    // while the events are still being dispatched.
-    size_t index = m_controllers.find(controller);
-    if (index == kNotFound)
-        return;
-
-    m_controllers[index] = nullptr;
-    m_needsPurge = true;
+    ASSERT(m_controllers.contains(controller));
 
-    if (!m_isDispatching)
-        purgeControllers();
-}
-
-void PlatformEventDispatcher::purgeControllers()
-{
-    ASSERT(m_needsPurge);
-
-    size_t i = 0;
-    while (i < m_controllers.size()) {
-        if (!m_controllers[i]) {
-            m_controllers[i] = m_controllers.last();
-            m_controllers.removeLast();
-        } else {
-            ++i;
-        }
-    }
-
-    m_needsPurge = false;
-
-    if (m_controllers.isEmpty())
+    m_controllers.remove(controller);
+    if (!m_isDispatching && m_controllers.isEmpty()) {
         stopListening();
+        m_isListening = false;
+    }
 }
 
 void PlatformEventDispatcher::notifyControllers()
 {
+    if (m_controllers.isEmpty())
+        return;
+
     {
         TemporaryChange<bool> changeIsDispatching(m_isDispatching, true);
-        // Don't notify controllers removed or added during event dispatch.
-        size_t size = m_controllers.size();
-        for (size_t i = 0; i < size; ++i) {
-            if (m_controllers[i])
-                m_controllers[i]->didUpdateData();
+        // HashSet m_controllers can be updated during an iteration, and it stops the iteration.
+        // Thus we store it into a Vector to access all elements.
+        WillBeHeapVector<RawPtrWillBeMember<PlatformEventController>> snapshotVector;
+        copyToVector(m_controllers, snapshotVector);
+        for (PlatformEventController* controller : snapshotVector) {
+            if (m_controllers.contains(controller))
+                controller->didUpdateData();
         }
     }
 
-    if (m_needsPurge)
-        purgeControllers();
+    if (m_controllers.isEmpty()) {
+        stopListening();
+        m_isListening = false;
+    }
 }
 
 DEFINE_TRACE(PlatformEventDispatcher)
 {
 #if ENABLE(OILPAN)
-    // Trace the backing store, the weak(&bare) element references won't be.
     visitor->trace(m_controllers);
-    visitor->template registerWeakMembers<PlatformEventDispatcher, &PlatformEventDispatcher::clearWeakMembers>(this);
 #endif
 }
 
-#if ENABLE(OILPAN)
-void PlatformEventDispatcher::clearWeakMembers(Visitor* visitor)
-{
-    for (size_t i = 0; i < m_controllers.size(); ++i) {
-        if (!Heap::isHeapObjectAlive(m_controllers[i])) {
-            m_controllers[i] = nullptr;
-            m_needsPurge = true;
-        }
-    }
-    // Next notification will purge the empty slots.
-}
-#endif
-
 } // namespace blink