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

Source/core/frame/PlatformEventDispatcher.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/frame/PlatformEventDispatcher.h"
 
 #include "core/frame/PlatformEventController.h"
 #include "wtf/TemporaryChange.h"
 
 namespace blink {
 
 PlatformEventDispatcher::PlatformEventDispatcher()
-    : m_needsPurge(false)
-    , m_isDispatching(false)
 {
 }
 
 void PlatformEventDispatcher::addController(PlatformEventController* controller)
 {
+    ASSERT(controller);
+
     bool wasEmpty = m_controllers.isEmpty();
-    if (!m_controllers.contains(controller))
-        m_controllers.append(controller);
+    m_controllers.add(controller);
     if (wasEmpty)
         startListening();
 }
 
 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.

[haraken, 2015/08/19 11:12:51]

This comment explains why we have to use a Vector instead of a HashSet.

In order to use a HashSet (I think this is a good thing), we need to add code to
protect the controller while the events are being dispatched.

[peria, 2015/08/20 09:35:50]

Ah, I understand that the main focus of this comment is the last sentence,
that means Dispatcher does not stop/restart listening events even
if they have no controllers during a dispatching.
're-entrancy' is not of controllers, but of this dispatcher.

[timvolodine, 2015/08/20 11:16:17]

yes, the main concern here was that we can execute another
addEventListener/removeEventListener inside the event callback which would
potentially modify m_controllers and have an impact on the dispatch in
notifyControllers.

-    size_t index = m_controllers.find(controller);
-    if (index == kNotFound)
+    if (!m_controllers.contains(controller))
         return;
 
-    m_controllers[index] = nullptr;
-    m_needsPurge = true;
-
-    if (!m_isDispatching)
-        purgeControllers();
+    m_controllers.remove(controller);
+    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())
         stopListening();
 }
 
 void PlatformEventDispatcher::notifyControllers()
 {
-    {
-        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();
-        }
-    }
+    // didUpdateData() may delete the object itself, and it can lead to restructure m_controllers.
+    // Thus we restore elements of m_controllers into a Vector to access all elements.
+    HeapVector<PlatformEventController*> snapshotVector;
+    for (PlatformEventController* controller : m_controllers)
+        snapshotVector.append(controller);
+    for (PlatformEventController* controller : snapshotVector)
+        controller->didUpdateData();
 
-    if (m_needsPurge)
-        purgeControllers();
+    purgeControllers();
 }
 
 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