Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(590)

Unified Diff: chrome/browser/chromeos/arc/arc_auth_service.cc

Issue 2547073002: Fix race issue in ArcAuthService. (Closed)
Patch Set: Address comments Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/chromeos/arc/arc_auth_service.h ('k') | chrome/browser/chromeos/arc/arc_session_manager.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..218497c0c813f7c9b8ee0e5a52c434d37b9d9ff0 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 retrieve auth code in silent mode.
+ 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
« no previous file with comments | « chrome/browser/chromeos/arc/arc_auth_service.h ('k') | chrome/browser/chromeos/arc/arc_session_manager.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698