Convert callbacks in //chrome/browser/chromeos/settings

This change converts callbacks from base::Bind and base::Callback to
Once/Repeating in //chrome/browser/chromeos/settings.

Bug: 1148570
Change-Id: I10bcc9e993a9df44932719297ed1893821ae2525
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2641476
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845589}

chrome/browser/chromeos/settings/cros_settings.cc

@@ -81,9 +81,9 @@ CrosSettings::CrosSettings() = default;
 CrosSettings::CrosSettings(DeviceSettingsService* device_settings_service,
                            PrefService* local_state) {
   CrosSettingsProvider::NotifyObserversCallback notify_cb(
-      base::Bind(&CrosSettings::FireObservers,
-                 // This is safe since |this| is never deleted.
-                 base::Unretained(this)));
+      base::BindRepeating(&CrosSettings::FireObservers,
+                          // This is safe since |this| is never deleted.
+                          base::Unretained(this)));
 
   auto supervised_user_cros_provider =
       std::make_unique<SupervisedUserCrosSettingsProvider>(notify_cb);
@@ -272,8 +272,8 @@ bool CrosSettings::AddSettingsProvider(
   // Providers instantiated inside this class will have the same callback
   // passed to their constructor, but doing it here allows for providers
   // to be instantiated outside this class.
-  CrosSettingsProvider::NotifyObserversCallback notify_cb(
-      base::Bind(&CrosSettings::FireObservers, base::Unretained(this)));
+  CrosSettingsProvider::NotifyObserversCallback notify_cb(base::BindRepeating(
+      &CrosSettings::FireObservers, base::Unretained(this)));
   provider_ptr->SetNotifyObserversCallback(notify_cb);
   return true;
 }

chrome/browser/chromeos/settings/device_settings_provider.h

@@ -42,9 +42,6 @@ class DeviceSettingsProvider
       public DeviceSettingsService::Observer,
       public ownership::OwnerSettingsService::Observer {
  public:
-  // The callback type that is called to get the device mode.
-  typedef base::Callback<policy::DeviceMode(void)> GetDeviceModeCallback;
-
   DeviceSettingsProvider(const NotifyObserversCallback& notify_cb,
                          DeviceSettingsService* device_settings_service,
                          PrefService* pref_service);

chrome/browser/chromeos/settings/device_settings_provider_unittest.cc

@@ -66,8 +66,8 @@ class DeviceSettingsProviderTest : public DeviceSettingsTestBase {
 
     EXPECT_CALL(*this, SettingChanged(_)).Times(AnyNumber());
     provider_.reset(new DeviceSettingsProvider(
-        base::Bind(&DeviceSettingsProviderTest::SettingChanged,
-                   base::Unretained(this)),
+        base::BindRepeating(&DeviceSettingsProviderTest::SettingChanged,
+                            base::Unretained(this)),
         device_settings_service_.get(), local_state_.Get()));
     Mock::VerifyAndClearExpectations(this);
   }

chrome/browser/chromeos/settings/device_settings_service.cc

@@ -156,19 +156,19 @@ void DeviceSettingsService::LoadImmediately() {
   }
   std::unique_ptr<SessionManagerOperation> operation(new LoadSettingsOperation(
       request_key_load, cloud_validations, true /*force_immediate_load*/,
-      base::Bind(&DeviceSettingsService::HandleCompletedOperation,
-                 weak_factory_.GetWeakPtr(), base::Closure())));
+      base::BindOnce(&DeviceSettingsService::HandleCompletedOperation,
+                     weak_factory_.GetWeakPtr(), base::OnceClosure())));
   operation->Start(session_manager_client_, owner_key_util_, public_key_);
 }
 
 void DeviceSettingsService::Store(
     std::unique_ptr<em::PolicyFetchResponse> policy,
-    const base::Closure& callback) {
+    base::OnceClosure callback) {
   // On Active Directory managed devices policy is written only by authpolicyd.
   CHECK(device_mode_ != policy::DEVICE_MODE_ENTERPRISE_AD);
   Enqueue(std::make_unique<StoreSettingsOperation>(
-      base::Bind(&DeviceSettingsService::HandleCompletedAsyncOperation,
-                 weak_factory_.GetWeakPtr(), callback),
+      base::BindOnce(&DeviceSettingsService::HandleCompletedAsyncOperation,
+                     weak_factory_.GetWeakPtr(), std::move(callback)),
       std::move(policy)));
 }
 
