Oilpan: move BatteryStatus to the heap.

Oilpan: move BatteryStatus to the heap.

R=haraken@chromium.org,timvolodine@chromium.org
BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176525

[sof, 2014-06-19 10:48:08]

Please take a look.

[haraken, 2014-06-19 10:59:16]

LGTM

https://codereview.chromium.org/341053002/diff/1/Source/modules/battery/Batte...
File Source/modules/battery/BatteryManager.cpp (right):

https://codereview.chromium.org/341053002/diff/1/Source/modules/battery/Batte...
Source/modules/battery/BatteryManager.cpp:23: stopUpdating();

This calls unregisterWithDispatcher(), which removes |this| object from
DeviceEventDispatcherBase::m_controllers.

In a follow-up, you might want to move the DeviceEventDispatcherBase's hierarchy
to the heap and replace the logic with weak processing (you can make
m_controllers a hash set of the weak members).

[haraken, 2014-06-19 11:00:15]

+timvolodine

[timvolodine, 2014-06-19 11:44:26]

On 2014/06/19 11:00:15, haraken wrote:
> +timvolodine

this is a transitional move right? otherwise I would suggest migrating the base
classes as well and postponing landing of this patch until Monday (i.e. after
the cut, because the battery status feature is very fresh).
lgtm

[sof, 2014-06-19 11:48:04]

On 2014/06/19 11:44:26, timvolodine wrote:
> On 2014/06/19 11:00:15, haraken wrote:
> > +timvolodine
> 
> this is a transitional move right? otherwise I would suggest migrating the
base
> classes as well and postponing landing of this patch until Monday (i.e. after
> the cut, because the battery status feature is very fresh).
> lgtm

Yes, only adding transitional types here.

[haraken, 2014-06-19 11:52:07]

Before moving DeviceEventDispatcherBase's hierarchy to the heap, you might want
to try to change DeviceEventDispatcherBase::m_controllers from
Vector<DeviceEventControllerBase*> to HashSet<DeviceEventControllerBase*>. I
guess it will enable us to remove DeviceEventDispatcherBase::purgeControllers,
which makes it easier to move m_controllers to an on-heap HashSet.

[timvolodine, 2014-06-19 12:01:08]

On 2014/06/19 11:52:07, haraken wrote:
> Before moving DeviceEventDispatcherBase's hierarchy to the heap, you might
want
> to try to change DeviceEventDispatcherBase::m_controllers from
> Vector<DeviceEventControllerBase*> to HashSet<DeviceEventControllerBase*>. I
> guess it will enable us to remove DeviceEventDispatcherBase::purgeControllers,
> which makes it easier to move m_controllers to an on-heap HashSet.

There was some logic for having the purgeControllers the way it is now. When a
controller removes itself from m_controllers we just zero it out to prevent
issues with re-entrancy, because the removal can happen during dispatch. The
for-loop on the vector simply skips zeroed entries, not sure if that's easy to
do with a HashSet..

[sof, 2014-06-19 13:57:50]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-06-19 13:58:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/341053002/1

[commit-bot: I haz the power, 2014-06-19 15:34:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-19 15:34:43]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[sof, 2014-06-19 16:44:36]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-06-19 16:45:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/341053002/1

[commit-bot: I haz the power, 2014-06-19 16:45:50]

Change committed as 176525