Generalize and refactor DeviceSensorEvent* architecture to support multi-event type targets.

Source/modules/battery/BatteryManager.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 "modules/battery/BatteryManager.h"
 
 #include "RuntimeEnabledFeatures.h"
 #include "modules/battery/BatteryDispatcher.h"
 #include "modules/battery/BatteryStatus.h"
 #include <limits>
 
 namespace WebCore {
 
 PassRefPtrWillBeRawPtr<BatteryManager> BatteryManager::create(ExecutionContext* context)
 {
     RefPtrWillBeRawPtr<BatteryManager> batteryManager(adoptRefWillBeRefCountedGarbageCollected(new BatteryManager(context)));
     batteryManager->suspendIfNeeded();
     return batteryManager.release();
 }
 
 BatteryManager::~BatteryManager()
 {
 }
 
 BatteryManager::BatteryManager(ExecutionContext* context)
     : ActiveDOMObject(context)
-    , DeviceSensorEventController(toDocument(context)->page())
+    , DeviceEventControllerBase(toDocument(context)->page())
 {
     m_hasEventListener = true;
+    m_batteryStatus = BatteryStatus::create();

[Inactive, 2014/06/03 13:49:33]

This could be part of the initializer list to avoid calling RefPtr's default
constructor unnecessarily.

[timvolodine, 2014/06/03 18:45:54]

Done.

     startUpdating();
 }
 
 bool BatteryManager::charging()
 {
-    if (const BatteryStatus* lastData = BatteryDispatcher::instance().getLatestData())
-        return lastData->charging();
-
-    return true;
+    return m_batteryStatus->charging();
 }
 
 double BatteryManager::chargingTime()
 {
-    if (const BatteryStatus* lastData = BatteryDispatcher::instance().getLatestData())
-        return lastData->chargingTime();
-
-    return 0;
+    return m_batteryStatus->chargingTime();
 }
 
 double BatteryManager::dischargingTime()
 {
-    if (const BatteryStatus* lastData = BatteryDispatcher::instance().getLatestData())
-        return lastData->dischargingTime();
-
-    return std::numeric_limits<double>::infinity();
+    return m_batteryStatus->dischargingTime();
 }
 
 double BatteryManager::level()
 {
-    if (const BatteryStatus* lastData = BatteryDispatcher::instance().getLatestData())
-        return lastData->level();
-
-    return 1;
+    return m_batteryStatus->level();
 }
 
-void BatteryManager::didChangeBatteryStatus(PassRefPtrWillBeRawPtr<Event> event)
+void BatteryManager::didUpdateData()
 {
     ASSERT(RuntimeEnabledFeatures::batteryStatusEnabled());
 
-    dispatchEvent(event);
+    RefPtr<BatteryStatus> oldStatus = m_batteryStatus;
+    m_batteryStatus = BatteryDispatcher::instance().getLatestData();

[Inactive, 2014/06/03 13:49:33]

It is OK to assign a raw ptr to a RefPtr so this should be OK if latestData()
returns a BatteryStatus*.

[timvolodine, 2014/06/03 18:45:54]

hmm, as I wrote previously the idea is to share the same object between
BatteryManager and BatteryDispatcher. If assigning a raw pointer here would wrap
and increase its ref-count that'll work. I am not sure what the preferred way is
to do this, e.g. SVGPathSegList::getItem seems to return a PassRefPtr in such
case..

[Inactive, 2014/06/03 19:04:23]

I believe SVGPathSegList::getItem() is a bad example. I still believe we should
return a raw pointer and assign it to a RefPtr here. It will properly increase
the refcount, this is not a problem.

+
+    Document* document = toDocument(executionContext());
+    if (document->activeDOMObjectsAreSuspended() || document->activeDOMObjectsAreStopped())
+        return;
+
+    ASSERT(oldStatus);
+
+    if (m_batteryStatus->charging() != oldStatus->charging())
+        dispatchEvent(Event::create(EventTypeNames::chargingchange));
+    if (m_batteryStatus->chargingTime() != oldStatus->chargingTime())
+        dispatchEvent(Event::create(EventTypeNames::chargingtimechange));
+    if (m_batteryStatus->dischargingTime() != oldStatus->dischargingTime())
+        dispatchEvent(Event::create(EventTypeNames::dischargingtimechange));
+    if (m_batteryStatus->level() != oldStatus->level())
+        dispatchEvent(Event::create(EventTypeNames::levelchange));
 }
 
 void BatteryManager::registerWithDispatcher()
 {
-    BatteryDispatcher::instance().addClient(this);
+    BatteryDispatcher::instance().addController(this);
 }
 
 void BatteryManager::unregisterWithDispatcher()
 {
-    BatteryDispatcher::instance().removeClient(this);
+    BatteryDispatcher::instance().removeController(this);
 }
 
-bool BatteryManager::hasLastData()
+bool BatteryManager::isInvokeInitialUpdate()

[Inactive, 2014/06/03 13:49:33]

This naming seems a bit weird to me but I am not a native English speaker. Does
this mean "isInvokedDuringInitialUpdate?

[timvolodine, 2014/06/03 18:45:54]

what I meant is: if true this class should perform a one-shot update of the data
upon startUpdating(). Not sure how to name it better at this point.

[Inactive, 2014/06/03 19:04:23]

What was wrong with the previous hasLastData() naming then? It seems clearer to
me.

 {
-    return false;
-}
-
-PassRefPtrWillBeRawPtr<Event> BatteryManager::getLastEvent()
-{
-    // Events are dispached via BatteryManager::didChangeBatteryStatus()
-    return nullptr;
-}
-
-bool BatteryManager::isNullEvent(Event*)
-{
-    return false;
-}
-
-Document* BatteryManager::document()
-{
-    return toDocument(executionContext());
+    return BatteryDispatcher::instance().getLatestData();
 }
 
 void BatteryManager::suspend()
 {
     m_hasEventListener = false;
     stopUpdating();
 }
 
 void BatteryManager::resume()
 {
     m_hasEventListener = true;
     startUpdating();
 }
 
 void BatteryManager::stop()
 {
     m_hasEventListener = false;
     stopUpdating();
 }
 
 } // namespace WebCore