@@ -281,8 +281,8 @@ void DeviceSettingsService::EnqueueLoad(bool request_key_load) {
   }
   Enqueue(std::make_unique<LoadSettingsOperation>(
       request_key_load, cloud_validations, false /*force_immediate_load*/,
-      base::Bind(&DeviceSettingsService::HandleCompletedAsyncOperation,
-                 weak_factory_.GetWeakPtr(), base::Closure())));
+      base::BindOnce(&DeviceSettingsService::HandleCompletedAsyncOperation,
+                     weak_factory_.GetWeakPtr(), base::OnceClosure())));
 }
 
 void DeviceSettingsService::EnsureReload(bool request_key_load) {
@@ -301,11 +301,11 @@ void DeviceSettingsService::StartNextOperation() {
 }
 
 void DeviceSettingsService::HandleCompletedAsyncOperation(
-    const base::Closure& callback,
+    base::OnceClosure callback,
     SessionManagerOperation* operation,
     Status status) {
   DCHECK_EQ(operation, pending_operations_.front().get());
-  HandleCompletedOperation(callback, operation, status);
+  HandleCompletedOperation(std::move(callback), operation, status);
   // Only remove the pending operation here, so new operations triggered by
   // any of the callbacks above are queued up properly.
   pending_operations_.pop_front();
@@ -314,7 +314,7 @@ void DeviceSettingsService::HandleCompletedAsyncOperation(
 }
 
 void DeviceSettingsService::HandleCompletedOperation(
-    const base::Closure& callback,
+    base::OnceClosure callback,
     SessionManagerOperation* operation,
     Status status) {
   store_status_ = status;
@@ -349,7 +349,7 @@ void DeviceSettingsService::HandleCompletedOperation(
   // The completion callback happens after the notification so clients can
   // filter self-triggered updates.
   if (!callback.is_null())
-    callback.Run();
+    std::move(callback).Run();
 }
 
 void DeviceSettingsService::NotifyOwnershipStatusChanged() const {

chrome/browser/chromeos/settings/device_settings_service.h

@@ -163,7 +163,7 @@ class DeviceSettingsService : public SessionManagerClient::Observer {
   // reported through |callback|. If successful, the updated device settings are
   // present in policy_data() and device_settings() when the callback runs.
   void Store(std::unique_ptr<enterprise_management::PolicyFetchResponse> policy,
-             const base::Closure& callback);
+             base::OnceClosure callback);
 
   // Returns the ownership status. May return OWNERSHIP_UNKNOWN if the disk
   // hasn't been checked yet.
@@ -229,13 +229,13 @@ class DeviceSettingsService : public SessionManagerClient::Observer {
   void StartNextOperation();
 
   // Updates status, policy data and owner key from a finished operation.
-  void HandleCompletedOperation(const base::Closure& callback,
+  void HandleCompletedOperation(base::OnceClosure callback,
                                 SessionManagerOperation* operation,
                                 Status status);
 
   // Same as HandleCompletedOperation(), but also starts the next pending
   // operation if available.
-  void HandleCompletedAsyncOperation(const base::Closure& callback,
+  void HandleCompletedAsyncOperation(base::OnceClosure callback,
                                      SessionManagerOperation* operation,
                                      Status status);
 

chrome/browser/chromeos/settings/device_settings_service_unittest.cc

@@ -156,8 +156,8 @@ TEST_F(DeviceSettingsServiceTest, StoreFailure) {
   session_manager_client_.ForceStorePolicyFailure(true);
   device_settings_service_->Store(
       device_policy_->GetCopy(),
-      base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted,
-                 base::Unretained(this)));
+      base::BindOnce(&DeviceSettingsServiceTest::SetOperationCompleted,
+                     base::Unretained(this)));
   FlushDeviceSettings();
   EXPECT_TRUE(operation_completed_);
   EXPECT_EQ(DeviceSettingsService::STORE_OPERATION_FAILED,
@@ -176,8 +176,8 @@ TEST_F(DeviceSettingsServiceTest, StoreSuccess) {
             true);
   device_settings_service_->Store(
       device_policy_->GetCopy(),
-      base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted,
-                 base::Unretained(this)));
+      base::BindOnce(&DeviceSettingsServiceTest::SetOperationCompleted,
+                     base::Unretained(this)));
   FlushDeviceSettings();
   EXPECT_TRUE(operation_completed_);
   EXPECT_EQ(DeviceSettingsService::STORE_SUCCESS,
@@ -195,7 +195,8 @@ TEST_F(DeviceSettingsServiceTest, StoreRotation) {
       ->set_device_policy_refresh_rate(300);
   device_policy_->SetDefaultNewSigningKey();
   device_policy_->Build();
-  device_settings_service_->Store(device_policy_->GetCopy(), base::Closure());
+  device_settings_service_->Store(device_policy_->GetCopy(),
+                                  base::OnceClosure());
   FlushDeviceSettings();
   owner_key_util_->SetPublicKeyFromPrivateKey(
       *device_policy_->GetNewSigningKey());
@@ -218,7 +219,7 @@ TEST_F(DeviceSettingsServiceTest, OwnershipStatus) {
   EXPECT_EQ(DeviceSettingsService::OWNERSHIP_UNKNOWN,
             device_settings_service_->GetOwnershipStatus());
 
-  device_settings_service_->GetOwnershipStatusAsync(base::Bind(
+  device_settings_service_->GetOwnershipStatusAsync(base::BindOnce(
       &DeviceSettingsServiceTest::SetOwnershipStatus, base::Unretained(this)));
   FlushDeviceSettings();
   EXPECT_FALSE(device_settings_service_->HasPrivateOwnerKey());
@@ -230,7 +231,7 @@ TEST_F(DeviceSettingsServiceTest, OwnershipStatus) {
 
   owner_key_util_->SetPublicKeyFromPrivateKey(*device_policy_->GetSigningKey());
   ReloadDeviceSettings();
-  device_settings_service_->GetOwnershipStatusAsync(base::Bind(
+  device_settings_service_->GetOwnershipStatusAsync(base::BindOnce(
       &DeviceSettingsServiceTest::SetOwnershipStatus, base::Unretained(this)));
   FlushDeviceSettings();
   EXPECT_FALSE(device_settings_service_->HasPrivateOwnerKey());
@@ -245,7 +246,7 @@ TEST_F(DeviceSettingsServiceTest, OwnershipStatus) {
   owner_key_util_->SetPrivateKey(device_policy_->GetSigningKey());
   InitOwner(AccountId::FromUserEmail(device_policy_->policy_data().username()),
             true);
-  device_settings_service_->GetOwnershipStatusAsync(base::Bind(
+  device_settings_service_->GetOwnershipStatusAsync(base::BindOnce(
       &DeviceSettingsServiceTest::SetOwnershipStatus, base::Unretained(this)));
   FlushDeviceSettings();
   EXPECT_TRUE(device_settings_service_->HasPrivateOwnerKey());
@@ -442,7 +443,8 @@ TEST_F(DeviceSettingsServiceTest, Observer) {
 
   EXPECT_CALL(observer_, OwnershipStatusChanged()).Times(0);
   EXPECT_CALL(observer_, DeviceSettingsUpdated()).Times(1);
-  device_settings_service_->Store(device_policy_->GetCopy(), base::Closure());
+  device_settings_service_->Store(device_policy_->GetCopy(),
+                                  base::OnceClosure());
   FlushDeviceSettings();
   Mock::VerifyAndClearExpectations(&observer_);
 

chrome/browser/chromeos/settings/session_manager_operation.cc

@@ -32,8 +32,8 @@ namespace em = enterprise_management;
 
 namespace chromeos {
 
-SessionManagerOperation::SessionManagerOperation(const Callback& callback)
-    : callback_(callback) {}
+SessionManagerOperation::SessionManagerOperation(Callback callback)
+    : callback_(std::move(callback)) {}
 
 SessionManagerOperation::~SessionManagerOperation() {}
 
@@ -66,8 +66,9 @@ void SessionManagerOperation::StartLoading() {
     return;
   is_loading_ = true;
   if (cloud_validations_) {
-    EnsurePublicKey(base::Bind(&SessionManagerOperation::RetrieveDeviceSettings,
-                               weak_factory_.GetWeakPtr()));
+    EnsurePublicKey(
+        base::BindOnce(&SessionManagerOperation::RetrieveDeviceSettings,
+                       weak_factory_.GetWeakPtr()));
   } else {
     RetrieveDeviceSettings();
   }
@@ -76,8 +77,8 @@ void SessionManagerOperation::StartLoading() {
 void SessionManagerOperation::LoadImmediately() {
   if (cloud_validations_) {
     StorePublicKey(
-        base::Bind(&SessionManagerOperation::BlockingRetrieveDeviceSettings,
-                   weak_factory_.GetWeakPtr()),
+        base::BindOnce(&SessionManagerOperation::BlockingRetrieveDeviceSettings,
+                       weak_factory_.GetWeakPtr()),
         LoadPublicKey(owner_key_util_, public_key_));
   } else {
     BlockingRetrieveDeviceSettings();
@@ -86,10 +87,10 @@ void SessionManagerOperation::LoadImmediately() {
 
 void SessionManagerOperation::ReportResult(
     DeviceSettingsService::Status status) {
-  callback_.Run(this, status);
+  std::move(callback_).Run(this, status);
 }
 
-void SessionManagerOperation::EnsurePublicKey(const base::Closure& callback) {
+void SessionManagerOperation::EnsurePublicKey(base::OnceClosure callback) {
   if (force_key_load_ || !public_key_ || !public_key_->is_loaded()) {
     base::ThreadPool::PostTaskAndReplyWithResult(
         FROM_HERE,
@@ -98,9 +99,9 @@ void SessionManagerOperation::EnsurePublicKey(const base::Closure& callback) {
         base::BindOnce(&SessionManagerOperation::LoadPublicKey, owner_key_util_,
                        force_key_load_ ? nullptr : public_key_),
         base::BindOnce(&SessionManagerOperation::StorePublicKey,
-                       weak_factory_.GetWeakPtr(), callback));
+                       weak_factory_.GetWeakPtr(), std::move(callback)));
   } else {
-    callback.Run();
+    std::move(callback).Run();
   }
 }
 
@@ -122,7 +123,7 @@ scoped_refptr<PublicKey> SessionManagerOperation::LoadPublicKey(
   return public_key;
 }
 
-void SessionManagerOperation::StorePublicKey(const base::Closure& callback,
+void SessionManagerOperation::StorePublicKey(base::OnceClosure callback,
                                              scoped_refptr<PublicKey> new_key) {
   force_key_load_ = false;
   public_key_ = new_key;
@@ -132,7 +133,7 @@ void SessionManagerOperation::StorePublicKey(const base::Closure& callback,
     return;
   }
 
-  callback.Run();
+  std::move(callback).Run();
 }
 
 void SessionManagerOperation::RetrieveDeviceSettings() {
@@ -224,8 +225,8 @@ void SessionManagerOperation::ReportValidatorStatus(
 LoadSettingsOperation::LoadSettingsOperation(bool force_key_load,
                                              bool cloud_validations,
                                              bool force_immediate_load,
-                                             const Callback& callback)
-    : SessionManagerOperation(callback) {
+                                             Callback callback)
+    : SessionManagerOperation(std::move(callback)) {
   force_key_load_ = force_key_load;
   cloud_validations_ = cloud_validations;
   force_immediate_load_ = force_immediate_load;
@@ -241,9 +242,9 @@ void LoadSettingsOperation::Run() {
 }
 
 StoreSettingsOperation::StoreSettingsOperation(
-    const Callback& callback,
+    Callback callback,
     std::unique_ptr<em::PolicyFetchResponse> policy)
-    : SessionManagerOperation(callback), policy_(std::move(policy)) {
+    : SessionManagerOperation(std::move(callback)), policy_(std::move(policy)) {
   if (policy_->has_new_public_key())
     force_key_load_ = true;
 }

chrome/browser/chromeos/settings/session_manager_operation.h

@@ -34,11 +34,11 @@ namespace chromeos {
 // are subclasses for loading, storing and signing policy blobs.
 class SessionManagerOperation {
  public:
-  typedef base::Callback<void(SessionManagerOperation*,
-                              DeviceSettingsService::Status)> Callback;
+  using Callback = base::OnceCallback<void(SessionManagerOperation*,
+                                           DeviceSettingsService::Status)>;
 
   // Creates a new load operation.
-  explicit SessionManagerOperation(const Callback& callback);
+  explicit SessionManagerOperation(Callback callback);
   virtual ~SessionManagerOperation();
 
   // Starts the operation.
@@ -69,7 +69,7 @@ class SessionManagerOperation {
   virtual void Run() = 0;
 
   // Ensures the public key is loaded.
-  void EnsurePublicKey(const base::Closure& callback);
+  void EnsurePublicKey(base::OnceClosure callback);
 
   // Starts a load operation.
   void StartLoading();
@@ -101,7 +101,7 @@ class SessionManagerOperation {
       scoped_refptr<ownership::PublicKey> current_key);
 
   // Stores the owner key loaded by LoadOwnerKey and calls |callback|.
-  void StorePublicKey(const base::Closure& callback,
+  void StorePublicKey(base::OnceClosure callback,
                       scoped_refptr<ownership::PublicKey> new_key);
 
   // Triggers a device settings load.
@@ -146,7 +146,7 @@ class LoadSettingsOperation : public SessionManagerOperation {
   LoadSettingsOperation(bool force_key_load,
                         bool cloud_validations,
                         bool force_immediate_load,
-                        const Callback& callback);
+                        Callback callback);
   ~LoadSettingsOperation() override;
 
  protected:
@@ -163,7 +163,7 @@ class StoreSettingsOperation : public SessionManagerOperation {
  public:
   // Creates a new store operation.
   StoreSettingsOperation(
-      const Callback& callback,
+      Callback callback,
       std::unique_ptr<enterprise_management::PolicyFetchResponse> policy);
   ~StoreSettingsOperation() override;
 

chrome/browser/chromeos/settings/session_manager_operation_unittest.cc

@@ -130,8 +130,8 @@ TEST_F(SessionManagerOperationTest, LoadNoPolicyNoKey) {
   LoadSettingsOperation op(
       false /* force_key_load */, true /* cloud_validations */,
       false /* force_immediate_load */,
-      base::Bind(&SessionManagerOperationTest::OnOperationCompleted,
-                 base::Unretained(this)));
+      base::BindOnce(&SessionManagerOperationTest::OnOperationCompleted,
+                     base::Unretained(this)));
 
   EXPECT_CALL(*this,
               OnOperationCompleted(
@@ -151,8 +151,8 @@ TEST_F(SessionManagerOperationTest, LoadOwnerKey) {
   LoadSettingsOperation op(
       false /* force_key_load */, true /* cloud_validations */,
       false /* force_immediate_load */,
-      base::Bind(&SessionManagerOperationTest::OnOperationCompleted,
-                 base::Unretained(this)));
+      base::BindOnce(&SessionManagerOperationTest::OnOperationCompleted,
+                     base::Unretained(this)));
 
   EXPECT_CALL(*this,
               OnOperationCompleted(
@@ -170,8 +170,8 @@ TEST_F(SessionManagerOperationTest, LoadPolicy) {
   LoadSettingsOperation op(
       false /* force_key_load */, true /* cloud_validations */,
       false /* force_immediate_load */,
-      base::Bind(&SessionManagerOperationTest::OnOperationCompleted,
-                 base::Unretained(this)));
+      base::BindOnce(&SessionManagerOperationTest::OnOperationCompleted,
+                     base::Unretained(this)));
 
   EXPECT_CALL(*this,
               OnOperationCompleted(&op, DeviceSettingsService::STORE_SUCCESS));
@@ -193,8 +193,8 @@ TEST_F(SessionManagerOperationTest, LoadImmediately) {
   LoadSettingsOperation op(
       false /* force_key_load */, true /* cloud_validations */,
       true /* force_immediate_load */,
-      base::Bind(&SessionManagerOperationTest::OnOperationCompleted,
-                 base::Unretained(this)));
+      base::BindOnce(&SessionManagerOperationTest::OnOperationCompleted,
+                     base::Unretained(this)));
 
   EXPECT_CALL(*this,
               OnOperationCompleted(
@@ -217,8 +217,8 @@ TEST_F(SessionManagerOperationTest, RestartLoad) {
   LoadSettingsOperation op(
       false /* force_key_load */, true /* cloud_validations */,
       false /* force_immediate_load */,
-      base::Bind(&SessionManagerOperationTest::OnOperationCompleted,
-                 base::Unretained(this)));
+      base::BindOnce(&SessionManagerOperationTest::OnOperationCompleted,
+                     base::Unretained(this)));
 
   // Just after the first RetrieveDevicePolicy() completion,
   // verify the state, install a different key, then RestartLoad().
@@ -272,8 +272,8 @@ TEST_F(SessionManagerOperationTest, RestartLoad) {
 TEST_F(SessionManagerOperationTest, StoreSettings) {
   owner_key_util_->SetPublicKeyFromPrivateKey(*policy_.GetSigningKey());
   StoreSettingsOperation op(
-      base::Bind(&SessionManagerOperationTest::OnOperationCompleted,
-                 base::Unretained(this)),
+      base::BindOnce(&SessionManagerOperationTest::OnOperationCompleted,
+                     base::Unretained(this)),
       policy_.GetCopy());
 
   EXPECT_CALL(*this,

chrome/browser/chromeos/settings/shutdown_policy_handler.cc

@@ -18,8 +18,9 @@ ShutdownPolicyHandler::ShutdownPolicyHandler(CrosSettings* cros_settings,
   DCHECK(delegate);
   shutdown_policy_subscription_ = cros_settings_->AddSettingsObserver(
       kRebootOnShutdown,
-      base::Bind(&ShutdownPolicyHandler::NotifyDelegateWithShutdownPolicy,
-                 weak_factory_.GetWeakPtr()));
+      base::BindRepeating(
+          &ShutdownPolicyHandler::NotifyDelegateWithShutdownPolicy,
+          weak_factory_.GetWeakPtr()));
 }
 
 ShutdownPolicyHandler::~ShutdownPolicyHandler() {}

chrome/browser/chromeos/settings/stub_cros_settings_provider_unittest.cc

@@ -19,9 +19,8 @@ class StubCrosSettingsProviderTest : public testing::Test {
  protected:
   StubCrosSettingsProviderTest()
       : provider_(new StubCrosSettingsProvider(
-          base::Bind(&StubCrosSettingsProviderTest::FireObservers,
-                     base::Unretained(this)))) {
-  }
+            base::BindRepeating(&StubCrosSettingsProviderTest::FireObservers,
+                                base::Unretained(this)))) {}
 
   ~StubCrosSettingsProviderTest() override {}