[CrOS Cellular] Add a test double for NetworkActivationHandler.

This process includes making NetworkActivationHandler a pure virtual
class with an implementation named NetworkActivationHandlerImpl.

Bug: 961084
Change-Id: Icda121db0a0febc46e2d3b64848279fd58ac3a76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1601770Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658210}

chromeos/network/BUILD.gn

@@ -63,8 +63,9 @@ component("network") {
     "managed_network_configuration_handler_impl.h",
     "managed_state.cc",
     "managed_state.h",
-    "network_activation_handler.cc",
     "network_activation_handler.h",
+    "network_activation_handler_impl.cc",
+    "network_activation_handler_impl.h",
     "network_cert_loader.cc",
     "network_cert_loader.h",
     "network_cert_migrator.cc",
@@ -181,6 +182,8 @@ source_set("test_support") {
     "//testing/gtest",
   ]
   sources = [
+    "fake_network_activation_handler.cc",
+    "fake_network_activation_handler.h",
     "fake_network_device_handler.cc",
     "fake_network_device_handler.h",
     "mock_managed_network_configuration_handler.cc",

chromeos/network/fake_network_activation_handler.cc

+// Copyright 2019 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 "chromeos/network/fake_network_activation_handler.h"
+
+#include "base/callback.h"
+#include "base/values.h"
+
+namespace chromeos {
+
+FakeNetworkActivationHandler::ActivationParams::ActivationParams(
+    const std::string& service_path,
+    const std::string& carrier,
+    const base::Closure& success_callback,
+    const network_handler::ErrorCallback& error_callback)
+    : service_path_(service_path),
+      carrier_(carrier),
+      success_callback_(success_callback),
+      error_callback_(error_callback) {}
+
+FakeNetworkActivationHandler::ActivationParams::ActivationParams(
+    const std::string& service_path,
+    const base::Closure& success_callback,
+    const network_handler::ErrorCallback& error_callback)
+    : service_path_(service_path),
+      success_callback_(success_callback),
+      error_callback_(error_callback) {}
+
+FakeNetworkActivationHandler::ActivationParams::ActivationParams(
+    const ActivationParams& other) = default;
+
+FakeNetworkActivationHandler::ActivationParams::~ActivationParams() = default;
+
+void FakeNetworkActivationHandler::ActivationParams::InvokeSuccessCallback() {
+  success_callback_.Run();
+}
+
+void FakeNetworkActivationHandler::ActivationParams::InvokeErrorCallback(
+    const std::string& error_name,
+    std::unique_ptr<base::DictionaryValue> error_data) {
+  error_callback_.Run(error_name, std::move(error_data));
+}
+
+FakeNetworkActivationHandler::FakeNetworkActivationHandler() = default;
+
+FakeNetworkActivationHandler::~FakeNetworkActivationHandler() = default;
+
+void FakeNetworkActivationHandler::Activate(
+    const std::string& service_path,
+    const std::string& carrier,
+    const base::Closure& success_callback,
+    const network_handler::ErrorCallback& error_callback) {
+  activate_calls_.emplace_back(service_path, carrier, success_callback,
+                               error_callback);
+}
+
+void FakeNetworkActivationHandler::CompleteActivation(
+    const std::string& service_path,
+    const base::Closure& success_callback,
+    const network_handler::ErrorCallback& error_callback) {
+  complete_activation_calls_.emplace_back(service_path, success_callback,
+                                          error_callback);
+}
+
+}  // namespace chromeos

chromeos/network/fake_network_activation_handler.h

+// Copyright 2019 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 CHROMEOS_NETWORK_FAKE_NETWORK_ACTIVATION_HANDLER_H_
+#define CHROMEOS_NETWORK_FAKE_NETWORK_ACTIVATION_HANDLER_H_
+
+#include <string>
+
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "chromeos/network/network_activation_handler.h"
+#include "chromeos/network/network_handler_callbacks.h"
+
+namespace chromeos {
+
+// Fake NetworkActivationHandler implementation for tests.
+class COMPONENT_EXPORT(CHROMEOS_NETWORK) FakeNetworkActivationHandler
+    : public NetworkActivationHandler {
+ public:
+  FakeNetworkActivationHandler();
+  ~FakeNetworkActivationHandler() override;
+
+  class ActivationParams {
+   public:
+    // For Activate() calls.
+    ActivationParams(const std::string& service_path,
+                     const std::string& carrier,
+                     const base::Closure& success_callback,
+                     const network_handler::ErrorCallback& error_callback);
+
+    // For CompleteActivation() calls.
+    ActivationParams(const std::string& service_path,
+                     const base::Closure& success_callback,
+                     const network_handler::ErrorCallback& error_callback);
+
+    ActivationParams(const ActivationParams& other);
+    ~ActivationParams();
+
+    const std::string& service_path() const { return service_path_; }
+    const std::string& carrier() const { return *carrier_; }
+
+    void InvokeSuccessCallback();
+    void InvokeErrorCallback(const std::string& error_name,
+                             std::unique_ptr<base::DictionaryValue> error_data);
+
+   private:
+    std::string service_path_;
+    base::Optional<std::string> carrier_;
+    base::Closure success_callback_;
+    network_handler::ErrorCallback error_callback_;
+  };
+
+  std::vector<ActivationParams>& activate_calls() { return activate_calls_; }
+  std::vector<ActivationParams>& complete_activation_calls() {
+    return complete_activation_calls_;
+  }
+
+ private:
+  // NetworkActivationHandler:
+  void Activate(const std::string& service_path,
+                const std::string& carrier,
+                const base::Closure& success_callback,
+                const network_handler::ErrorCallback& error_callback) override;
+  void CompleteActivation(
+      const std::string& service_path,
+      const base::Closure& success_callback,
+      const network_handler::ErrorCallback& error_callback) override;
+
+  std::vector<ActivationParams> activate_calls_;
+  std::vector<ActivationParams> complete_activation_calls_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeNetworkActivationHandler);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_NETWORK_FAKE_NETWORK_ACTIVATION_HANDLER_H_

chromeos/network/network_activation_handler.h

@@ -9,23 +9,15 @@
 
 #include "base/component_export.h"
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
 #include "chromeos/network/network_handler_callbacks.h"
 
 namespace chromeos {
 
 // The NetworkActivationHandler class allows making service specific
 // calls required for activation on mobile networks.
-class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkActivationHandler
-    : public base::SupportsWeakPtr<NetworkActivationHandler> {
+class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkActivationHandler {
  public:
-  // Constants for |error_name| from |error_callback|.
-  // TODO(gauravsh): Merge various error constants from Network*Handlers into
-  // a single place. crbug.com/272554
-  static const char kErrorNotFound[];
-  static const char kErrorShillError[];
-
-  virtual ~NetworkActivationHandler();
+  virtual ~NetworkActivationHandler() = default;
 
   // ActivateNetwork() will start an asynchronous activation attempt.
   // |carrier| may be empty or may specify a carrier to activate.
@@ -33,10 +25,11 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkActivationHandler
   // On failure, |error_callback| will be called with |error_name| one of:
   //  kErrorNotFound if no network matching |service_path| is found.
   //  kErrorShillError if a DBus or Shill error occurred.
-  void Activate(const std::string& service_path,
-                const std::string& carrier,
-                const base::Closure& success_callback,
-                const network_handler::ErrorCallback& error_callback);
+  virtual void Activate(
+      const std::string& service_path,
+      const std::string& carrier,
+      const base::Closure& success_callback,
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // CompleteActivation() will start an asynchronous activation completion
   // attempt.
@@ -44,32 +37,15 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkActivationHandler
   // On failure, |error_callback| will be called with |error_name| one of:
   //  kErrorNotFound if no network matching |service_path| is found.
   //  kErrorShillError if a DBus or Shill error occurred.
-  void CompleteActivation(const std::string& service_path,
-                          const base::Closure& success_callback,
-                          const network_handler::ErrorCallback& error_callback);
-
- private:
-  friend class NetworkHandler;
-
-  NetworkActivationHandler();
-
-  // Calls Shill.Service.ActivateCellularModem asynchronously.
-  void CallShillActivate(const std::string& service_path,
-                         const std::string& carrier,
-                         const base::Closure& success_callback,
-                         const network_handler::ErrorCallback& error_callback);
-
-  // Calls Shill.Service.CompleteCellularActivation asynchronously.
-  void CallShillCompleteActivation(
+  virtual void CompleteActivation(
       const std::string& service_path,
       const base::Closure& success_callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
-  // Handle success from Shill.Service.ActivateCellularModem or
-  // Shill.Service.CompleteCellularActivation.
-  void HandleShillSuccess(const std::string& service_path,
-                          const base::Closure& success_callback);
+ protected:
+  NetworkActivationHandler() = default;
 
+ private:
   DISALLOW_COPY_AND_ASSIGN(NetworkActivationHandler);
 };
 

chromeos/network/network_activation_handler.cc → chromeos/network/network_activation_handler_impl.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright 2019 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 "chromeos/network/network_activation_handler.h"
+#include "chromeos/network/network_activation_handler_impl.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -13,57 +13,44 @@
 
 namespace chromeos {
 
-// static
-const char NetworkActivationHandler::kErrorShillError[] = "shill-error";
+namespace {
 
-NetworkActivationHandler::NetworkActivationHandler() = default;
-NetworkActivationHandler::~NetworkActivationHandler() = default;
+const char kErrorShillError[] = "shill-error";
 
-void NetworkActivationHandler::Activate(
-    const std::string& service_path,
-    const std::string& carrier,
-    const base::Closure& success_callback,
-    const network_handler::ErrorCallback& error_callback) {
-  NET_LOG_USER("ActivateNetwork", service_path);
-  CallShillActivate(service_path, carrier, success_callback, error_callback);
-}
+}  // namespace
 
-void NetworkActivationHandler::CompleteActivation(
-    const std::string& service_path,
-    const base::Closure& success_callback,
-    const network_handler::ErrorCallback& error_callback) {
-  NET_LOG_USER("CompleteActivation", service_path);
-  CallShillCompleteActivation(service_path, success_callback, error_callback);
-}
+NetworkActivationHandlerImpl::NetworkActivationHandlerImpl() = default;
+
+NetworkActivationHandlerImpl::~NetworkActivationHandlerImpl() = default;
 
-void NetworkActivationHandler::CallShillActivate(
+void NetworkActivationHandlerImpl::Activate(
     const std::string& service_path,
     const std::string& carrier,
     const base::Closure& success_callback,
     const network_handler::ErrorCallback& error_callback) {
-  NET_LOG_USER("Activation Request", service_path + ": '" + carrier + "'");
+  NET_LOG_USER("ActivateNetwork", service_path + ": '" + carrier + "'");
   ShillServiceClient::Get()->ActivateCellularModem(
       dbus::ObjectPath(service_path), carrier,
-      base::Bind(&NetworkActivationHandler::HandleShillSuccess, AsWeakPtr(),
+      base::Bind(&NetworkActivationHandlerImpl::HandleShillSuccess, AsWeakPtr(),
                  service_path, success_callback),
       base::Bind(&network_handler::ShillErrorCallbackFunction, kErrorShillError,
                  service_path, error_callback));
 }
 
-void NetworkActivationHandler::CallShillCompleteActivation(
+void NetworkActivationHandlerImpl::CompleteActivation(
     const std::string& service_path,
     const base::Closure& success_callback,
     const network_handler::ErrorCallback& error_callback) {
-  NET_LOG_USER("CompleteActivation Request", service_path);
+  NET_LOG_USER("CompleteActivation", service_path);
   ShillServiceClient::Get()->CompleteCellularActivation(
       dbus::ObjectPath(service_path),
-      base::Bind(&NetworkActivationHandler::HandleShillSuccess, AsWeakPtr(),
+      base::Bind(&NetworkActivationHandlerImpl::HandleShillSuccess, AsWeakPtr(),
                  service_path, success_callback),
       base::Bind(&network_handler::ShillErrorCallbackFunction, kErrorShillError,
                  service_path, error_callback));
 }
 
-void NetworkActivationHandler::HandleShillSuccess(
+void NetworkActivationHandlerImpl::HandleShillSuccess(
     const std::string& service_path,
     const base::Closure& success_callback) {
   if (!success_callback.is_null())

chromeos/network/network_activation_handler_impl.h

+// Copyright 2019 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 CHROMEOS_NETWORK_NETWORK_ACTIVATION_HANDLER_IMPL_H_
+#define CHROMEOS_NETWORK_NETWORK_ACTIVATION_HANDLER_IMPL_H_
+
+#include <string>
+
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/network/network_activation_handler.h"
+#include "chromeos/network/network_handler_callbacks.h"
+
+namespace chromeos {
+
+// The NetworkActivationHandlerImpl class allows making service specific
+// calls required for activation on mobile networks.
+class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkActivationHandlerImpl
+    : public NetworkActivationHandler,
+      public base::SupportsWeakPtr<NetworkActivationHandlerImpl> {
+ public:
+  ~NetworkActivationHandlerImpl() override;
+
+ private:
+  // NetworkActivationHandler:
+  void Activate(const std::string& service_path,
+                const std::string& carrier,
+                const base::Closure& success_callback,
+                const network_handler::ErrorCallback& error_callback) override;
+  void CompleteActivation(
+      const std::string& service_path,
+      const base::Closure& success_callback,
+      const network_handler::ErrorCallback& error_callback) override;
+
+ private:
+  friend class NetworkHandler;
+
+  NetworkActivationHandlerImpl();
+
+  // Handle success from Shill.Service.ActivateCellularModem or
+  // Shill.Service.CompleteCellularActivation.
+  void HandleShillSuccess(const std::string& service_path,
+                          const base::Closure& success_callback);
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkActivationHandlerImpl);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_NETWORK_NETWORK_ACTIVATION_HANDLER_IMPL_H_

chromeos/network/network_handler.cc

@@ -10,7 +10,7 @@
 #include "chromeos/network/client_cert_resolver.h"
 #include "chromeos/network/geolocation_handler.h"
 #include "chromeos/network/managed_network_configuration_handler_impl.h"
-#include "chromeos/network/network_activation_handler.h"
+#include "chromeos/network/network_activation_handler_impl.h"
 #include "chromeos/network/network_cert_loader.h"
 #include "chromeos/network/network_cert_migrator.h"
 #include "chromeos/network/network_certificate_handler.h"
@@ -44,7 +44,7 @@ NetworkHandler::NetworkHandler()
     network_certificate_handler_.reset(new NetworkCertificateHandler());
     client_cert_resolver_.reset(new ClientCertResolver());
   }
-  network_activation_handler_.reset(new NetworkActivationHandler());
+  network_activation_handler_.reset(new NetworkActivationHandlerImpl());
   network_connection_handler_.reset(new NetworkConnectionHandlerImpl());
   cellular_metrics_logger_.reset(new CellularMetricsLogger());
   network_sms_handler_.reset(new NetworkSmsHandler());