Remove delegates from ArcAuthCodeFetcher and ArcAuthContext.

chrome/browser/chromeos/arc/arc_auth_service.cc

 // Copyright 2015 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 "chrome/browser/chromeos/arc/arc_auth_service.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 371 matching lines @@
   // No other auth code-related operation may be in progress.
   DCHECK(!account_info_notifier_);
 
   if (IsOptInVerificationDisabled()) {
     account_info_notifier->Notify(false /* = is_enforced */, std::string(),
                                   GetAccountType(),
                                   policy_util::IsAccountManaged(profile_));
     return;
   }
 
+  // Hereafter asynchronous operation. Remember the notifier.
   account_info_notifier_ = std::move(account_info_notifier);
 
+  // In Kiosk mode, use Robot auth code fetching.
   if (IsArcKioskMode()) {
     arc_robot_auth_.reset(new ArcRobotAuth());
     arc_robot_auth_->FetchRobotAuthCode(
         base::Bind(&ArcAuthService::OnRobotAuthCodeFetched,
                    weak_ptr_factory_.GetWeakPtr()));
-  } else {
-    PrepareContextForAuthCodeRequest();
+    return;
   }
+
+  // If endpoint is passed via command line flag, use automatic auth code
+  // fetching.
+  const base::CommandLine* command_line =
+      base::CommandLine::ForCurrentProcess();
+  std::string auth_endpoint;
+  if (command_line->HasSwitch(chromeos::switches::kArcUseAuthEndpoint)) {
+    auth_endpoint = command_line->GetSwitchValueASCII(
+        chromeos::switches::kArcUseAuthEndpoint);
+  }
+  if (!auth_endpoint.empty()) {

[Luis Héctor Chávez, 2016/11/15 23:27:37]

IIUC this can only be true in L411. Is it possible to move this block there?
(possibly even without the condition, because it doesn't make sense to set that
flag to an empty value?)

Furthermore, auth_endpoint can also be declared and initialized in L410.

[hidehiko, 2016/11/16 01:00:34]

Done. Can we work on making L411 DCHECK() in a separate CL, just in case? I'd
like to avoid the risk to revert the CL just for it.

[Luis Héctor Chávez, 2016/11/16 03:29:58]

sgtm

+    DCHECK(!auth_code_fetcher_);
+    auth_code_fetcher_ = base::MakeUnique<ArcAuthCodeFetcher>(
+        profile_, context_.get(), auth_endpoint);
+    auth_code_fetcher_->Fetch(base::Bind(&ArcAuthService::OnAuthCodeFetched,
+                                         weak_ptr_factory_.GetWeakPtr()));
+    return;
+  }
+
+  // Otherwise, show LSO page to user, and let them click "Sign in" button.
+  if (support_host_)
+    support_host_->ShowLso();
 }
 
 void ArcAuthService::OnRobotAuthCodeFetched(
     const std::string& robot_auth_code) {
   // We fetching robot auth code for ARC kiosk only.
   DCHECK(IsArcKioskMode());
 
   // Current instance of ArcRobotAuth became useless.
   arc_robot_auth_.reset();
 
   if (robot_auth_code.empty()) {
     VLOG(1) << "Robot account auth code fetching error";
     // Log out the user. All the cleanup will be done in Shutdown() method.
     // The callback is not called because auth code is empty.
     chrome::AttemptUserExit();
     return;
   }
 
-  account_info_notifier_->Notify(
-      !IsOptInVerificationDisabled(), robot_auth_code,
-      mojom::ChromeAccountType::ROBOT_ACCOUNT, false);
-  account_info_notifier_.reset();
+  OnAuthCodeObtained(robot_auth_code);
 }
 
-bool ArcAuthService::IsAuthCodeRequest() const {
-  return account_info_notifier_ != nullptr;
-}
+void ArcAuthService::OnAuthCodeFetched(const std::string& auth_code) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  auth_code_fetcher_.reset();
 
-void ArcAuthService::PrepareContextForAuthCodeRequest() {
-  // Requesting auth code on demand happens in following cases:
-  // 1. To handle account password revoke.
-  // 2. In case Arc is activated in OOBE flow.
-  // 3. For any other state on Android side that leads device appears in
-  // non-signed state.
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(state_ == State::ACTIVE);
-  DCHECK(IsAuthCodeRequest());
-  DCHECK(!IsArcKioskMode());
-  context_->PrepareContext();
+  if (auth_code.empty()) {
+    OnProvisioningFinished(
+        ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR);
+    return;
+  }
+
+  OnAuthCodeObtained(auth_code);
 }
 
 void ArcAuthService::OnSignInComplete() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK_EQ(state_, State::ACTIVE);
   OnProvisioningFinished(ProvisioningResult::SUCCESS);
 }
 
 void ArcAuthService::OnSignInFailed(mojom::ArcSignInFailureReason reason) {
   OnProvisioningFinished(
@@ skipping 151 matching lines @@
     // This automatically updates all preferences.
     preference_handler_->Start();
   }
 
   DCHECK_EQ(State::NOT_INITIALIZED, state_);
   SetState(State::STOPPED);
 
   PrefServiceSyncableFromProfile(profile_)->AddSyncedPrefObserver(
       prefs::kArcEnabled, this);
 
-  context_.reset(new ArcAuthContext(this, profile_));
+  context_.reset(new ArcAuthContext(profile_));
 
   // In case UI is disabled we assume that ARC is opted-in. For ARC Kiosk we
   // skip ToS because it is very likely that near the device there will be
   // no one who is eligible to accept them. We skip if Android management check
   // because there are no managed human users for Kiosk exist.
   if (IsOptInVerificationDisabled() || IsArcKioskMode()) {
     StartArc();
     return;
   }
 
@@ skipping 36 matching lines @@
     pref_service_syncable->RemoveObserver(this);
     pref_service_syncable->RemoveSyncedPrefObserver(prefs::kArcEnabled, this);
   }
   pref_change_registrar_.RemoveAll();
   context_.reset();
   profile_ = nullptr;
   arc_robot_auth_.reset();
   SetState(State::NOT_INITIALIZED);
 }
 
