arc: Remove redundant overload

There are two overlods of OnProvisioningFinished and one is used mostly by tests.
The two internal references are switched to using the remaining overload.

BUG=b:62613791
TEST=unit tests

Change-Id: I8df3b031355e9b3262fb4efa0f33f29cd8c1eb23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2328017Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Muhammad Hasan Khan <mhasank@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793107}

chrome/browser/chromeos/arc/intent_helper/arc_settings_service_unittest.cc

@@ -163,7 +163,8 @@ TEST_F(ArcSettingsServiceTest,
   EXPECT_FALSE(
       profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
 
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
 
   EXPECT_TRUE(
       profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));
@@ -201,7 +202,8 @@ TEST_F(ArcSettingsServiceTest,
                   ->GetBroadcastsForAction(kActionLocaionEnabled)
                   .empty());
 
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
 
   EXPECT_FALSE(
       profile()->GetPrefs()->GetBoolean(prefs::kArcInitialSettingsPending));

chrome/browser/chromeos/arc/notification/arc_provision_notification_service_unittest.cc

@@ -120,7 +120,8 @@ TEST_F(ArcProvisionNotificationServiceTest,
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager_->state());
 
   // Emulate successful provisioning. The notification gets removed.
-  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                               nullptr);
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
 }
@@ -151,7 +152,8 @@ TEST_F(ArcProvisionNotificationServiceTest,
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager_->state());
-  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                               nullptr);
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
 }
@@ -183,7 +185,7 @@ TEST_F(ArcProvisionNotificationServiceTest,
   // Emulate provisioning failure that leads to stopping ARC. The notification
   // gets removed.
   arc_session_manager_->OnProvisioningFinished(
-      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR);
+      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR, nullptr);
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
 }
@@ -215,7 +217,7 @@ TEST_F(ArcProvisionNotificationServiceTest,
   // Emulate provisioning failure that leads to showing an error screen without
   // shutting ARC down. The notification gets removed.
   arc_session_manager_->OnProvisioningFinished(
-      ProvisioningResult::NO_NETWORK_CONNECTION);
+      ProvisioningResult::NO_NETWORK_CONNECTION, nullptr);
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
 }
@@ -246,7 +248,8 @@ TEST_F(ArcProvisionNotificationServiceTest,
   // Emulate successful provisioning.
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
-  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                               nullptr);
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
 }
@@ -309,7 +312,8 @@ TEST_F(ArcProvisionNotificationServiceOobeTest,
   // Emulate successful provisioning.
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
-  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager_->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                               nullptr);
   EXPECT_FALSE(
       display_service_->GetNotification(kArcManagedProvisionNotificationId));
 }

chrome/browser/chromeos/arc/session/arc_session_manager.cc

