Rename CloudPolicyService::Observer method for clarity

This is a drive-by cleanup: On tonikitoo's chain of CLs converting
UserCloudPolicyTokenForwarder to IdentityManager, I noticed the
CloudPolicyService::Observer::OnInitializationComplete() method. This
method is ambiguously named from a client POV, as their override
simply ends up being Foo::OnInitializationComplete(). This was
especially weird from the POV of UserCloudPolicyTokenForwarder because
its implementation of that method starts its *own* initialization, i.e.,
invokes Initialize() ;).

This CL renames the method to
OnCloudPolicyServiceInitializationComplete() and additionally eliminates
the CloudPolicyService* argument, which is not needed: its only usage
is to have a few clients confirm that it is the same as the instance on
which they registered as an observer, but this will always be the case
by the nature of observer callbacks.

Change-Id: Iaf73c26b9b4ad1305ad6774c23da0828200d499c
Reviewed-on: https://chromium-review.googlesource.com/c/1329146Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607183}

chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc

@@ -266,7 +266,7 @@ void UserCloudPolicyManagerChromeOS::Connect(
 
     // Initialization has completed before our observer was registered
     // so invoke our callback directly.
-    OnInitializationCompleted(service());
+    OnCloudPolicyServiceInitializationCompleted();
   } else {
     // Wait for the CloudPolicyStore to finish initializing.
     service()->AddObserver(this);
@@ -352,10 +352,9 @@ bool UserCloudPolicyManagerChromeOS::IsInitializationComplete(
   return true;
 }
 
-void UserCloudPolicyManagerChromeOS::OnInitializationCompleted(
-    CloudPolicyService* cloud_policy_service) {
-  DCHECK_EQ(service(), cloud_policy_service);
-  cloud_policy_service->RemoveObserver(this);
+void UserCloudPolicyManagerChromeOS::
+    OnCloudPolicyServiceInitializationCompleted() {
+  service()->RemoveObserver(this);
 
   time_init_completed_ = base::Time::Now();
   UMA_HISTOGRAM_MEDIUM_TIMES(kUMADelayInitialization,

chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h

@@ -140,7 +140,7 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
   bool IsInitializationComplete(PolicyDomain domain) const override;
 
   // CloudPolicyService::Observer:
-  void OnInitializationCompleted(CloudPolicyService* service) override;
+  void OnCloudPolicyServiceInitializationCompleted() override;
 
   // CloudPolicyClient::Observer:
   void OnPolicyFetched(CloudPolicyClient* client) override;
@@ -259,7 +259,8 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
   std::unique_ptr<WildcardLoginChecker> wildcard_login_checker_;
 
   // The access token passed to OnAccessTokenAvailable. It is stored here so
-  // that it can be used if OnInitializationCompleted is called later.
+  // that it can be used if OnCloudPolicyServiceInitializationCompleted is
+  // called later.
   std::string access_token_;
 
   // Timestamps for collecting timing UMA stats.

chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc

@@ -33,8 +33,8 @@ void UserCloudPolicyTokenForwarder::Shutdown() {
   manager_->core()->service()->RemoveObserver(this);
 }
 
-void UserCloudPolicyTokenForwarder::OnInitializationCompleted(
-    CloudPolicyService* service) {
+void UserCloudPolicyTokenForwarder::
+    OnCloudPolicyServiceInitializationCompleted() {
   Initialize();
 }
 

chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.h

@@ -40,7 +40,7 @@ class UserCloudPolicyTokenForwarder
   void Shutdown() override;
 
   // CloudPolicyService::Observer:
-  void OnInitializationCompleted(CloudPolicyService* service) override;
+  void OnCloudPolicyServiceInitializationCompleted() override;
 
  private:
   void Initialize();

chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.cc

@@ -119,8 +119,8 @@ void MachineLevelUserCloudPolicyFetcher::SetupRegistrationAndFetchPolicy(
       base::BindRepeating(&OnPolicyFetchCompleted));
 }
 
-void MachineLevelUserCloudPolicyFetcher::OnInitializationCompleted(
-    CloudPolicyService* service) {
+void MachineLevelUserCloudPolicyFetcher::
+    OnCloudPolicyServiceInitializationCompleted() {
   // Client will be registered before policy fetch. A non-registered client
   // means there is no validated policy cache on the disk while the device has
   // been enrolled already. Hence, we need to fetch the
@@ -129,8 +129,8 @@ void MachineLevelUserCloudPolicyFetcher::OnInitializationCompleted(
   // Note that Chrome will not fetch policy again immediately here if DM server
   // returns a policy that Chrome is not able to validate.
   if (!policy_manager_->IsClientRegistered()) {
-    VLOG(1) << "OnInitializationCompleted: Fetching policy when there is no "
-               "valid local cache.";
+    VLOG(1) << "OnCloudPolicyServiceInitializationCompleted: Fetching policy "
+               "when there is no valid local cache.";
     TryToFetchPolicy();
   }
 }
@@ -140,9 +140,10 @@ void MachineLevelUserCloudPolicyFetcher::InitializeManager(
   policy_manager_->Connect(local_state_, std::move(client));
   policy_manager_->core()->service()->AddObserver(this);
 
-  // If CloudPolicyStore is already initialized then |OnInitializationCompleted|
-  // has already fired. Fetch policy if CloudPolicyClient hasn't been registered
-  // which means there is no valid policy cache.
+  // If CloudPolicyStore is already initialized then
+  // |OnCloudPolicyServiceInitializationCompleted| has already fired. Fetch
+  // policy if CloudPolicyClient hasn't been registered which means there is no
+  // valid policy cache.
   if (policy_manager_->store()->is_initialized() &&
       !policy_manager_->IsClientRegistered()) {
     VLOG(1) << "InitializeManager: Fetching policy when there is no valid "

chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.h

@@ -76,7 +76,7 @@ class MachineLevelUserCloudPolicyFetcher : public CloudPolicyService::Observer {
                                        const std::string& client_id);
 
   // CloudPolicyService::Observer:
-  void OnInitializationCompleted(CloudPolicyService* service) override;
+  void OnCloudPolicyServiceInitializationCompleted() override;
 
  private:
   void InitializeManager(std::unique_ptr<CloudPolicyClient> client);

chrome/browser/policy/cloud/user_policy_signin_service.cc

@@ -179,11 +179,9 @@ void UserPolicySigninService::ShutdownUserCloudPolicyManager() {
   UserPolicySigninServiceBase::ShutdownUserCloudPolicyManager();
 }
 
-void UserPolicySigninService::OnInitializationCompleted(
-    CloudPolicyService* service) {
+void UserPolicySigninService::OnCloudPolicyServiceInitializationCompleted() {
   UserCloudPolicyManager* manager = policy_manager();
-  DCHECK_EQ(service, manager->core()->service());
-  DCHECK(service->IsInitializationComplete());
+  DCHECK(manager->core()->service()->IsInitializationComplete());
   // The service is now initialized - if the client is not yet registered, then
   // it means that there is no cached policy and so we need to initiate a new
   // client registration.

chrome/browser/policy/cloud/user_policy_signin_service.h

@@ -66,7 +66,7 @@ class UserPolicySigninService : public UserPolicySigninServiceBase {
                                        bool is_valid) override;
 
   // CloudPolicyService::Observer implementation:
-  void OnInitializationCompleted(CloudPolicyService* service) override;
+  void OnCloudPolicyServiceInitializationCompleted() override;
 
  protected:
   // UserPolicySigninServiceBase implementation:

chrome/browser/policy/cloud/user_policy_signin_service_base.cc

@@ -97,8 +97,8 @@ void UserPolicySigninServiceBase::Observe(
   InitializeOnProfileReady(content::Source<Profile>(source).ptr());
 }
 
-void UserPolicySigninServiceBase::OnInitializationCompleted(
-    CloudPolicyService* service) {
+void UserPolicySigninServiceBase::
+    OnCloudPolicyServiceInitializationCompleted() {
   // This is meant to be overridden by subclasses. Starting and stopping to
   // observe the CloudPolicyService from this base class avoids the need for
   // more virtuals.
@@ -213,8 +213,8 @@ void UserPolicySigninServiceBase::InitializeForSignedInUser(
   // Initialize the UCPM if it is not already initialized.
   if (!manager->core()->service()) {
     // If there is no cached DMToken then we can detect this when the
-    // OnInitializationCompleted() callback is invoked and this will
-    // initiate a policy fetch.
+    // OnCloudPolicyServiceInitializationCompleted() callback is invoked and
+    // this will initiate a policy fetch.
     InitializeUserCloudPolicyManager(
         account_id,
         UserCloudPolicyManager::CreateCloudPolicyClient(
@@ -224,10 +224,10 @@ void UserPolicySigninServiceBase::InitializeForSignedInUser(
   }
 
   // If the CloudPolicyService is initialized, kick off registration.
-  // Otherwise OnInitializationCompleted is invoked as soon as the service
-  // finishes its initialization.
+  // Otherwise OnCloudPolicyServiceInitializationCompleted is invoked as soon as
+  // the service finishes its initialization.
   if (manager->core()->service()->IsInitializationComplete())
-    OnInitializationCompleted(manager->core()->service());
+    OnCloudPolicyServiceInitializationCompleted();
 }
 
 void UserPolicySigninServiceBase::InitializeUserCloudPolicyManager(

chrome/browser/policy/cloud/user_policy_signin_service_base.h

@@ -94,7 +94,7 @@ class UserPolicySigninServiceBase : public KeyedService,
                const content::NotificationDetails& details) override;
 
   // CloudPolicyService::Observer implementation:
-  void OnInitializationCompleted(CloudPolicyService* service) override;
+  void OnCloudPolicyServiceInitializationCompleted() override;
 
   // CloudPolicyClient::Observer implementation:
   void OnPolicyFetched(CloudPolicyClient* client) override;

chrome/browser/policy/cloud/user_policy_signin_service_mobile.cc

@@ -104,11 +104,9 @@ void UserPolicySigninService::Shutdown() {
   UserPolicySigninServiceBase::Shutdown();
 }
 
-void UserPolicySigninService::OnInitializationCompleted(
-    CloudPolicyService* service) {
+void UserPolicySigninService::OnCloudPolicyServiceInitializationCompleted() {
   UserCloudPolicyManager* manager = policy_manager();
-  DCHECK_EQ(service, manager->core()->service());
-  DCHECK(service->IsInitializationComplete());
+  DCHECK(manager->core()->service()->IsInitializationComplete());
   // The service is now initialized - if the client is not yet registered, then
   // it means that there is no cached policy and so we need to initiate a new
   // client registration.

chrome/browser/policy/cloud/user_policy_signin_service_mobile.h

@@ -72,7 +72,7 @@ class UserPolicySigninService : public UserPolicySigninServiceBase {
   void Shutdown() override;
 
   // CloudPolicyService::Observer implementation:
-  void OnInitializationCompleted(CloudPolicyService* service) override;
+  void OnCloudPolicyServiceInitializationCompleted() override;
 
   // Registers for cloud policy for an already signed-in user.
   void RegisterCloudPolicyService();

components/policy/core/common/cloud/cloud_policy_service.cc

@@ -211,7 +211,7 @@ void CloudPolicyService::CheckInitializationCompleted() {
   if (!IsInitializationComplete() && store_->is_initialized()) {
     initialization_complete_ = true;
     for (auto& observer : observers_)
-      observer.OnInitializationCompleted(this);
+      observer.OnCloudPolicyServiceInitializationCompleted();
   }
 }
 

components/policy/core/common/cloud/cloud_policy_service.h

@@ -37,7 +37,7 @@ class POLICY_EXPORT CloudPolicyService : public CloudPolicyClient::Observer,
     // Invoked when CloudPolicyService has finished initializing (any initial
     // policy load activity has completed and the CloudPolicyClient has
     // been registered, if possible).
-    virtual void OnInitializationCompleted(CloudPolicyService* service) = 0;
+    virtual void OnCloudPolicyServiceInitializationCompleted() = 0;
     virtual ~Observer() {}
   };
 

components/policy/core/common/cloud/cloud_policy_service_unittest.cc

@@ -26,7 +26,8 @@ class MockCloudPolicyServiceObserver : public CloudPolicyService::Observer {
   MockCloudPolicyServiceObserver() {}
   ~MockCloudPolicyServiceObserver() override {}
 
-  MOCK_METHOD1(OnInitializationCompleted, void(CloudPolicyService* service));
+  MOCK_METHOD0(OnCloudPolicyServiceInitializationCompleted, void());
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MockCloudPolicyServiceObserver);
 };
@@ -260,14 +261,14 @@ TEST_F(CloudPolicyServiceTest, StoreLoadAfterCreation) {
   MockCloudPolicyServiceObserver observer;
   service_.AddObserver(&observer);
   // Service should be marked as initialized and observer should be called back.
-  EXPECT_CALL(observer, OnInitializationCompleted(&service_)).Times(1);
+  EXPECT_CALL(observer, OnCloudPolicyServiceInitializationCompleted()).Times(1);
   store_.NotifyStoreLoaded();
   EXPECT_TRUE(service_.IsInitializationComplete());
   testing::Mock::VerifyAndClearExpectations(&observer);
 
   // Now, the next time the store is loaded, the observer should not be called
   // again.
-  EXPECT_CALL(observer, OnInitializationCompleted(&service_)).Times(0);
+  EXPECT_CALL(observer, OnCloudPolicyServiceInitializationCompleted()).Times(0);
   store_.NotifyStoreLoaded();
   service_.RemoveObserver(&observer);
 }