Update cloud print to use base::Value and base::BindOnce()

This change updates CloudPrintProxyService methods to take base::Value
instead of base::DictionaryValue pointers/references and call base::BindOnce()
instead of base::Bind(). ServiceProcessControl implementation was updated to
use base::OnceClosure accordingly.

Bug: 646113
Change-Id: Id4ff1b0ffd96ec27969bd489ab64503d85c7d6c6
Reviewed-on: https://chromium-review.googlesource.com/998522Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548643}

chrome/browser/extensions/api/cloud_print_private/cloud_print_private_api.cc

@@ -55,8 +55,9 @@ bool CloudPrintPrivateSetupConnectorFunction::RunAsync() {
     return true;
   }
 
-  std::unique_ptr<base::DictionaryValue> user_settings(
-      params->user_settings.ToValue());
+  base::Value user_settings_value =
+      base::Value::FromUniquePtrValue(params->user_settings.ToValue());
+
   CloudPrintProxyService* service =
       CloudPrintProxyServiceFactory::GetForProfile(GetProfile());
   if (!service) {
@@ -64,7 +65,8 @@ bool CloudPrintPrivateSetupConnectorFunction::RunAsync() {
     return false;
   }
   service->EnableForUserWithRobot(params->credentials, params->robot_email,
-                                  params->user_email, *user_settings);
+                                  params->user_email,
+                                  std::move(user_settings_value));
   SendResponse(true);
   return true;
 }

chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc

@@ -93,16 +93,16 @@ void CloudPrintProxyService::EnableForUserWithRobot(
     const std::string& robot_auth_code,
     const std::string& robot_email,
     const std::string& user_email,
-    const base::DictionaryValue& user_preferences) {
+    base::Value user_preferences) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents",
                             ServiceProcessControl::SERVICE_EVENT_ENABLE,
                             ServiceProcessControl::SERVICE_EVENT_MAX);
   if (profile_->GetPrefs()->GetBoolean(prefs::kCloudPrintProxyEnabled)) {
     InvokeServiceTask(
-        base::Bind(&CloudPrintProxyService::EnableCloudPrintProxyWithRobot,
-                   weak_factory_.GetWeakPtr(), robot_auth_code, robot_email,
-                   user_email, base::Owned(user_preferences.DeepCopy())));
+        base::BindOnce(&CloudPrintProxyService::EnableCloudPrintProxyWithRobot,
+                       weak_factory_.GetWeakPtr(), robot_auth_code, robot_email,
+                       user_email, std::move(user_preferences)));
   }
 }
 
@@ -195,12 +195,11 @@ void CloudPrintProxyService::EnableCloudPrintProxyWithRobot(
     const std::string& robot_auth_code,
     const std::string& robot_email,
     const std::string& user_email,
-    const base::DictionaryValue* user_preferences) {
+    base::Value user_preferences) {
   ServiceProcessControl* process_control = GetServiceProcessControl();
   DCHECK(process_control->IsConnected());
   GetCloudPrintProxy().EnableCloudPrintProxyWithRobot(
-      robot_auth_code, robot_email, user_email,
-      std::move(*user_preferences->CreateDeepCopy()));
+      robot_auth_code, robot_email, user_email, std::move(user_preferences));
 
   // Assume the IPC worked.
   profile_->GetPrefs()->SetString(prefs::kCloudPrintEmail, user_email);
@@ -226,8 +225,8 @@ void CloudPrintProxyService::ProxyInfoCallback(bool enabled,
   ApplyCloudPrintConnectorPolicy();
 }
 