@@ -532,7 +532,7 @@ void ArcSessionManager::OnSessionStopped(ArcStopReason reason,
 
   // TODO(crbug.com/625923): Use |reason| to report more detailed errors.
   if (arc_sign_in_timer_.IsRunning())
-    OnProvisioningFinished(ProvisioningResult::ARC_STOPPED);
+    OnProvisioningFinished(ProvisioningResult::ARC_STOPPED, nullptr);
 
   for (auto& observer : observer_list_)
     observer.OnArcSessionStopped(reason);
@@ -545,11 +545,6 @@ void ArcSessionManager::OnSessionRestarting() {
     observer.OnArcSessionRestarting();
 }
 
-void ArcSessionManager::OnProvisioningFinished(ProvisioningResult result) {
-  // TODO(mhasank) update all callers to use the second overload
-  OnProvisioningFinished(result, nullptr);
-}
-
 void ArcSessionManager::OnProvisioningFinished(
     ProvisioningResult result,
     mojom::ArcSignInErrorPtr signin_error) {
@@ -912,7 +907,7 @@ void ArcSessionManager::StopAndEnableArc() {
 
 void ArcSessionManager::OnArcSignInTimeout() {
   LOG(ERROR) << "Timed out waiting for first sign in.";
-  OnProvisioningFinished(ProvisioningResult::OVERALL_SIGN_IN_TIMEOUT);
+  OnProvisioningFinished(ProvisioningResult::OVERALL_SIGN_IN_TIMEOUT, nullptr);
 }
 
 void ArcSessionManager::CancelAuthCode() {

chrome/browser/chromeos/arc/session/arc_session_manager.h

@@ -207,8 +207,6 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   // On provisioning completion (regardless of whether successfully done or
   // not), this is called with its status. On success, called with
   // ProvisioningResult::SUCCESS, otherwise |result| is the error reason.
-  void OnProvisioningFinished(ProvisioningResult result);
-
   void OnProvisioningFinished(ProvisioningResult result,
                               mojom::ArcSignInErrorPtr error);
 

chrome/browser/chromeos/arc/session/arc_session_manager_unittest.cc

@@ -402,7 +402,8 @@ TEST_F(ArcSessionManagerTest, ArcInitialStartFirstProvisioning) {
 
   EXPECT_FALSE(start_handler.was_called());
 
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
   EXPECT_TRUE(start_handler.was_called());
 
   arc_session_manager()->Shutdown();
@@ -423,7 +424,8 @@ TEST_F(ArcSessionManagerTest, ArcInitialStartNextProvisioning) {
   ArcInitialStartHandler start_handler(arc_session_manager());
 
   arc_session_manager()->RequestEnable();
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
   EXPECT_FALSE(start_handler.was_called());
 
   arc_session_manager()->Shutdown();
@@ -547,7 +549,8 @@ TEST_F(ArcSessionManagerTest, Provisioning_Success) {
   EXPECT_FALSE(arc_session_manager()->IsPlaystoreLaunchRequestedForTesting());
 
   // Emulate successful provisioning.
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
   EXPECT_TRUE(prefs->GetBoolean(prefs::kArcSignedIn));
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
   EXPECT_TRUE(arc_session_manager()->IsPlaystoreLaunchRequestedForTesting());
@@ -570,7 +573,8 @@ TEST_F(ArcSessionManagerTest, PlayStoreSuppressed) {
   // Second start, no fetching code is expected.
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
   EXPECT_FALSE(arc_session_manager()->IsPlaystoreLaunchRequestedForTesting());
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
   // Completing the provisioning resets this flag.
   EXPECT_FALSE(prefs->GetBoolean(prefs::kArcProvisioningInitiatedFromOobe));
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
@@ -618,7 +622,7 @@ TEST_F(ArcSessionManagerTest, Provisioning_Restart) {
 
   // Report failure.
   arc_session_manager()->OnProvisioningFinished(
-      ProvisioningResult::GMS_NETWORK_ERROR);
+      ProvisioningResult::GMS_NETWORK_ERROR, nullptr);
   // On error, UI to send feedback is showing. In that case,
   // the ARC is still necessary to run on background for gathering the logs.
   EXPECT_TRUE(prefs->GetBoolean(prefs::kArcSignedIn));
@@ -750,7 +754,8 @@ TEST_F(ArcSessionManagerTest, ClearArcTransitionOnShutdown) {
             arc_session_manager()->state());
   arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
   arc_session_manager()->StartArcForTesting();
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
 
   EXPECT_EQ(
       static_cast<int>(ArcSupervisionTransition::NO_TRANSITION),
@@ -785,7 +790,8 @@ TEST_F(ArcSessionManagerTest, ClearArcTransitionOnArcDataRemoval) {
             arc_session_manager()->state());
   arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
   arc_session_manager()->StartArcForTesting();
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
 
   EXPECT_EQ(ArcSupervisionTransition::NO_TRANSITION,
             arc::GetSupervisionTransition(profile()));
@@ -812,13 +818,13 @@ TEST_F(ArcSessionManagerTest, IgnoreSecondErrorReporting) {
 
   // Report some failure that does not stop the bridge.
   arc_session_manager()->OnProvisioningFinished(
-      ProvisioningResult::GMS_SIGN_IN_FAILED);
+      ProvisioningResult::GMS_SIGN_IN_FAILED, nullptr);
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
 
   // Try to send another error that stops the bridge if sent first. It should
   // be ignored.
   arc_session_manager()->OnProvisioningFinished(
-      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR);
+      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR, nullptr);
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
 
   arc_session_manager()->Shutdown();
@@ -838,7 +844,8 @@ TEST_F(ArcSessionManagerTest, IsDirectlyStartedFalse) {
             arc_session_manager()->state());
   arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
   arc_session_manager()->StartArcForTesting();
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
   EXPECT_FALSE(arc_session_manager()->is_directly_started());
   arc_session_manager()->Shutdown();
@@ -877,7 +884,8 @@ TEST_F(ArcSessionManagerTest, IsDirectlyStartedOnInternalRestart) {
             arc_session_manager()->state());
   arc_session_manager()->OnTermsOfServiceNegotiatedForTesting(true);
   arc_session_manager()->StartArcForTesting();
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
   EXPECT_FALSE(arc_session_manager()->is_directly_started());
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
   EXPECT_FALSE(arc_session_manager()->is_directly_started());
@@ -1137,7 +1145,8 @@ TEST_P(ArcSessionManagerPolicyTest, SkippingTerms) {
 
   arc_session_manager()->StartArcForTesting();
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
 
   // Play Store app is launched unless the Terms screen was suppressed or Tos is
   // accepted during OOBE.
@@ -1215,7 +1224,7 @@ TEST_F(ArcSessionManagerKioskTest, AuthFailure) {
       base::Bind([](bool* terminated) { *terminated = true; }, &terminated));
 
   arc_session_manager()->OnProvisioningFinished(
-      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR);
+      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR, nullptr);
   EXPECT_TRUE(terminated);
 }
 
@@ -1251,7 +1260,7 @@ TEST_F(ArcSessionManagerPublicSessionTest, AuthFailure) {
                           &terminated));
 
   arc_session_manager()->OnProvisioningFinished(
-      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR);
+      ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR, nullptr);
   EXPECT_FALSE(terminated);
   EXPECT_EQ(ArcSessionManager::State::STOPPED, arc_session_manager()->state());
 }
@@ -1525,7 +1534,7 @@ TEST_P(ArcSessionRetryTest, ContainerRestarted) {
   arc_session_manager()->StartArcForTesting();
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
 
-  arc_session_manager()->OnProvisioningFinished(GetParam().error);
+  arc_session_manager()->OnProvisioningFinished(GetParam().error, nullptr);
 
   // In case of permanent error data removal request is scheduled.
   EXPECT_EQ(GetParam().data_removed,
@@ -1553,7 +1562,8 @@ TEST_P(ArcSessionRetryTest, ContainerRestarted) {
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
 
   // Successful retry keeps ARC++ container running.
-  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS);
+  arc_session_manager()->OnProvisioningFinished(ProvisioningResult::SUCCESS,
+                                                nullptr);
   EXPECT_EQ(ArcSessionManager::State::ACTIVE, arc_session_manager()->state());
 
   arc_session_manager()->Shutdown();