-void ArcAuthService::OnContextReady() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  FetchAuthCode();
-}
-
 void ArcAuthService::OnSyncedPrefChanged(const std::string& path,
                                          bool from_sync) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // Update UMA only for local changes
   if (!from_sync) {
     const bool arc_enabled =
         profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled);
     UpdateOptInActionUMA(arc_enabled ? OptInActionType::OPTED_IN
                                      : OptInActionType::OPTED_OUT);
@@ skipping 203 matching lines @@
           ArcSupportHost::Error::SIGN_IN_SERVICE_UNAVAILABLE_ERROR, false);
     }
     return;
   }
 
   SetState(State::SHOWING_TERMS_OF_SERVICE);
   if (support_host_)
     support_host_->ShowTermsOfService();
 }
 
-void ArcAuthService::OnPrepareContextFailed() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  OnProvisioningFinished(ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR);
-}
-
-void ArcAuthService::OnAuthCodeSuccess(const std::string& auth_code) {
-  OnAuthCodeObtained(auth_code);
-}
-
-void ArcAuthService::OnAuthCodeFailed() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  OnProvisioningFinished(ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR);
-}
-
 void ArcAuthService::StartArcAndroidManagementCheck() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(arc_bridge_service()->stopped());
   DCHECK(state_ == State::SHOWING_TERMS_OF_SERVICE ||
          state_ == State::CHECKING_ANDROID_MANAGEMENT);
   SetState(State::CHECKING_ANDROID_MANAGEMENT);
 
   android_management_checker_.reset(new ArcAndroidManagementChecker(
       profile_, context_->token_service(), context_->account_id(),
       false /* retry_on_error */));
@@ skipping 45 matching lines @@
     case policy::AndroidManagementClient::Result::MANAGED:
       DisableArc();
       break;
     case policy::AndroidManagementClient::Result::ERROR:
       // This code should not be reached. For background check,
       // retry_on_error should be set.
       NOTREACHED();
   }
 }
 
-void ArcAuthService::FetchAuthCode() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
-  const base::CommandLine* command_line =
-      base::CommandLine::ForCurrentProcess();
-  std::string auth_endpoint;
-  if (command_line->HasSwitch(chromeos::switches::kArcUseAuthEndpoint)) {
-    auth_endpoint = command_line->GetSwitchValueASCII(
-        chromeos::switches::kArcUseAuthEndpoint);
-  }
-
-  if (!auth_endpoint.empty()) {
-    auth_code_fetcher_.reset(new ArcAuthCodeFetcher(
-        this, context_->GetURLRequestContext(), profile_, auth_endpoint));
-  } else {
-    if (support_host_)
-      support_host_->ShowLso();
-  }
-}
-
 void ArcAuthService::OnWindowClosed() {
   DCHECK(support_host_);
   CancelAuthCode();
 }
 
 void ArcAuthService::OnTermsAgreed(bool is_metrics_enabled,
                                    bool is_backup_and_restore_enabled,
                                    bool is_location_service_enabled) {
   DCHECK(support_host_);
 
@@ skipping 30 matching lines @@
     // ERROR_WITH_FEEDBACK is set in OnSignInFailed(). In the case, stopping
     // ARC was postponed to contain its internal state into the report.
     // Here, on retry, stop it, then restart.
     DCHECK_EQ(State::ACTIVE, state_);
     support_host_->ShowArcLoading();
     ShutdownBridge();
     reenable_arc_ = true;
   } else if (state_ == State::ACTIVE) {
     // This happens when ARC support Chrome app reports an error on "Sign in"
     // page.
-    // TODO(hidehiko): Currently, due to the existing code structure, we need
-    // to call PrepareContextForAuthCodeRequest() always. However, to fetch
-    // an authtoken via LSO page, it is not necessary to call PrepareContext().
-    // Instead, it is possible to show LSO page, immediately.
-    support_host_->ShowArcLoading();
-    PrepareContextForAuthCodeRequest();
+    support_host_->ShowLso();
   } else {
     // Otherwise, we restart ARC. Note: this is the first boot case.
     // For second or later boot, either ERROR_WITH_FEEDBACK case or ACTIVE
     // case must hit.
     support_host_->ShowArcLoading();
     StartArcAndroidManagementCheck();
   }
 }
 
 void ArcAuthService::OnSendFeedbackClicked() {
@@ skipping 33 matching lines @@
       return os << "ACTIVE";
   }
 
   // Some compiler reports an error even if all values of an enum-class are
   // covered indivisually in a switch statement.
   NOTREACHED();
   return os;
 }
 
 }  // namespace arc