-bool CloudPrintProxyService::InvokeServiceTask(const base::Closure& task) {
-  GetServiceProcessControl()->Launch(task, base::Closure());
+bool CloudPrintProxyService::InvokeServiceTask(base::OnceClosure task) {
+  GetServiceProcessControl()->Launch(std::move(task), base::OnceClosure());
   return true;
 }
 

chrome/browser/printing/cloud_print/cloud_print_proxy_service.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/common/cloud_print.mojom.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -25,10 +26,6 @@
 class Profile;
 class ServiceProcessControl;
 
-namespace base {
-class DictionaryValue;
-}  // namespace base
-
 // Layer between the browser user interface and the cloud print proxy code
 // running in the service process.
 class CloudPrintProxyService : public KeyedService {
@@ -47,11 +44,10 @@ class CloudPrintProxyService : public KeyedService {
   void GetPrinters(const PrintersCallback& callback);
 
   // Enables/disables cloud printing for the user
-  virtual void EnableForUserWithRobot(
-      const std::string& robot_auth_code,
-      const std::string& robot_email,
-      const std::string& user_email,
-      const base::DictionaryValue& user_settings);
+  virtual void EnableForUserWithRobot(const std::string& robot_auth_code,
+                                      const std::string& robot_email,
+                                      const std::string& user_email,
+                                      base::Value user_settings);
   virtual void DisableForUser();
 
   // Query the service process for the status of the cloud print proxy and
@@ -68,11 +64,10 @@ class CloudPrintProxyService : public KeyedService {
   // Methods that send an IPC to the service.
   void GetCloudPrintProxyPrinters(const PrintersCallback& callback);
   void RefreshCloudPrintProxyStatus();
-  void EnableCloudPrintProxyWithRobot(
-      const std::string& robot_auth_code,
-      const std::string& robot_email,
-      const std::string& user_email,
-      const base::DictionaryValue* user_preferences);
+  void EnableCloudPrintProxyWithRobot(const std::string& robot_auth_code,
+                                      const std::string& robot_email,
+                                      const std::string& user_email,
+                                      base::Value user_preferences);
   void DisableCloudPrintProxy();
 
   // Callback that gets the cloud print proxy info.
@@ -83,7 +78,7 @@ class CloudPrintProxyService : public KeyedService {
   // Invoke a task that gets run after the service process successfully
   // launches. The task typically involves sending an IPC to the service
   // process.
-  bool InvokeServiceTask(const base::Closure& task);
+  bool InvokeServiceTask(base::OnceClosure task);
 
   // Checks the policy. Returns true if nothing needs to be done (the policy is
   // not set or the connector is not enabled).

chrome/browser/printing/cloud_print/cloud_print_proxy_service_unittest.cc

@@ -51,12 +51,16 @@ class MockServiceProcessControl : public ServiceProcessControl {
 
   MockServiceProcessControl() : connected_(false) { }
 
+  ~MockServiceProcessControl() override {}
+
   MOCK_CONST_METHOD0(IsConnected, bool());
 
-  MOCK_METHOD2(Launch, void(const base::Closure&, const base::Closure&));
+  void Launch(base::OnceClosure success_task,
+              base::OnceClosure failure_task) override;
+
   MOCK_METHOD0(Disconnect, void());
 
-  void SetConnectSuccessMockExpectations(bool post_task);
+  void SetConnectSuccessMockExpectations();
 
  private:
   bool connected_;
@@ -68,28 +72,22 @@ std::string MockServiceProcessControl::EnabledUserId() {
   return std::string("dorothy@somewhere.otr");
 }
 
-void CallTask(const base::Closure& task) {
-  if (!task.is_null())
-    task.Run();
+void CallTask(base::OnceClosure task) {
+  if (task)
+    std::move(task).Run();
 }
 
-void PostTask(const base::Closure& task) {
-  if (!task.is_null())
-    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task);
+void MockServiceProcessControl::Launch(base::OnceClosure success_task,
+                                       base::OnceClosure failure_task) {
+  connected_ = true;
+  CallTask(std::move(success_task));
 }
 
-void MockServiceProcessControl::SetConnectSuccessMockExpectations(
-    bool post_task) {
+void MockServiceProcessControl::SetConnectSuccessMockExpectations() {
   EXPECT_CALL(*this, IsConnected()).WillRepeatedly(ReturnPointee(&connected_));
 
-  EXPECT_CALL(*this, Launch(_, _))
-      .WillRepeatedly(
-          DoAll(Assign(&connected_, true),
-                WithArgs<0>(Invoke(post_task ? PostTask : CallTask))));
-
   EXPECT_CALL(*this, Disconnect()).Times(AtMost(1))
       .WillRepeatedly(Assign(&connected_, false));
-
 }
 
 class MockCloudPrintProxy : public cloud_print::mojom::CloudPrint {
@@ -189,7 +187,7 @@ class TestCloudPrintProxyService : public CloudPrintProxyService {
   void EnableForUser() {
     EnableForUserWithRobot("123", "123@gmail.com",
                            MockServiceProcessControl::EnabledUserId(),
-                           base::DictionaryValue());
+                           base::Value(base::Value::Type::DICTIONARY));
   }
 
  private:
@@ -220,12 +218,12 @@ class CloudPrintProxyPolicyTest : public ::testing::Test {
 
 TEST_F(CloudPrintProxyPolicyTest, VerifyExpectations) {
   MockServiceProcessControl mock_control;
-  mock_control.SetConnectSuccessMockExpectations(false);
+  mock_control.SetConnectSuccessMockExpectations();
 
   EXPECT_FALSE(mock_control.IsConnected());
-  mock_control.Launch(base::Closure(), base::Closure());
+  mock_control.Launch(base::OnceClosure(), base::OnceClosure());
   EXPECT_TRUE(mock_control.IsConnected());
-  mock_control.Launch(base::Closure(), base::Closure());
+  mock_control.Launch(base::OnceClosure(), base::OnceClosure());
   EXPECT_TRUE(mock_control.IsConnected());
   mock_control.Disconnect();
   EXPECT_FALSE(mock_control.IsConnected());
@@ -235,8 +233,7 @@ TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabled) {
   TestCloudPrintProxyService service(&profile_);
 
   service.GetMockCloudPrintProxy().ReturnDisabledInfo();
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
       profile_.GetTestingPrefService();
@@ -252,8 +249,7 @@ TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabled) {
 TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyEnabled) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnEnabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -271,8 +267,7 @@ TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyEnabled) {
 TEST_F(CloudPrintProxyPolicyTest, StartWithPolicySetProxyDisabled) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnDisabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -290,8 +285,7 @@ TEST_F(CloudPrintProxyPolicyTest, StartWithPolicySetProxyDisabled) {
 TEST_F(CloudPrintProxyPolicyTest, StartWithPolicySetProxyEnabled) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnEnabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -310,8 +304,7 @@ TEST_F(CloudPrintProxyPolicyTest, StartWithPolicySetProxyEnabled) {
 TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabledThenSetPolicy) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnDisabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -333,8 +326,7 @@ TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabledThenSetPolicy) {
 TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyEnabledThenSetPolicy) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnEnabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -359,8 +351,7 @@ TEST_F(CloudPrintProxyPolicyTest,
        StartWithPolicySetProxyDisabledThenClearPolicy) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnDisabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -381,8 +372,7 @@ TEST_F(CloudPrintProxyPolicyTest,
        StartWithPolicySetProxyEnabledThenClearPolicy) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnEnabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -403,8 +393,7 @@ TEST_F(CloudPrintProxyPolicyTest,
 TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabledThenEnable) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnDisabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
@@ -427,8 +416,7 @@ TEST_F(CloudPrintProxyPolicyTest,
        StartWithPolicySetProxyEnabledThenClearPolicyAndEnable) {
   TestCloudPrintProxyService service(&profile_);
 
-  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations(
-      false);
+  service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations();
   service.GetMockCloudPrintProxy().ReturnEnabledInfo();
 
   sync_preferences::TestingPrefServiceSyncable* prefs =

chrome/browser/service_process/service_process_control.cc

@@ -159,7 +159,7 @@ void ServiceProcessControl::RunConnectDoneTasks() {
 void ServiceProcessControl::RunAllTasksHelper(TaskList* task_list) {
   TaskList::iterator index = task_list->begin();
   while (index != task_list->end()) {
-    (*index).Run();
+    std::move(*index).Run();
     index = task_list->erase(index);
   }
 }
@@ -168,16 +168,15 @@ bool ServiceProcessControl::IsConnected() const {
   return !!service_process_;
 }
 
-void ServiceProcessControl::Launch(const base::Closure& success_task,
-                                   const base::Closure& failure_task) {
+void ServiceProcessControl::Launch(base::OnceClosure success_task,
+                                   base::OnceClosure failure_task) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::Closure failure = failure_task;
-  if (!success_task.is_null())
-    connect_success_tasks_.push_back(success_task);
+  if (success_task)
+    connect_success_tasks_.emplace_back(std::move(success_task));
 
-  if (!failure.is_null())
-    connect_failure_tasks_.push_back(failure);
+  if (failure_task)
+    connect_failure_tasks_.emplace_back(std::move(failure_task));
 
   // If we already in the process of launching, then we are done.
   if (launcher_.get())

chrome/browser/service_process/service_process_control.h

@@ -87,8 +87,8 @@ class ServiceProcessControl : public UpgradeObserver {
   // Note that if we are already connected to service process then
   // |success_task| can be invoked in the context of the Launch call.
   // Virtual for testing.
-  virtual void Launch(const base::Closure& success_task,
-                      const base::Closure& failure_task);
+  virtual void Launch(base::OnceClosure success_task,
+                      base::OnceClosure failure_task);
 
   // Disconnect the IPC channel from the service process.
   // Virtual for testing.
@@ -155,7 +155,7 @@ class ServiceProcessControl : public UpgradeObserver {
 
   friend struct base::DefaultSingletonTraits<ServiceProcessControl>;
 
-  typedef std::vector<base::Closure> TaskList;
+  using TaskList = std::vector<base::OnceClosure>;
 
   void OnChannelConnected();
   void OnChannelError();

chrome/service/cloud_print/cloud_print_message_handler.cc

@@ -33,11 +33,8 @@ void CloudPrintMessageHandler::EnableCloudPrintProxyWithRobot(
     const std::string& robot_email,
     const std::string& user_email,
     base::Value user_settings) {
-  std::unique_ptr<base::DictionaryValue> user_settings_dict =
-      base::DictionaryValue::From(
-          base::Value::ToUniquePtrValue(std::move(user_settings)));
   proxy_provider_->GetCloudPrintProxy()->EnableForUserWithRobot(
-      robot_auth_code, robot_email, user_email, *user_settings_dict);
+      robot_auth_code, robot_email, user_email, std::move(user_settings));
 }
 
 void CloudPrintMessageHandler::GetCloudPrintProxyInfo(

chrome/service/cloud_print/cloud_print_proxy.cc

@@ -73,11 +73,10 @@ void CloudPrintProxy::EnableForUser() {
   }
 }
 
-void CloudPrintProxy::EnableForUserWithRobot(
-    const std::string& robot_auth_code,
-    const std::string& robot_email,
-    const std::string& user_email,
-    const base::DictionaryValue& user_settings) {
+void CloudPrintProxy::EnableForUserWithRobot(const std::string& robot_auth_code,
+                                             const std::string& robot_email,
+                                             const std::string& user_email,
+                                             base::Value user_settings) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   ShutdownBackend();

chrome/service/cloud_print/cloud_print_proxy.h

@@ -12,6 +12,7 @@
 
 #include "base/macros.h"
 #include "base/sequence_checker.h"
+#include "base/values.h"
 #include "chrome/service/cloud_print/cloud_print_proxy_backend.h"
 #include "chrome/service/cloud_print/cloud_print_wipeout.h"
 
@@ -47,11 +48,10 @@ class CloudPrintProxy : public CloudPrintProxyFrontend,
 
   // Enables/disables cloud printing for the user
   void EnableForUser();
-  void EnableForUserWithRobot(
-      const std::string& robot_auth_code,
-      const std::string& robot_email,
-      const std::string& user_email,
-      const base::DictionaryValue& user_settings);
+  void EnableForUserWithRobot(const std::string& robot_auth_code,
+                              const std::string& robot_email,
+                              const std::string& user_email,
+                              base::Value user_settings);
   void UnregisterPrintersAndDisableForUser();
   void DisableForUser();
   // Returns the proxy info.