[BackgroundSync] Reland of PowerObserver

This is a reland of https://codereview.chromium.org/1126563002/

There is a funky test that creates and deletes a PowerMonitor before deleting a profile. This trips a BackgroundSyncPowerObserver destructor DCHECK to verify that the PowerMonitor still exists.

This CL changes the DCHECK to verify that the PowerMonitor still exists only when the BackgroundSyncPowerObserver is being used.

BUG=482053

Review URL: https://codereview.chromium.org/1162643002

Cr-Commit-Position: refs/heads/master@{#331943}

7 files changed, 273 insertions, 1 deletions

diff --git a/content/browser/background_sync/background_sync_manager.cc b/content/browser/background_sync/background_sync_manager.cc
index dd4f2f8..24398f8 100644
--- a/content/browser/background_sync/background_sync_manager.cc
+++ b/content/browser/background_sync/background_sync_manager.cc
@@ -7,6 +7,7 @@
 #include "base/barrier_closure.h"
 #include "base/bind.h"
 #include "content/browser/background_sync/background_sync_network_observer.h"
+#include "content/browser/background_sync/background_sync_power_observer.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/public/browser/browser_thread.h"
@@ -180,6 +181,8 @@ BackgroundSyncManager::BackgroundSyncManager(
   network_observer_.reset(new BackgroundSyncNetworkObserver(
       base::Bind(&BackgroundSyncManager::OnNetworkChanged,
                  weak_ptr_factory_.GetWeakPtr())));
+  power_observer_.reset(new BackgroundSyncPowerObserver(base::Bind(
+      &BackgroundSyncManager::OnPowerChanged, weak_ptr_factory_.GetWeakPtr())));
 }
 
 void BackgroundSyncManager::Init() {
@@ -639,7 +642,8 @@ bool BackgroundSyncManager::IsRegistrationReadyToFire(
 
   DCHECK_EQ(SYNC_ONE_SHOT, registration.periodicity);
 
-  return network_observer_->NetworkSufficient(registration.network_state);
+  return network_observer_->NetworkSufficient(registration.network_state) &&
+         power_observer_->PowerSufficient(registration.power_state);
 }
 
 void BackgroundSyncManager::FireReadyEvents() {
@@ -829,6 +833,12 @@ void BackgroundSyncManager::OnNetworkChanged() {
   FireReadyEvents();
 }
 
+void BackgroundSyncManager::OnPowerChanged() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  FireReadyEvents();
+}
+
 template <typename CallbackT, typename... Params>
 void BackgroundSyncManager::CompleteOperationCallback(const CallbackT& callback,
                                                       Params... parameters) {
diff --git a/content/browser/background_sync/background_sync_manager.h b/content/browser/background_sync/background_sync_manager.h
index f9c8960..5ce09b2 100644
--- a/content/browser/background_sync/background_sync_manager.h
+++ b/content/browser/background_sync/background_sync_manager.h
@@ -22,6 +22,7 @@
 namespace content {
 
 class BackgroundSyncNetworkObserver;
+class BackgroundSyncPowerObserver;
 class ServiceWorkerContextWrapper;
 
 // BackgroundSyncManager manages and stores the set of background sync
@@ -287,6 +288,7 @@ class CONTENT_EXPORT BackgroundSyncManager
   void OnStorageWipedImpl(const base::Closure& callback);
 
   void OnNetworkChanged();
+  void OnPowerChanged();
 
   // Operation Scheduling callback and convenience functions.
   template <typename CallbackT, typename... Params>
@@ -306,6 +308,7 @@ class CONTENT_EXPORT BackgroundSyncManager
   bool disabled_;
 
   scoped_ptr<BackgroundSyncNetworkObserver> network_observer_;
+  scoped_ptr<BackgroundSyncPowerObserver> power_observer_;
 
   base::WeakPtrFactory<BackgroundSyncManager> weak_ptr_factory_;
 
diff --git a/content/browser/background_sync/background_sync_manager_unittest.cc b/content/browser/background_sync/background_sync_manager_unittest.cc
index 8c9861c6..426f56f 100644
--- a/content/browser/background_sync/background_sync_manager_unittest.cc
+++ b/content/browser/background_sync/background_sync_manager_unittest.cc
@@ -6,6 +6,8 @@
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
+#include "base/power_monitor/power_monitor.h"
+#include "base/power_monitor/power_monitor_source.h"
 #include "base/run_loop.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/browser/browser_thread_impl.h"
@@ -76,6 +78,18 @@ void OneShotDelayedCallback(
   *out_callback = callback;
 }
 
+class TestPowerSource : public base::PowerMonitorSource {
+ public:
+  void GeneratePowerStateEvent(bool on_battery_power) {
+    test_on_battery_power_ = on_battery_power;
+    ProcessPowerEvent(POWER_STATE_EVENT);
+  }
+
+ private:
+  bool IsOnBatteryPowerImpl() final { return test_on_battery_power_; }
+  bool test_on_battery_power_ = false;
+};
+
 }  // namespace
 
 // A BackgroundSyncManager that can simulate delaying and corrupting the backend
@@ -211,6 +225,13 @@ class BackgroundSyncManagerTest : public testing::Test {
     helper_.reset(
         new EmbeddedWorkerTestHelper(base::FilePath(), kRenderProcessId));
 
+    power_monitor_source_ = new TestPowerSource();
+    // power_monitor_ takes ownership of power_monitor_source.
+    power_monitor_.reset(new base::PowerMonitor(
+        scoped_ptr<base::PowerMonitorSource>(power_monitor_source_)));
+
+    SetOnBatteryPower(false);
+
     background_sync_manager_ =
         BackgroundSyncManager::Create(helper_->context_wrapper());
 
@@ -256,6 +277,11 @@ class BackgroundSyncManagerTest : public testing::Test {
     base::RunLoop().RunUntilIdle();
   }
 
+  void SetOnBatteryPower(bool on_battery_power) {
+    power_monitor_source_->GeneratePowerStateEvent(on_battery_power);
+    base::RunLoop().RunUntilIdle();
+  }
+
   void StatusAndRegistrationCallback(
       bool* was_called,
       BackgroundSyncManager::ErrorType error,
@@ -430,6 +456,8 @@ class BackgroundSyncManagerTest : public testing::Test {
 
   TestBrowserThreadBundle browser_thread_bundle_;
   scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
+  TestPowerSource* power_monitor_source_;  // owned by power_monitor_
+  scoped_ptr<base::PowerMonitor> power_monitor_;
   scoped_ptr<EmbeddedWorkerTestHelper> helper_;
   scoped_ptr<BackgroundSyncManager> background_sync_manager_;
   TestBackgroundSyncManager* test_background_sync_manager_;
@@ -974,6 +1002,42 @@ TEST_F(BackgroundSyncManagerTest, OneShotFiresOnRegistration) {
   EXPECT_FALSE(GetRegistration(sync_reg_1_));
 }
 
+// TODO(jkarlin): Change this to a periodic test as one-shots can't be power
+// dependent according to spec.
+TEST_F(BackgroundSyncManagerTest, OneShotFiresOnPowerChange) {
+  InitSyncEventTest();
+  sync_reg_1_.power_state = POWER_STATE_AVOID_DRAINING;
+
+  SetOnBatteryPower(true);
+  EXPECT_TRUE(Register(sync_reg_1_));
+  EXPECT_EQ(0, sync_events_called_);
+  EXPECT_TRUE(GetRegistration(sync_reg_1_));
+
+  SetOnBatteryPower(false);
+  EXPECT_EQ(1, sync_events_called_);
+  EXPECT_FALSE(GetRegistration(sync_reg_1_));
+}
+
+// TODO(jkarlin): Change this to a periodic test as one-shots can't be power
+// dependent according to spec.
+TEST_F(BackgroundSyncManagerTest, MultipleOneShotsFireOnPowerChange) {
+  InitSyncEventTest();
+  sync_reg_1_.power_state = POWER_STATE_AVOID_DRAINING;
+  sync_reg_2_.power_state = POWER_STATE_AVOID_DRAINING;
+
+  SetOnBatteryPower(true);
+  EXPECT_TRUE(Register(sync_reg_1_));
+  EXPECT_TRUE(Register(sync_reg_2_));
+  EXPECT_EQ(0, sync_events_called_);
+  EXPECT_TRUE(GetRegistration(sync_reg_1_));
+  EXPECT_TRUE(GetRegistration(sync_reg_2_));
+
+  SetOnBatteryPower(false);
+  EXPECT_EQ(2, sync_events_called_);
+  EXPECT_FALSE(GetRegistration(sync_reg_1_));
+  EXPECT_FALSE(GetRegistration(sync_reg_2_));
+}
+
 TEST_F(BackgroundSyncManagerTest, OneShotFiresOnNetworkChange) {
   InitSyncEventTest();
 
diff --git a/content/browser/background_sync/background_sync_power_observer.cc b/content/browser/background_sync/background_sync_power_observer.cc
new file mode 100644
index 0000000..087fe21
--- /dev/null
+++ b/content/browser/background_sync/background_sync_power_observer.cc
@@ -0,0 +1,62 @@
+// Copyright 2015 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/background_sync/background_sync_power_observer.h"
+
+#include "base/power_monitor/power_monitor.h"
+#include "base/thread_task_runner_handle.h"
+
+namespace content {
+
+BackgroundSyncPowerObserver::BackgroundSyncPowerObserver(
+    const base::Closure& power_callback)
+    : observing_power_monitor_(false),
+      on_battery_(true),
+      power_changed_callback_(power_callback) {
+  base::PowerMonitor* power_monitor = base::PowerMonitor::Get();
+  if (power_monitor) {
+    observing_power_monitor_ = true;
+    on_battery_ = power_monitor->IsOnBatteryPower();
+    power_monitor->AddObserver(this);
+  }
+}
+
+BackgroundSyncPowerObserver::~BackgroundSyncPowerObserver() {
+  base::PowerMonitor* power_monitor = base::PowerMonitor::Get();
+
+  if (power_monitor)
+    power_monitor->RemoveObserver(this);
+}
+
+bool BackgroundSyncPowerObserver::PowerSufficient(
+    SyncPowerState power_state) const {
+  DCHECK(observing_power_monitor_);
+  DCHECK(base::PowerMonitor::Get());
+
+  switch (power_state) {
+    case POWER_STATE_AUTO:
+      // TODO(jkarlin): Also check for device status, such as power saving mode
+      // or user preferences. crbug.com/482088.
+      return true;
+    case POWER_STATE_AVOID_DRAINING:
+      return !on_battery_;
+  }
+
+  NOTREACHED();
+  return false;
+}
+
+void BackgroundSyncPowerObserver::OnPowerStateChange(bool on_battery_power) {
+  DCHECK(observing_power_monitor_);
+  DCHECK(base::PowerMonitor::Get());
+
+  if (on_battery_ == on_battery_power)
+    return;
+
+  on_battery_ = on_battery_power;
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                power_changed_callback_);
+}
+
+}  // namespace content
diff --git a/content/browser/background_sync/background_sync_power_observer.h b/content/browser/background_sync/background_sync_power_observer.h
new file mode 100644
index 0000000..80b26f0
--- /dev/null
+++ b/content/browser/background_sync/background_sync_power_observer.h
@@ -0,0 +1,50 @@
+// Copyright 2015 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.
+
+#ifndef CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_POWER_OBSERVER_H_
+#define CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_POWER_OBSERVER_H_
+
+#include "base/bind.h"
+#include "base/power_monitor/power_observer.h"
+#include "content/browser/background_sync/background_sync.pb.h"
+#include "content/common/content_export.h"
+
+namespace content {
+
+// BackgroundSyncPowerObserver monitors the charging status of the device and
+// determines if the power conditions of BackgroundSyncRegistrations are met.
+class CONTENT_EXPORT BackgroundSyncPowerObserver : public base::PowerObserver {
+ public:
+  // Creates a BackgroundSyncPowerObserver. |power_callback| is run when the
+  // battery states changes asynchronously via PostMessage.
+  explicit BackgroundSyncPowerObserver(const base::Closure& power_callback);
+
+  ~BackgroundSyncPowerObserver() override;
+
+  // Returns true if the state of the battery (charging or not) meets the needs
+  // of |power_state|.
+  bool PowerSufficient(SyncPowerState power_state) const;
+
+ private:
+  friend class BackgroundSyncPowerObserverTest;
+
+  // PowerObserver overrides
+  void OnPowerStateChange(bool on_battery_power) override;
+
+  // |observing_power_monitor_| is true when the constructor is able to find and
+  // register an observer with the base::PowerMonitor. This should always be
+  // true except for tests in which the browser initialization isn't done.
+  bool observing_power_monitor_;
+
+  bool on_battery_;
+
+  // The callback to run when the battery state changes.
+  base::Closure power_changed_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(BackgroundSyncPowerObserver);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_POWER_OBSERVER_H_
diff --git a/content/browser/background_sync/background_sync_power_observer_unittest.cc b/content/browser/background_sync/background_sync_power_observer_unittest.cc
new file mode 100644
index 0000000..8d3ed32
--- /dev/null
+++ b/content/browser/background_sync/background_sync_power_observer_unittest.cc
@@ -0,0 +1,71 @@
+// Copyright 2015 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/background_sync/background_sync_power_observer.h>
+
+#include "base/run_loop.h"
+#include "base/test/power_monitor_test_base.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+class BackgroundSyncPowerObserverTest : public testing::Test {
+ protected:
+  BackgroundSyncPowerObserverTest() : power_changed_count_(0) {
+    power_monitor_source_ = new base::PowerMonitorTestSource();
+    power_monitor_.reset(new base::PowerMonitor(
+        scoped_ptr<base::PowerMonitorSource>(power_monitor_source_)));
+    power_observer_.reset(new BackgroundSyncPowerObserver(
+        base::Bind(&BackgroundSyncPowerObserverTest::OnPowerChanged,
+                   base::Unretained(this))));
+  }
+
+  void SetOnBatteryPower(bool on_battery_power) {
+    power_monitor_source_->GeneratePowerStateEvent(on_battery_power);
+  }
+
+  void OnPowerChanged() { power_changed_count_++; }
+
+  // power_monitor_source_ is owned by power_monitor_
+  base::PowerMonitorTestSource* power_monitor_source_;
+  scoped_ptr<base::PowerMonitor> power_monitor_;
+  scoped_ptr<BackgroundSyncPowerObserver> power_observer_;
+  int power_changed_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(BackgroundSyncPowerObserverTest);
+};
+
+TEST_F(BackgroundSyncPowerObserverTest, PowerChangeInvokesCallback) {
+  SetOnBatteryPower(true);
+  power_changed_count_ = 0;
+
+  SetOnBatteryPower(false);
+  EXPECT_EQ(1, power_changed_count_);
+  SetOnBatteryPower(true);
+  EXPECT_EQ(2, power_changed_count_);
+  SetOnBatteryPower(true);
+  EXPECT_EQ(2, power_changed_count_);
+}
+
+TEST_F(BackgroundSyncPowerObserverTest, PowerSufficientAuto) {
+  SetOnBatteryPower(false);
+  EXPECT_TRUE(power_observer_->PowerSufficient(POWER_STATE_AUTO));
+
+  SetOnBatteryPower(true);
+  EXPECT_TRUE(power_observer_->PowerSufficient(POWER_STATE_AUTO));
+}
+
+TEST_F(BackgroundSyncPowerObserverTest, PowerSufficientAvoidDraining) {
+  SetOnBatteryPower(false);
+  EXPECT_TRUE(power_observer_->PowerSufficient(POWER_STATE_AVOID_DRAINING));
+
+  SetOnBatteryPower(true);
+  EXPECT_FALSE(power_observer_->PowerSufficient(POWER_STATE_AVOID_DRAINING));
+}
+
+}  // namespace
+
+}  // namespace content
diff --git a/content/browser/background_sync/background_sync_service_impl_unittest.cc b/content/browser/background_sync/background_sync_service_impl_unittest.cc
index 1aee396..9c2ed5d 100644
--- a/content/browser/background_sync/background_sync_service_impl_unittest.cc
+++ b/content/browser/background_sync/background_sync_service_impl_unittest.cc
@@ -7,6 +7,8 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/power_monitor/power_monitor.h"
+#include "base/power_monitor/power_monitor_source.h"
 #include "base/run_loop.h"
 #include "content/browser/background_sync/background_sync_context_impl.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
@@ -75,6 +77,12 @@ void ErrorAndRegistrationListCallback(
     *out_array_size = registrations.size();
 }
 
+class MockPowerMonitorSource : public base::PowerMonitorSource {
+ private:
+  // PowerMonitorSource overrides.
+  bool IsOnBatteryPowerImpl() final { return false; }
+};
+
 }  // namespace
 
 class BackgroundSyncServiceImplTest : public testing::Test {
@@ -106,6 +114,9 @@ class BackgroundSyncServiceImplTest : public testing::Test {
   }
 
   void CreateBackgroundSyncContext() {
+    power_monitor_.reset(
+        new base::PowerMonitor(make_scoped_ptr(new MockPowerMonitorSource())));
+
     background_sync_context_ = new BackgroundSyncContextImpl();
     background_sync_context_->Init(embedded_worker_helper_->context_wrapper());
     base::RunLoop().RunUntilIdle();
@@ -173,6 +184,7 @@ class BackgroundSyncServiceImplTest : public testing::Test {
 
   scoped_ptr<TestBrowserThreadBundle> thread_bundle_;
   scoped_ptr<EmbeddedWorkerTestHelper> embedded_worker_helper_;
+  scoped_ptr<base::PowerMonitor> power_monitor_;
   scoped_refptr<BackgroundSyncContextImpl> background_sync_context_;
   int64 sw_registration_id_;
   scoped_refptr<ServiceWorkerRegistration> sw_registration_;