Background download: Network/battery status listener implementation on Mac.

net::NetworkChangeNotifier and base::PowerObserver are not hooked to
correct platform code on Mac, thus make background download service not
working correctly.

This CL provides default implementation for device status monitoring on
Mac that always treat the device connected to internet and power source.

This is mostly a temporary solution before platform code is implemented
correctly.

Also pull out BatteryStatusListener and NetworkStatusListener to a
separate file.

Bug: 825878
Change-Id: If34d8eb0cbe5d0575920c4251eda5e5161695adf
Reviewed-on: https://chromium-review.googlesource.com/1178689Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584345}

components/download/content/factory/download_service_factory.cc

@@ -27,6 +27,13 @@
 
 #if defined(OS_ANDROID)
 #include "components/download/internal/background_service/android/battery_status_listener_android.h"
+#include "components/download/internal/background_service/android/network_status_listener_android.h"
+#elif defined(OS_MACOSX)
+#include "components/download/internal/background_service/scheduler/battery_status_listener_mac.h"
+#include "components/download/internal/background_service/scheduler/network_status_listener_mac.h"
+#else
+#include "components/download/internal/background_service/scheduler/battery_status_listener_impl.h"
+#include "components/download/internal/background_service/scheduler/network_status_listener_impl.h"
 #endif
 
 namespace download {
@@ -51,17 +58,24 @@ DownloadService* CreateDownloadServiceInternal(
   auto client_set = std::make_unique<ClientSet>(std::move(clients));
   auto model = std::make_unique<ModelImpl>(std::move(store));
 
+// Build platform network/battery status listener.
 #if defined(OS_ANDROID)
   auto battery_listener = std::make_unique<BatteryStatusListenerAndroid>(
       config->battery_query_interval);
+  auto network_listener = std::make_unique<NetworkStatusListenerAndroid>();
+#elif defined(OS_MACOSX)
+  auto battery_listener = std::make_unique<BatteryStatusListenerMac>();
+  auto network_listener = std::make_unique<NetworkStatusListenerMac>();
 #else
-  auto battery_listener =
-      std::make_unique<BatteryStatusListener>(config->battery_query_interval);
+  auto battery_listener = std::make_unique<BatteryStatusListenerImpl>(
+      config->battery_query_interval);
+  auto network_listener =
+      std::make_unique<NetworkStatusListenerImpl>(network_connection_tracker);
 #endif
 
   auto device_status_listener = std::make_unique<DeviceStatusListener>(
       config->network_startup_delay, config->network_change_delay,
-      std::move(battery_listener), network_connection_tracker);
+      std::move(battery_listener), std::move(network_listener));
   NavigationMonitor* navigation_monitor =
       NavigationMonitorFactory::GetForBrowserContext(browser_context);
   auto scheduler = std::make_unique<SchedulerImpl>(

components/download/internal/background_service/BUILD.gn

@@ -65,12 +65,17 @@ static_library("internal") {
     "noop_store.h",
     "proto_conversions.cc",
     "proto_conversions.h",
+    "scheduler/battery_status_listener.h",
+    "scheduler/battery_status_listener_impl.cc",
+    "scheduler/battery_status_listener_impl.h",
     "scheduler/device_status.cc",
     "scheduler/device_status.h",
     "scheduler/device_status_listener.cc",
     "scheduler/device_status_listener.h",
     "scheduler/network_status_listener.cc",
     "scheduler/network_status_listener.h",
+    "scheduler/network_status_listener_impl.cc",
+    "scheduler/network_status_listener_impl.h",
     "scheduler/scheduler.h",
     "scheduler/scheduler_impl.cc",
     "scheduler/scheduler_impl.h",
@@ -103,6 +108,15 @@ static_library("internal") {
 
     deps += [ ":jni_headers" ]
   }
+
+  if (is_mac) {
+    sources += [
+      "scheduler/battery_status_listener_mac.cc",
+      "scheduler/battery_status_listener_mac.h",
+      "scheduler/network_status_listener_mac.cc",
+      "scheduler/network_status_listener_mac.h",
+    ]
+  }
 }
 
 if (is_android) {

components/download/internal/background_service/android/battery_status_listener_android.cc

@@ -11,7 +11,7 @@ namespace download {
 
 BatteryStatusListenerAndroid::BatteryStatusListenerAndroid(
     const base::TimeDelta& battery_query_interval)
-    : BatteryStatusListener(battery_query_interval) {}
+    : BatteryStatusListenerImpl(battery_query_interval) {}
 
 BatteryStatusListenerAndroid::~BatteryStatusListenerAndroid() = default;
 

components/download/internal/background_service/android/battery_status_listener_android.h

@@ -5,7 +5,7 @@
 #ifndef COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_ANDROID_BATTERY_STATUS_LISTENER_ANDROID_H_
 #define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_ANDROID_BATTERY_STATUS_LISTENER_ANDROID_H_
 
-#include "components/download/internal/background_service/scheduler/device_status_listener.h"
+#include "components/download/internal/background_service/scheduler/battery_status_listener_impl.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
@@ -13,7 +13,7 @@
 namespace download {
 
 // Backed by a Java class that holds helper functions to query battery status.
-class BatteryStatusListenerAndroid : public BatteryStatusListener {
+class BatteryStatusListenerAndroid : public BatteryStatusListenerImpl {
  public:
   BatteryStatusListenerAndroid(const base::TimeDelta& battery_query_interval);
   ~BatteryStatusListenerAndroid() override;

components/download/internal/background_service/android/network_status_listener_android.h

@@ -9,6 +9,7 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/macros.h"
 
 namespace download {
 

components/download/internal/background_service/scheduler/battery_status_listener.h

+// Copyright 2018 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 COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_H_
+#define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_H_
+
+namespace download {
+
+// Interface to monitor device battery status.
+class BatteryStatusListener {
+ public:
+  class Observer {
+   public:
+    // Called when device charging state changed.
+    virtual void OnPowerStateChange(bool on_battery_power) = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+  virtual ~BatteryStatusListener() = default;
+
+  // Get the device battery percentage.
+  virtual int GetBatteryPercentage() = 0;
+
+  // Is the device is using battery power instead of charging.
+  virtual bool IsOnBatteryPower() = 0;
+
+  // Start/Stop to listen to battery status changes.
+  virtual void Start(Observer* observer) = 0;
+  virtual void Stop() = 0;
+
+ protected:
+  BatteryStatusListener() = default;
+};
+
+}  // namespace download
+
+#endif  // COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_H_

components/download/internal/background_service/scheduler/battery_status_listener_impl.cc

+// Copyright 2018 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 "components/download/internal/background_service/scheduler/battery_status_listener_impl.h"
+
+#include "components/download/internal/background_service/scheduler/device_status.h"
+
+namespace download {
+
+BatteryStatusListenerImpl::BatteryStatusListenerImpl(
+    const base::TimeDelta& battery_query_interval)
+    : battery_percentage_(100),
+      battery_query_interval_(battery_query_interval),
+      last_battery_query_(base::Time::Now()),
+      observer_(nullptr) {}
+
+BatteryStatusListenerImpl::~BatteryStatusListenerImpl() = default;
+
+int BatteryStatusListenerImpl::GetBatteryPercentage() {
+  UpdateBatteryPercentage(false);
+  return battery_percentage_;
+}
+
+bool BatteryStatusListenerImpl::IsOnBatteryPower() {
+  return base::PowerMonitor::Get()->IsOnBatteryPower();
+}
+
+void BatteryStatusListenerImpl::Start(Observer* observer) {
+  observer_ = observer;
+
+  base::PowerMonitor* power_monitor = base::PowerMonitor::Get();
+  DCHECK(power_monitor);
+  power_monitor->AddObserver(this);
+
+  UpdateBatteryPercentage(true);
+}
+
+void BatteryStatusListenerImpl::Stop() {
+  base::PowerMonitor::Get()->RemoveObserver(this);
+}
+
+int BatteryStatusListenerImpl::GetBatteryPercentageInternal() {
+  // Non-Android implementation currently always return full battery.
+  return 100;
+}
+
+void BatteryStatusListenerImpl::UpdateBatteryPercentage(bool force) {
+  // Throttle the battery queries.
+  if (!force &&
+      base::Time::Now() - last_battery_query_ < battery_query_interval_) {
+    return;
+  }
+
+  battery_percentage_ = GetBatteryPercentageInternal();
+  last_battery_query_ = base::Time::Now();
+}
+
+void BatteryStatusListenerImpl::OnPowerStateChange(bool on_battery_power) {
+  if (observer_)
+    observer_->OnPowerStateChange(on_battery_power);
+}
+
+}  // namespace download

components/download/internal/background_service/scheduler/battery_status_listener_impl.h

+// Copyright 2018 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 COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_IMPL_H_
+#define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_IMPL_H_
+
+#include "base/macros.h"
+#include "base/power_monitor/power_monitor.h"
+#include "base/timer/timer.h"
+#include "components/download/internal/background_service/scheduler/battery_status_listener.h"
+
+namespace download {
+
+// Default implementation of BatteryStatusListener.
+class BatteryStatusListenerImpl : public BatteryStatusListener,
+                                  public base::PowerObserver {
+ public:
+  explicit BatteryStatusListenerImpl(
+      const base::TimeDelta& battery_query_interval);
+  ~BatteryStatusListenerImpl() override;
+
+ protected:
+  // Platform specific code should override to query the actual battery state.
+  virtual int GetBatteryPercentageInternal();
+
+ private:
+  // BatteryStatusListener implementation.
+  int GetBatteryPercentage() override;
+  bool IsOnBatteryPower() override;
+  void Start(Observer* observer) override;
+  void Stop() override;
+
+  // Updates battery percentage. Will throttle based on
+  // |battery_query_interval_| when |force| is false.
+  void UpdateBatteryPercentage(bool force);
+
+  // base::PowerObserver implementation.
+  void OnPowerStateChange(bool on_battery_power) override;
+
+  // Cached battery percentage.
+  int battery_percentage_;
+
+  // Interval to throttle battery queries. Cached value will be returned inside
+  // this interval.
+  base::TimeDelta battery_query_interval_;
+
+  // Time stamp to record last battery query.
+  base::Time last_battery_query_;
+
+  Observer* observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(BatteryStatusListenerImpl);
+};
+
+}  // namespace download
+
+#endif  // COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_IMPL_H_

components/download/internal/background_service/scheduler/battery_status_listener_mac.cc

+// Copyright 2018 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 "components/download/internal/background_service/scheduler/battery_status_listener_mac.h"
+
+namespace download {
+
+BatteryStatusListenerMac::BatteryStatusListenerMac() = default;
+
+BatteryStatusListenerMac::~BatteryStatusListenerMac() = default;
+
+int BatteryStatusListenerMac::GetBatteryPercentage() {
+  return 100;
+}
+
+bool BatteryStatusListenerMac::IsOnBatteryPower() {
+  return false;
+}
+
+void BatteryStatusListenerMac::Start(Observer* observer) {}
+
+void BatteryStatusListenerMac::Stop() {}
+
+}  // namespace download

components/download/internal/background_service/scheduler/battery_status_listener_mac.h

+// Copyright 2018 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 COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_MAC_H_
+#define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_MAC_H_
+
+#include "base/macros.h"
+#include "components/download/internal/background_service/scheduler/battery_status_listener.h"
+
+namespace download {
+
+// Mac implementation of BatteryStatusListener. Currently always Mac device
+// full battery and charging.
+// We should investigate if platform code can be correctly hooked to
+// base::PowerMonitor on Mac. See https://crbug.com/825878.
+class BatteryStatusListenerMac : public BatteryStatusListener {
+ public:
+  BatteryStatusListenerMac();
+  ~BatteryStatusListenerMac() override;
+
+ private:
+  // BatteryStatusListener implementation.
+  int GetBatteryPercentage() override;
+  bool IsOnBatteryPower() override;
+  void Start(Observer* observer) override;
+  void Stop() override;
+
+  DISALLOW_COPY_AND_ASSIGN(BatteryStatusListenerMac);
+};
+
+}  // namespace download
+
+#endif  // COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_BATTERY_STATUS_LISTENER_MAC_H_

components/download/internal/background_service/scheduler/device_status_listener.cc

@@ -4,14 +4,6 @@
 
 #include "components/download/internal/background_service/scheduler/device_status_listener.h"
 
-#include "base/power_monitor/power_monitor.h"
-#include "build/build_config.h"
-#include "components/download/internal/background_service/scheduler/network_status_listener.h"
-
-#if defined(OS_ANDROID)
-#include "components/download/internal/background_service/android/network_status_listener_android.h"
-#endif
-
 namespace download {
 
 namespace {
@@ -43,72 +35,18 @@ NetworkStatus ToNetworkStatus(network::mojom::ConnectionType type) {
 
 }  // namespace
 
-BatteryStatusListener::BatteryStatusListener(
-    const base::TimeDelta& battery_query_interval)
-    : battery_percentage_(0),
-      battery_query_interval_(battery_query_interval),
-      last_battery_query_(base::Time::Now()) {}
-
-BatteryStatusListener::~BatteryStatusListener() = default;
-
-int BatteryStatusListener::GetBatteryPercentage() {
-  UpdateBatteryPercentage(false);
-  return battery_percentage_;
-}
-
-bool BatteryStatusListener::IsOnBatteryPower() {
-  return base::PowerMonitor::Get()->IsOnBatteryPower();
-}
-
-void BatteryStatusListener::Start(Observer* observer) {
-  observer_ = observer;
-
-  base::PowerMonitor* power_monitor = base::PowerMonitor::Get();
-  DCHECK(power_monitor);
-  power_monitor->AddObserver(this);
-
-  UpdateBatteryPercentage(true);
-}
-
-void BatteryStatusListener::Stop() {
-  base::PowerMonitor::Get()->RemoveObserver(this);
-}
-
-int BatteryStatusListener::GetBatteryPercentageInternal() {
-  // Non-Android implementation currently always return full battery.
-  return 100;
-}
-
-void BatteryStatusListener::UpdateBatteryPercentage(bool force) {
-  // Throttle the battery queries.
-  if (!force &&
-      base::Time::Now() - last_battery_query_ < battery_query_interval_)
-    return;
-
-  battery_percentage_ = GetBatteryPercentageInternal();
-  last_battery_query_ = base::Time::Now();
-}
-
-void BatteryStatusListener::OnPowerStateChange(bool on_battery_power) {
-  if (observer_)
-    observer_->OnPowerStateChange(on_battery_power);
-}
-
 DeviceStatusListener::DeviceStatusListener(
     const base::TimeDelta& startup_delay,
     const base::TimeDelta& online_delay,
     std::unique_ptr<BatteryStatusListener> battery_listener,
-    network::NetworkConnectionTracker* network_connection_tracker)
-    : observer_(nullptr),
+    std::unique_ptr<NetworkStatusListener> network_listener)
+    : network_listener_(std::move(network_listener)),
+      observer_(nullptr),
       listening_(false),
       is_valid_state_(false),
       startup_delay_(startup_delay),
       online_delay_(online_delay),
-#if !defined(OS_ANDROID)
-      network_connection_tracker_(network_connection_tracker),
-#endif
-      battery_listener_(std::move(battery_listener)) {
-}
+      battery_listener_(std::move(battery_listener)) {}
 
 DeviceStatusListener::~DeviceStatusListener() {
   Stop();
@@ -142,11 +80,10 @@ void DeviceStatusListener::StartAfterDelay() {
       ToBatteryStatus(battery_listener_->IsOnBatteryPower());
 
   // Listen to network status changes.
-  BuildNetworkStatusListener();
   network_listener_->Start(this);
 
   status_.battery_status =
-      ToBatteryStatus(base::PowerMonitor::Get()->IsOnBatteryPower());
+      ToBatteryStatus(battery_listener_->IsOnBatteryPower());
   status_.network_status =
       ToNetworkStatus(network_listener_->GetConnectionType());
   pending_network_status_ = status_.network_status;
@@ -222,13 +159,4 @@ void DeviceStatusListener::NotifyNetworkChange() {
   NotifyStatusChange();
 }
 
-void DeviceStatusListener::BuildNetworkStatusListener() {
-#if defined(OS_ANDROID)
-  network_listener_ = std::make_unique<NetworkStatusListenerAndroid>();
-#else
-  network_listener_ =
-      std::make_unique<NetworkStatusListenerImpl>(network_connection_tracker_);
-#endif
-}
-
 }  // namespace download

components/download/internal/background_service/scheduler/device_status_listener.h

@@ -7,63 +7,14 @@
 
 #include <memory>
 
-#include "base/power_monitor/power_observer.h"
+#include "base/macros.h"
 #include "base/timer/timer.h"
-#include "build/build_config.h"
+#include "components/download/internal/background_service/scheduler/battery_status_listener.h"
 #include "components/download/internal/background_service/scheduler/device_status.h"
 #include "components/download/internal/background_service/scheduler/network_status_listener.h"
-#include "services/network/public/cpp/network_connection_tracker.h"
 
 namespace download {
 
-// Helper class to listen to battery changes.
-class BatteryStatusListener : public base::PowerObserver {
- public:
-  class Observer {
-   public:
-    // Called when device charging state changed.
-    virtual void OnPowerStateChange(bool on_battery_power) = 0;
-
-   protected:
-    virtual ~Observer() {}
-  };
-
-  BatteryStatusListener(const base::TimeDelta& battery_query_interval);
-  ~BatteryStatusListener() override;
-
-  int GetBatteryPercentage();
-  bool IsOnBatteryPower();
-
-  void Start(Observer* observer);
-  void Stop();
-
- protected:
-  // Platform specific code should override to query the actual battery state.
-  virtual int GetBatteryPercentageInternal();
-
- private:
-  // Updates battery percentage. Will throttle based on
-  // |battery_query_interval_| when |force| is false.
-  void UpdateBatteryPercentage(bool force);
-
-  // base::PowerObserver implementation.
-  void OnPowerStateChange(bool on_battery_power) override;
-
-  // Cached battery percentage.
-  int battery_percentage_;
-
-  // Interval to throttle battery queries. Cached value will be returned inside
-  // this interval.
-  base::TimeDelta battery_query_interval_;
-
-  // Time stamp to record last battery query.
-  base::Time last_battery_query_;
-
-  Observer* observer_ = nullptr;
-
-  DISALLOW_COPY_AND_ASSIGN(BatteryStatusListener);
-};
-
 // Listens to network and battery status change and notifies the observer.
 class DeviceStatusListener : public NetworkStatusListener::Observer,
                              public BatteryStatusListener::Observer {
@@ -74,11 +25,11 @@ class DeviceStatusListener : public NetworkStatusListener::Observer,
     virtual void OnDeviceStatusChanged(const DeviceStatus& device_status) = 0;
   };
 
-  DeviceStatusListener(
+  explicit DeviceStatusListener(
       const base::TimeDelta& startup_delay,
       const base::TimeDelta& online_delay,
       std::unique_ptr<BatteryStatusListener> battery_listener,
-      network::NetworkConnectionTracker* network_connection_tracker);
+      std::unique_ptr<NetworkStatusListener> network_listener);
   ~DeviceStatusListener() override;
 
   bool is_valid_state() { return is_valid_state_; }
@@ -93,9 +44,6 @@ class DeviceStatusListener : public NetworkStatusListener::Observer,
   virtual void Stop();
 
  protected:
-  // Creates the instance of |network_listener_|, visible for testing.
-  virtual void BuildNetworkStatusListener();
-
   // NetworkStatusListener::Observer implementation. Visible for testing.
   void OnNetworkChanged(network::mojom::ConnectionType type) override;
 
@@ -138,11 +86,6 @@ class DeviceStatusListener : public NetworkStatusListener::Observer,
   // Pending network status used to update the current network status.
   NetworkStatus pending_network_status_ = NetworkStatus::DISCONNECTED;
 
-#if !defined(OS_ANDROID)
-  // Used to listen for network connection changes.
-  network::NetworkConnectionTracker* network_connection_tracker_;
-#endif
-
   // Used to listen to battery status.
   std::unique_ptr<BatteryStatusListener> battery_listener_;
 

components/download/internal/background_service/scheduler/device_status_listener_unittest.cc

@@ -9,7 +9,8 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/test/power_monitor_test_base.h"
-#include "components/download/internal/background_service/scheduler/network_status_listener.h"
+#include "components/download/internal/background_service/scheduler/battery_status_listener_impl.h"
+#include "components/download/internal/background_service/scheduler/network_status_listener_impl.h"
 #include "services/network/test/test_network_connection_tracker.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -34,9 +35,9 @@ class MockObserver : public DeviceStatusListener::Observer {
   MOCK_METHOD1(OnDeviceStatusChanged, void(const DeviceStatus&));
 };
 
-class TestBatteryStatusListener : public BatteryStatusListener {
+class TestBatteryStatusListener : public BatteryStatusListenerImpl {
  public:
-  TestBatteryStatusListener() : BatteryStatusListener(base::TimeDelta()) {}
+  TestBatteryStatusListener() : BatteryStatusListenerImpl(base::TimeDelta()) {}
   ~TestBatteryStatusListener() override = default;
 
   void set_battery_percentage(int battery_percentage) {
@@ -56,22 +57,15 @@ class TestDeviceStatusListener : public DeviceStatusListener {
  public:
   explicit TestDeviceStatusListener(
       std::unique_ptr<TestBatteryStatusListener> battery_listener,
-      network::NetworkConnectionTracker* network_connection_tracker)
+      std::unique_ptr<NetworkStatusListener> network_listener)
       : DeviceStatusListener(base::TimeDelta(),
                              base::TimeDelta(),
                              std::move(battery_listener),
-                             network_connection_tracker),
-        network_connection_tracker_(network_connection_tracker) {}
-
-  void BuildNetworkStatusListener() override {
-    network_listener_ = std::make_unique<NetworkStatusListenerImpl>(
-        network_connection_tracker_);
-  }
+                             std::move(network_listener)) {}
 
  private:
-  network::NetworkConnectionTracker* network_connection_tracker_;
-
   friend class DeviceStatusListenerTest;
+  DISALLOW_COPY_AND_ASSIGN(TestDeviceStatusListener);
 };
 
 class DeviceStatusListenerTest : public testing::Test {
@@ -87,8 +81,12 @@ class DeviceStatusListenerTest : public testing::Test {
 
     auto battery_listener = std::make_unique<TestBatteryStatusListener>();
     test_battery_listener_ = battery_listener.get();
+
+    auto network_listener = std::make_unique<NetworkStatusListenerImpl>(
+        &network_connection_tracker_);
+
     listener_ = std::make_unique<TestDeviceStatusListener>(
-        std::move(battery_listener), &network_connection_tracker_);
+        std::move(battery_listener), std::move(network_listener));
   }
 
   void TearDown() override { listener_.reset(); }

components/download/internal/background_service/scheduler/network_status_listener.cc

@@ -4,12 +4,12 @@
 
 #include "components/download/internal/background_service/scheduler/network_status_listener.h"
 
-#include "services/network/public/cpp/network_connection_tracker.h"
-
 namespace download {
 
 NetworkStatusListener::NetworkStatusListener() = default;
 
+NetworkStatusListener::~NetworkStatusListener() = default;
+
 void NetworkStatusListener::Start(NetworkStatusListener::Observer* observer) {
   observer_ = observer;
 }
@@ -18,36 +18,8 @@ void NetworkStatusListener::Stop() {
   observer_ = nullptr;
 }
 
-NetworkStatusListenerImpl::NetworkStatusListenerImpl(
-    network::NetworkConnectionTracker* network_connection_tracker)
-    : network_connection_tracker_(network_connection_tracker),
-      connection_type_(network::mojom::ConnectionType::CONNECTION_UNKNOWN) {}
-
-NetworkStatusListenerImpl::~NetworkStatusListenerImpl() = default;
-
-void NetworkStatusListenerImpl::Start(
-    NetworkStatusListener::Observer* observer) {
-  NetworkStatusListener::Start(observer);
-  network_connection_tracker_->AddNetworkConnectionObserver(this);
-  network_connection_tracker_->GetConnectionType(
-      &connection_type_,
-      base::BindOnce(&NetworkStatusListenerImpl::OnConnectionChanged,
-                     base::Unretained(this)));
-}
-
-void NetworkStatusListenerImpl::Stop() {
-  network_connection_tracker_->RemoveNetworkConnectionObserver(this);
-  NetworkStatusListener::Stop();
-}
-
-network::mojom::ConnectionType NetworkStatusListenerImpl::GetConnectionType() {
+network::mojom::ConnectionType NetworkStatusListener::GetConnectionType() {
   return connection_type_;
 }
 
-void NetworkStatusListenerImpl::OnConnectionChanged(
-    network::mojom::ConnectionType type) {
-  DCHECK(observer_);
-  observer_->OnNetworkChanged(type);
-}
-
 }  // namespace download

components/download/internal/background_service/scheduler/network_status_listener.h

@@ -5,7 +5,8 @@
 #ifndef COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_H_
 #define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_H_
 
-#include "services/network/public/cpp/network_connection_tracker.h"
+#include "base/macros.h"
+#include "services/network/public/mojom/network_change_manager.mojom.h"
 
 namespace download {
 
@@ -25,15 +26,15 @@ class NetworkStatusListener {
   };
 
   // Starts to listen to network changes.
-  virtual void Start(Observer* observer);
+  virtual void Start(Observer* observer) = 0;
 
   // Stops to listen to network changes.
-  virtual void Stop();
+  virtual void Stop() = 0;
 
   // Gets the current connection type.
   virtual network::mojom::ConnectionType GetConnectionType() = 0;
 
-  virtual ~NetworkStatusListener() {}
+  virtual ~NetworkStatusListener();
 
  protected:
   NetworkStatusListener();
@@ -42,38 +43,13 @@ class NetworkStatusListener {
   Observer* observer_ = nullptr;
 
   // The current network status.
-  network::mojom::ConnectionType network_status_ =
-      network::mojom::ConnectionType::CONNECTION_NONE;
+  network::mojom::ConnectionType connection_type_ =
+      network::mojom::ConnectionType::CONNECTION_UNKNOWN;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NetworkStatusListener);
 };
 
-// Default implementation of NetworkStatusListener using
-// NetworkConnectionTracker to listen to connectivity changes.
-class NetworkStatusListenerImpl
-    : public network::NetworkConnectionTracker::NetworkConnectionObserver,
-      public NetworkStatusListener {
- public:
-  explicit NetworkStatusListenerImpl(
-      network::NetworkConnectionTracker* network_connection_tracker);
-  ~NetworkStatusListenerImpl() override;
-
-  // NetworkStatusListener implementation.
-  void Start(NetworkStatusListener::Observer* observer) override;
-  void Stop() override;
-  network::mojom::ConnectionType GetConnectionType() override;
-
- private:
-  // network::NetworkConnectionTracker::NetworkConnectionObserver.
-  void OnConnectionChanged(network::mojom::ConnectionType type) override;
-
-  network::NetworkConnectionTracker* network_connection_tracker_;
-  network::mojom::ConnectionType connection_type_;
-
-  DISALLOW_COPY_AND_ASSIGN(NetworkStatusListenerImpl);
-};
-
 }  // namespace download
 
 #endif  // COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_H_

components/download/internal/background_service/scheduler/network_status_listener_impl.cc

+// Copyright 2018 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 "components/download/internal/background_service/scheduler/network_status_listener_impl.h"
+
+#include "base/bind.h"
+
+namespace download {
+
+NetworkStatusListenerImpl::NetworkStatusListenerImpl(
+    network::NetworkConnectionTracker* network_connection_tracker)
+    : network_connection_tracker_(network_connection_tracker) {}
+
+NetworkStatusListenerImpl::~NetworkStatusListenerImpl() = default;
+
+void NetworkStatusListenerImpl::Start(
+    NetworkStatusListener::Observer* observer) {
+  NetworkStatusListener::Start(observer);
+  network_connection_tracker_->AddNetworkConnectionObserver(this);
+  network_connection_tracker_->GetConnectionType(
+      &connection_type_,
+      base::BindOnce(&NetworkStatusListenerImpl::OnConnectionChanged,
+                     base::Unretained(this)));
+}
+
+void NetworkStatusListenerImpl::Stop() {
+  network_connection_tracker_->RemoveNetworkConnectionObserver(this);
+  NetworkStatusListener::Stop();
+}
+
+network::mojom::ConnectionType NetworkStatusListenerImpl::GetConnectionType() {
+  return connection_type_;
+}
+
+void NetworkStatusListenerImpl::OnConnectionChanged(
+    network::mojom::ConnectionType type) {
+  DCHECK(observer_);
+  observer_->OnNetworkChanged(type);
+}
+
+}  // namespace download

components/download/internal/background_service/scheduler/network_status_listener_impl.h

+// Copyright 2018 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 COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_IMPL_H_
+#define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_IMPL_H_
+
+#include "base/macros.h"
+#include "components/download/internal/background_service/scheduler/network_status_listener.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
+
+namespace download {
+
+// Default implementation of NetworkStatusListener using
+// NetworkConnectionTracker to listen to connectivity changes.
+class NetworkStatusListenerImpl
+    : public network::NetworkConnectionTracker::NetworkConnectionObserver,
+      public NetworkStatusListener {
+ public:
+  explicit NetworkStatusListenerImpl(
+      network::NetworkConnectionTracker* network_connection_tracker);
+  ~NetworkStatusListenerImpl() override;
+
+  // NetworkStatusListener implementation.
+  void Start(NetworkStatusListener::Observer* observer) override;
+  void Stop() override;
+  network::mojom::ConnectionType GetConnectionType() override;
+
+ private:
+  // network::NetworkConnectionTracker::NetworkConnectionObserver.
+  void OnConnectionChanged(network::mojom::ConnectionType type) override;
+
+  network::NetworkConnectionTracker* network_connection_tracker_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkStatusListenerImpl);
+};
+
+}  // namespace download
+
+#endif  // COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_IMPL_H_

components/download/internal/background_service/scheduler/network_status_listener_mac.cc

+// Copyright 2018 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 "components/download/internal/background_service/scheduler/network_status_listener_mac.h"
+
+namespace download {
+
+NetworkStatusListenerMac::NetworkStatusListenerMac() = default;
+
+NetworkStatusListenerMac::~NetworkStatusListenerMac() = default;
+
+void NetworkStatusListenerMac::Start(
+    NetworkStatusListener::Observer* observer) {}
+
+void NetworkStatusListenerMac::Stop() {}
+
+network::mojom::ConnectionType NetworkStatusListenerMac::GetConnectionType() {
+  return network::mojom::ConnectionType::CONNECTION_WIFI;
+}
+
+}  // namespace download

components/download/internal/background_service/scheduler/network_status_listener_mac.h

+// Copyright 2018 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 COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_MAC_H_
+#define COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_MAC_H_
+
+#include "base/macros.h"
+#include "components/download/internal/background_service/scheduler/network_status_listener.h"
+
+namespace download {
+
+// Mac implementation of NetworkStatusListener. Always treat Mac device as
+// connected to internet.
+// We should investigate if platform code can be correctly hooked to
+// net::NetworkChangeNotifier on Mac. See https://crbug.com/825878.
+class NetworkStatusListenerMac : public NetworkStatusListener {
+ public:
+  NetworkStatusListenerMac();
+  ~NetworkStatusListenerMac() override;
+
+ private:
+  // NetworkStatusListener implementation.
+  void Start(NetworkStatusListener::Observer* observer) override;
+  void Stop() override;
+  network::mojom::ConnectionType GetConnectionType() override;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkStatusListenerMac);
+};
+
+}  // namespace download
+
+#endif  // COMPONENTS_DOWNLOAD_INTERNAL_BACKGROUND_SERVICE_SCHEDULER_NETWORK_STATUS_LISTENER_MAC_H_

components/download/internal/background_service/test/test_device_status_listener.cc

@@ -6,12 +6,15 @@
 
 #include <memory>
 
+#include "components/download/internal/background_service/scheduler/battery_status_listener_impl.h"
+#include "components/download/internal/background_service/scheduler/network_status_listener_impl.h"
+
 namespace download {
 namespace test {
 
-class FakeBatteryStatusListener : public BatteryStatusListener {
+class FakeBatteryStatusListener : public BatteryStatusListenerImpl {
  public:
-  FakeBatteryStatusListener() : BatteryStatusListener(base::TimeDelta()) {}
+  FakeBatteryStatusListener() : BatteryStatusListenerImpl(base::TimeDelta()) {}
   ~FakeBatteryStatusListener() override = default;
 
   // BatteryStatusListener implementation.
@@ -25,7 +28,8 @@ TestDeviceStatusListener::TestDeviceStatusListener()
     : DeviceStatusListener(base::TimeDelta(), /* startup_delay */
                            base::TimeDelta(), /* online_delay */
                            std::make_unique<FakeBatteryStatusListener>(),
-                           &test_network_connection_tracker_),
+                           std::make_unique<NetworkStatusListenerImpl>(
+                               &test_network_connection_tracker_)),
       test_network_connection_tracker_(
           true,
           network::mojom::ConnectionType::CONNECTION_UNKNOWN),