Replace Chrome IPC with Mojo IPC for querying BatteryStatus service

device/battery/battery_status_service.cc

 // 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 "content/browser/battery_status/battery_status_service.h"
+#include "device/battery/battery_status_service.h"
 
 #include "base/bind.h"
-#include "content/browser/battery_status/battery_status_manager.h"
-#include "content/public/browser/browser_thread.h"
+#include "base/message_loop/message_loop.h"
+#include "base/single_thread_task_runner.h"
+#include "device/battery/battery_monitor_impl.h"
+#include "device/battery/battery_status_manager.h"
 
-namespace content {
+namespace device {
 
 BatteryStatusService::BatteryStatusService()
-    : update_callback_(base::Bind(&BatteryStatusService::UpdateBatteryStatus,
+    : main_thread_task_runner_(base::MessageLoop::current()->task_runner()),
+      update_callback_(base::Bind(&BatteryStatusService::NotifyConsumers,
                                   base::Unretained(this))),
       status_updated_(false),
       is_shutdown_(false) {
   callback_list_.set_removal_callback(
       base::Bind(&BatteryStatusService::ConsumersChanged,
                  base::Unretained(this)));
 }
 
 BatteryStatusService::~BatteryStatusService() {
 }
 
 BatteryStatusService* BatteryStatusService::GetInstance() {
   return Singleton<BatteryStatusService,
                    LeakySingletonTraits<BatteryStatusService> >::get();
 }
 
 scoped_ptr<BatteryStatusService::BatteryUpdateSubscription>
 BatteryStatusService::AddCallback(const BatteryUpdateCallback& callback) {

[timvolodine, 2014/09/30 16:26:01]

how about adding a check that this only runs on the right thread.
since you have the main_thread_task_runner_ I think it can be simply done with
"BelongsToCurrentThread()" DCHECK.

[ppi, 2014/10/06 13:50:02]

Done.

-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(!is_shutdown_);
 
   if (!battery_fetcher_)
     battery_fetcher_ = BatteryStatusManager::Create(update_callback_);
 
   if (callback_list_.empty()) {
     bool success = battery_fetcher_->StartListeningBatteryChange();
-    if (!success) {
-        // Make sure the promise resolves with the default values in Blink.
-        callback.Run(blink::WebBatteryStatus());
-    }
+    // On failure pass the default values back.
+    if (!success)
+      callback.Run(BatteryStatus());
   }
 
   if (status_updated_) {
     // Send recent status to the new callback if already available.
     callback.Run(status_);
   }
 
   return callback_list_.Add(callback);
 }
 
 void BatteryStatusService::ConsumersChanged() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(!is_shutdown_);
+  if (is_shutdown_)
+    return;
 
   if (callback_list_.empty()) {
-      battery_fetcher_->StopListeningBatteryChange();
-      status_updated_ = false;
+    battery_fetcher_->StopListeningBatteryChange();
+    status_updated_ = false;
   }
 }
 
-void BatteryStatusService::UpdateBatteryStatus(
-    const blink::WebBatteryStatus& status) {
+void BatteryStatusService::NotifyConsumers(const BatteryStatus& status) {
   DCHECK(!is_shutdown_);
-  BrowserThread::PostTask(BrowserThread::IO,
-                          FROM_HERE,
-                          base::Bind(&BatteryStatusService::NotifyConsumers,
-                                     base::Unretained(this), status));
+
+  main_thread_task_runner_->PostTask(FROM_HERE, base::Bind(
+      &BatteryStatusService::NotifyConsumersOnMainThread,
+      base::Unretained(this),
+      status));
 }
 
-void BatteryStatusService::NotifyConsumers(
-    const blink::WebBatteryStatus& status) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+void BatteryStatusService::NotifyConsumersOnMainThread(
+    const BatteryStatus& status) {

[timvolodine, 2014/09/30 16:26:02]

also I think we need DCHECK here that this always runs on main thread.

[ppi, 2014/10/06 13:50:02]

Done.

+  status_ = status;

[timvolodine, 2014/09/30 16:26:01]

I think these two lines should come after the if(callback_list_.empty()) check.
Notify() can be executed after there are no more consumers anymore and in that
case status will be updated so that on next start a stale value will be
returned.

[ppi, 2014/10/06 13:50:02]

Done.

+  status_updated_ = true;
 
   if (callback_list_.empty())
     return;
 
-  status_ = status;
-  status_updated_ = true;
-  callback_list_.Notify(status);
+  callback_list_.Notify(status_);
 }
 
 void BatteryStatusService::Shutdown() {
   if (!callback_list_.empty())
     battery_fetcher_->StopListeningBatteryChange();
   battery_fetcher_.reset();
   is_shutdown_ = true;
 }
 
 const BatteryStatusService::BatteryUpdateCallback&
 BatteryStatusService::GetUpdateCallbackForTesting() const {
   return update_callback_;
 }
 
 void BatteryStatusService::SetBatteryManagerForTesting(
-    BatteryStatusManager* test_battery_manager) {
-  battery_fetcher_.reset(test_battery_manager);
-  blink::WebBatteryStatus status;
-  status_ = status;
+    scoped_ptr<BatteryStatusManager> test_battery_manager) {
+  battery_fetcher_ = test_battery_manager.Pass();
+  status_ = BatteryStatus();
   status_updated_ = false;
 }
 
-}  // namespace content
+}  // namespace device