Move PlatformEventDispatcher implementations to the Oilpan heap.

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)
 {
 }
 
-PlatformEventDispatcher::~PlatformEventDispatcher()
-{
-}
-
 void PlatformEventDispatcher::addController(PlatformEventController* controller)
 {
     bool wasEmpty = m_controllers.isEmpty();
     if (!m_controllers.contains(controller))
         m_controllers.append(controller);
     if (wasEmpty)
         startListening();
 }
 
 void PlatformEventDispatcher::removeController(PlatformEventController* controller)

[haraken, 2014/10/03 01:55:33]

We need to make sure that removeController() is never called in a destructor of
on-heap objects.

Currently ~BatteryManager calls stopUpdating() and it calls in
removeController().

[sof, 2014/10/03 06:34:04]

The singleton dispatcher instances are held by persistents, so why is this a
problem?

[haraken, 2014/10/03 10:10:56]

ah, you're right.

However, it looks a bit inconsistent that only ~BatteryManager calls
stopUpdating() while other PlatormEventControllers don't call stopUpdating() in
~BatteryManager.

Probably can we just remove the stopUpdating() in ~BatteryManager? Even if we
don't call it, the BatteryManager will be removed from the m_controllers by the
weak processing.

[sof, 2014/10/03 10:27:11]

With Oilpan enabled, it is redundant, but not without Oilpan - no weak
processing is done.

We could make that clear with ENABLE(OILPAN) usage, if that's preferable.

[sof, 2014/10/03 12:21:16]

Went ahead and added such.

The Oilpan-specific call to stopUpdating() needed in ~NavigatorGamepad was
removed entirely (it was originally added for reliable DOMWindowProperty
finalization.)

 {
     // 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;
@@ skipping 34 matching lines @@
         for (size_t i = 0; i < size; ++i) {
             if (m_controllers[i])
                 m_controllers[i]->didUpdateData();
         }
     }
 
     if (m_needsPurge)
         purgeControllers();
 }
 
+void PlatformEventDispatcher::trace(Visitor* visitor)
+{
+    visitor->trace(m_controllers);

[haraken, 2014/10/03 01:55:33]

Why do you trace m_controllers? If we trace m_controllers, it will strongly mark
all the controllers. Then clearWeakMembers will never run.

[sof, 2014/10/03 05:05:29]

It's a HeapVector<> of PlatformEventController* - that will mark the vector
backing store allocation, but not the elements. 

[haraken, 2014/10/03 05:36:01]

Makes sense. Shall we add a comment?

[sof, 2014/10/03 06:34:04]

Done. 

I incorrectly assumed that PlatformEventControllers were all on the heap by
default; they're not, m_controllers needs to be a WillBeHeapVector<> as a
result.

[haraken, 2014/10/03 10:10:56]

Can we move DeviceSingleWindowEventController, NavigatorGamepad and 
ScreenOrientationController to the heap at the same time?

It's confusing to use WillBeHeapVector because not all of
PlatformEventControllers are on the heap.

[sof, 2014/10/03 10:27:11]

NavigatorGamepad is a navigator supplement, so we cannot move it without also
having Navigator be on the heap always(*). Ditto for ScreenOrientationController
and LocalFrame.

* - assuming we don't want to reintroduce PersistentHeapSupplementables<>, which
I don't think we want to..?

+    visitor->registerWeakMembers<PlatformEventDispatcher, &PlatformEventDispatcher::clearWeakMembers>(this);
+}
+
+void PlatformEventDispatcher::clearWeakMembers(Visitor* visitor)
+{
+    for (size_t i = 0; i < m_controllers.size(); ++i) {
+        if (!visitor->isAlive(m_controllers[i])) {
+            m_controllers[i] = nullptr;
+            m_needsPurge = true;
+        }
+    }
+    // Next notification will purge the empty slots.
+}
+
 } // namespace blink