Fix race issue in ArcAuthService.

chrome/browser/chromeos/arc/arc_auth_service.cc

Index: chrome/browser/chromeos/arc/arc_auth_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_service.cc b/chrome/browser/chromeos/arc/arc_auth_service.cc
index 3fc09a0d3bcca8e06c33b8a5e09e547e4e11436e..ace1785426ca6979497f02068631886517303173 100644
--- a/chrome/browser/chromeos/arc/arc_auth_service.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_service.cc
@@ -4,14 +4,17 @@
 
 #include "chrome/browser/chromeos/arc/arc_auth_service.h"
 
+#include <utility>
+
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
-#include "chrome/browser/chromeos/arc/arc_auth_code_fetcher.h"
-#include "chrome/browser/chromeos/arc/arc_optin_uma.h"
 #include "chrome/browser/chromeos/arc/arc_optin_uma.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
-#include "chrome/browser/chromeos/arc/auth/arc_robot_auth.h"
+#include "chrome/browser/chromeos/arc/auth/arc_auth_code_fetcher.h"
+#include "chrome/browser/chromeos/arc/auth/arc_background_auth_code_fetcher.h"
+#include "chrome/browser/chromeos/arc/auth/arc_manual_auth_code_fetcher.h"
+#include "chrome/browser/chromeos/arc/auth/arc_robot_auth_code_fetcher.h"
 #include "chrome/browser/chromeos/arc/policy/arc_policy_util.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chromeos/chromeos_switches.h"
@@ -148,12 +151,8 @@ void ArcAuthService::OnInstanceReady() {
 }
 
 void ArcAuthService::OnInstanceClosed() {
-  ArcSupportHost* support_host = ArcSessionManager::Get()->support_host();
-  if (support_host)
-    support_host->RemoveObserver(this);
+  fetcher_.reset();
   notifier_.reset();
-  arc_robot_auth_.reset();
-  auth_code_fetcher_.reset();
 }
 
 void ArcAuthService::OnSignInComplete() {
@@ -172,6 +171,14 @@ void ArcAuthService::RequestAccountInfo() {
                      weak_ptr_factory_.GetWeakPtr())));
 }
 
+void ArcAuthService::OnAccountInfoReady(mojom::AccountInfoPtr account_info) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  auto* instance = arc_bridge_service()->auth()->GetInstanceForMethod(
+      "OnAccountInfoReady", kMinVersionForOnAccountInfoReady);
+  DCHECK(instance);
+  instance->OnAccountInfoReady(std::move(account_info));
+}
+
 void ArcAuthService::GetAuthCodeDeprecated0(
     const GetAuthCodeDeprecated0Callback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -205,6 +212,7 @@ void ArcAuthService::RequestAccountInfoInternal(
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // No other auth code-related operation may be in progress.
   DCHECK(!notifier_);
+  DCHECK(!fetcher_);
 
   if (ArcSessionManager::IsOptInVerificationDisabled()) {
     notifier->Notify(
@@ -216,80 +224,30 @@ void ArcAuthService::RequestAccountInfoInternal(
   // Hereafter asynchronous operation. Remember the notifier.
   notifier_ = std::move(notifier);
 
-  // In Kiosk mode, use Robot auth code fetching.
   if (ArcSessionManager::IsArcKioskMode()) {
-    arc_robot_auth_.reset(new ArcRobotAuth());
-    arc_robot_auth_->FetchRobotAuthCode(
-        base::Bind(&ArcAuthService::OnRobotAuthCodeFetched,
-                   weak_ptr_factory_.GetWeakPtr()));
-    return;
-  }
-
-  // Optionally retrive auth code in silent mode.
-  if (base::FeatureList::IsEnabled(arc::kArcUseAuthEndpointFeature)) {
-    DCHECK(!auth_code_fetcher_);
-    auth_code_fetcher_ = base::MakeUnique<ArcAuthCodeFetcher>(
+    // In Kiosk mode, use Robot auth code fetching.
+    fetcher_ = base::MakeUnique<ArcRobotAuthCodeFetcher>();
+  } else if (base::FeatureList::IsEnabled(arc::kArcUseAuthEndpointFeature)) {
+    // Optionally retrive auth code in silent mode.

[Luis Héctor Chávez, 2016/12/03 01:27:54]

nit: s/retrive/retrieve/

[hidehiko, 2016/12/05 06:00:31]

Done.

+    fetcher_ = base::MakeUnique<ArcBackgroundAuthCodeFetcher>(
         ArcSessionManager::Get()->profile(),
         ArcSessionManager::Get()->auth_context());
-    auth_code_fetcher_->Fetch(base::Bind(&ArcAuthService::OnAuthCodeFetched,
-                                         weak_ptr_factory_.GetWeakPtr()));
-    return;
-  }
-
-  // Otherwise, show LSO page to user after HTTP context preparation, and let
-  // them click "Sign in" button.
-  ArcSessionManager::Get()->auth_context()->Prepare(base::Bind(
-      &ArcAuthService::OnContextPrepared, weak_ptr_factory_.GetWeakPtr()));
-}
-
-void ArcAuthService::OnContextPrepared(
-    net::URLRequestContextGetter* request_context_getter) {
-  ArcSupportHost* support_host = ArcSessionManager::Get()->support_host();
-  // Here, support_host should be available always. The case support_host is
-  // not created is when 1) IsOptInVerificationDisabled() is true or 2)
-  // IsArcKioskMode() is true. Both cases are handled above.
-  DCHECK(support_host);
-  if (!support_host->HasObserver(this))
-    support_host->AddObserver(this);
-
-  if (request_context_getter) {
-    support_host->ShowLso();
   } else {
-    support_host->ShowError(ArcSupportHost::Error::SIGN_IN_NETWORK_ERROR,
-                            false);
-  }
-}
-
-void ArcAuthService::OnAccountInfoReady(mojom::AccountInfoPtr account_info) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  auto* instance = arc_bridge_service()->auth()->GetInstanceForMethod(
-      "OnAccountInfoReady", kMinVersionForOnAccountInfoReady);
-  DCHECK(instance);
-  instance->OnAccountInfoReady(std::move(account_info));
-}
-
-void ArcAuthService::OnRobotAuthCodeFetched(
-    const std::string& robot_auth_code) {
-  // We fetching robot auth code for ARC kiosk only.
-  DCHECK(ArcSessionManager::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;
+    // Otherwise, show LSO page and let user click "Sign in" button.
+    // Here, support_host should be available always. The case support_host is
+    // not created is when 1) IsOptInVerificationDisabled() is true or 2)
+    // IsArcKioskMode() is true. Both cases are handled above.
+    fetcher_ = base::MakeUnique<ArcManualAuthCodeFetcher>(
+        ArcSessionManager::Get()->auth_context(),
+        ArcSessionManager::Get()->support_host());
   }
-
-  OnAuthCodeObtained(robot_auth_code);
+  fetcher_->Fetch(base::Bind(&ArcAuthService::OnAuthCodeFetched,
+                             weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ArcAuthService::OnAuthCodeFetched(const std::string& auth_code) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  auth_code_fetcher_.reset();
+  fetcher_.reset();
 
   if (auth_code.empty()) {
     ArcSessionManager::Get()->OnProvisioningFinished(
@@ -297,13 +255,6 @@ void ArcAuthService::OnAuthCodeFetched(const std::string& auth_code) {
     return;
   }
 
-  OnAuthCodeObtained(auth_code);
-}
-
-void ArcAuthService::OnAuthCodeObtained(const std::string& auth_code) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(!auth_code.empty());
-
   notifier_->Notify(
       !ArcSessionManager::IsOptInVerificationDisabled(), auth_code,
       GetAccountType(),
@@ -311,22 +262,4 @@ void ArcAuthService::OnAuthCodeObtained(const std::string& auth_code) {
   notifier_.reset();
 }
 
-void ArcAuthService::OnAuthSucceeded(const std::string& auth_code) {
-  OnAuthCodeObtained(auth_code);
-}
-
-void ArcAuthService::OnRetryClicked() {
-  ArcSupportHost* support_host = ArcSessionManager::Get()->support_host();
-  // This is the callback for the UI event, so support_host should be always
-  // available here.
-  DCHECK(support_host);
-  if (support_host->ui_page() == ArcSupportHost::UIPage::ERROR) {
-    // This case is handled by ArcSessionManager::OnRetryClicked().
-    return;
-  }
-
-  ArcSessionManager::Get()->auth_context()->Prepare(base::Bind(
-      &ArcAuthService::OnContextPrepared, weak_ptr_factory_.GetWeakPtr()));
-}
-
 }  // namespace arc