arc: Cleanup merge session for auth context.

This removes merge session stage for auth context preparation once it
is deprecated and source of extra failures.

TEST=Locally
BUG=b:148670963

Change-Id: I61fafb236a83cf2b9e9f1f65cd8ae029c734fe3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2048109Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Commit-Queue: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740305}

chrome/browser/chromeos/arc/auth/arc_auth_context.cc

@@ -11,58 +11,25 @@
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
 #include "components/signin/public/identity_manager/access_token_fetcher.h"
-#include "components/signin/public/identity_manager/ubertoken_fetcher.h"
 #include "content/public/common/url_constants.h"
-#include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace arc {
 
 namespace {
 
-constexpr int kMaxRetryAttempts = 3;
-
 constexpr base::TimeDelta kRefreshTokenTimeout =
     base::TimeDelta::FromSeconds(10);
 
-constexpr net::BackoffEntry::Policy kRetryBackoffPolicy = {
-    // Number of initial errors (in sequence) to ignore before applying
-    // exponential back-off rules.
-    0,
-
-    // Initial delay for exponential back-off in ms.
-    5000,
-
-    // Factor by which the waiting time will be multiplied.
-    2.0,
-
-    // Fuzzing percentage. ex: 10% will spread requests randomly
-    // between 90%-100% of the calculated time.
-    0.0,  // 0%
-
-    // Maximum amount of time we are willing to delay our request in ms.
-    1000 * 15,  // 15 seconds.
-
-    // Time to keep an entry from being discarded even when it
-    // has no significant state, -1 to never discard.
-    -1,
-
-    // Don't use initial delay unless the last request was an error.
-    false,
-};
-
 }  // namespace
 
 ArcAuthContext::ArcAuthContext(Profile* profile,
                                const CoreAccountId& account_id)
-    : profile_(profile),
-      account_id_(account_id),
-      identity_manager_(IdentityManagerFactory::GetForProfile(profile)),
-      retry_backoff_(&kRetryBackoffPolicy) {
+    : account_id_(account_id),
+      identity_manager_(IdentityManagerFactory::GetForProfile(profile)) {
   DCHECK(identity_manager_->HasAccountWithRefreshToken(account_id));
 }
 
@@ -79,8 +46,6 @@ void ArcAuthContext::Prepare(const PrepareCallback& callback) {
   callback_ = callback;
   identity_manager_->RemoveObserver(this);
   refresh_token_timeout_.Stop();
-  ResetFetchers();
-  retry_backoff_.Reset();
 
   if (!identity_manager_->HasAccountWithRefreshToken(account_id_)) {
     identity_manager_->AddObserver(this);
@@ -90,7 +55,8 @@ void ArcAuthContext::Prepare(const PrepareCallback& callback) {
     return;
   }
 
-  StartFetchers();
+  // Refresh token is already available.
+  std::move(callback_).Run(true);
 }
 
 std::unique_ptr<signin::AccessTokenFetcher>
@@ -118,7 +84,7 @@ void ArcAuthContext::OnRefreshTokensLoaded() {
   identity_manager_->RemoveObserver(this);
   VLOG(1) << "Refresh token for account " << account_id_ << " loaded.";
   refresh_token_timeout_.Stop();
-  StartFetchers();
+  std::move(callback_).Run(true);
 }
 
 void ArcAuthContext::OnRefreshTokenTimeout() {
@@ -127,73 +93,4 @@ void ArcAuthContext::OnRefreshTokenTimeout() {
   std::move(callback_).Run(false);
 }
 
-void ArcAuthContext::StartFetchers() {
-  DCHECK(!refresh_token_timeout_.IsRunning());
-  ResetFetchers();
-
-  if (skip_merge_session_for_testing_) {
-    OnMergeSessionSuccess("");
-    return;
-  }
-
-  auto* identity_manager = IdentityManagerFactory::GetForProfile(profile_);
-  ubertoken_fetcher_ = identity_manager->CreateUbertokenFetcherForAccount(
-      account_id_,
-      base::BindOnce(&ArcAuthContext::OnUbertokenFetchComplete,
-                     base::Unretained(this)),
-      gaia::GaiaSource::kChromeOS, profile_->GetURLLoaderFactory());
-}
-
-void ArcAuthContext::ResetFetchers() {
-  merger_fetcher_.reset();
-  ubertoken_fetcher_.reset();
-  retry_timeout_.Stop();
-}
-
-void ArcAuthContext::OnFetcherError(const GoogleServiceAuthError& error) {
-  ResetFetchers();
-  DCHECK(error.state() != GoogleServiceAuthError::NONE);
-  if (error.IsTransientError()) {
-    retry_backoff_.InformOfRequest(false);
-    if (retry_backoff_.failure_count() <= kMaxRetryAttempts) {
-      LOG(WARNING) << "Found transient error. Retry attempt "
-                   << retry_backoff_.failure_count() << ".";
-      refresh_token_timeout_.Start(FROM_HERE,
-                                   retry_backoff_.GetTimeUntilRelease(), this,
-                                   &ArcAuthContext::StartFetchers);
-      return;
-    }
-    LOG(WARNING) << "Too many transient errors. Stop retrying.";
-  }
-  std::move(callback_).Run(false);
-}
-
-void ArcAuthContext::OnUbertokenFetchComplete(GoogleServiceAuthError error,
-                                              const std::string& token) {
-  if (error != GoogleServiceAuthError::AuthErrorNone()) {
-    LOG(WARNING) << "Failed to get ubertoken " << error.ToString() << ".";
-    OnFetcherError(error);
-    return;
-  }
-
-  ResetFetchers();
-  merger_fetcher_.reset(new GaiaAuthFetcher(this, gaia::GaiaSource::kChromeOS,
-                                            profile_->GetURLLoaderFactory()));
-  merger_fetcher_->StartMergeSession(token, std::string());
-}
-
-void ArcAuthContext::OnMergeSessionSuccess(const std::string& data) {
-  VLOG_IF(1, retry_backoff_.failure_count())
-      << "Auth context was successfully prepared after retry.";
-  context_prepared_ = true;
-  ResetFetchers();
-  std::move(callback_).Run(true);
-}
-
-void ArcAuthContext::OnMergeSessionFailure(
-    const GoogleServiceAuthError& error) {
-  LOG(WARNING) << "Failed to merge gaia session " << error.ToString() << ".";
-  OnFetcherError(error);
-}
-
 }  // namespace arc

chrome/browser/chromeos/arc/auth/arc_auth_context.h

@@ -12,20 +12,12 @@
 #include "base/macros.h"
 #include "base/timer/timer.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
-#include "google_apis/gaia/gaia_auth_consumer.h"
-#include "net/base/backoff_entry.h"
 
-class GaiaAuthFetcher;
 class Profile;
 
-namespace signin {
-class UbertokenFetcher;
-}
-
 namespace arc {
 
-class ArcAuthContext : public GaiaAuthConsumer,
-                       public signin::IdentityManager::Observer {
+class ArcAuthContext : public signin::IdentityManager::Observer {
  public:
   // Creates an |ArcAuthContext| for the given |account_id|. This |account_id|
   // must be the |account_id| used by the OAuth Token Service chain.
@@ -54,43 +46,16 @@ class ArcAuthContext : public GaiaAuthConsumer,
       const CoreAccountInfo& account_info) override;
   void OnRefreshTokensLoaded() override;
 
-  // Ubertoken fetch completion callback.
-  void OnUbertokenFetchComplete(GoogleServiceAuthError error,
-                                const std::string& uber_token);
-
-  // GaiaAuthConsumer:
-  void OnMergeSessionSuccess(const std::string& data) override;
-  void OnMergeSessionFailure(const GoogleServiceAuthError& error) override;
-
-  // Skips the merge session, instead calling the callback passed to |Prepare()|
-  // once the refresh token is available. Use only in testing.
-  void SkipMergeSessionForTesting() { skip_merge_session_for_testing_ = true; }
-
  private:
   void OnRefreshTokenTimeout();
 
-  void StartFetchers();
-  void ResetFetchers();
-  void OnFetcherError(const GoogleServiceAuthError& error);
-
-  // Unowned pointer.
-  Profile* const profile_;
   const CoreAccountId account_id_;
   signin::IdentityManager* const identity_manager_;
 
-  // Whether the merge session should be skipped. Set to true only in testing.
-  bool skip_merge_session_for_testing_ = false;
-
   PrepareCallback callback_;
   bool context_prepared_ = false;
 
-  // Defines retry logic in case of transient error.
-  net::BackoffEntry retry_backoff_;
-
   base::OneShotTimer refresh_token_timeout_;
-  base::OneShotTimer retry_timeout_;
-  std::unique_ptr<GaiaAuthFetcher> merger_fetcher_;
-  std::unique_ptr<signin::UbertokenFetcher> ubertoken_fetcher_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcAuthContext);
 };

chrome/browser/chromeos/arc/auth/arc_auth_service.cc

@@ -818,16 +818,10 @@ ArcAuthService::CreateArcBackgroundAuthCodeFetcher(
   auto fetcher = std::make_unique<ArcBackgroundAuthCodeFetcher>(
       url_loader_factory_, profile_, account_id, initial_signin,
       IsPrimaryGaiaAccount(account_info.value().gaia));
-  if (skip_merge_session_for_testing_)
-    fetcher->SkipMergeSessionForTesting();
 
   return fetcher;
 }
 
-void ArcAuthService::SkipMergeSessionForTesting() {
-  skip_merge_session_for_testing_ = true;
-}
-
 void ArcAuthService::TriggerAccountsPushToArc(bool filter_primary_account) {
   if (!chromeos::IsAccountManagerAvailable(profile_))
     return;

chrome/browser/chromeos/arc/auth/arc_auth_service.h

@@ -110,8 +110,6 @@ class ArcAuthService : public KeyedService,
   // KeyedService:
   void Shutdown() override;
 
-  void SkipMergeSessionForTesting();
-
  private:
   // Callback when Active Directory Enrollment Token is fetched.
   // |callback| is completed with |ArcSignInStatus| and |AccountInfo| depending
@@ -210,8 +208,6 @@ class ArcAuthService : public KeyedService,
   // ready.
   GetGoogleAccountsInArcCallback pending_get_arc_accounts_callback_;
 
-  bool skip_merge_session_for_testing_ = false;
-
   base::WeakPtrFactory<ArcAuthService> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ArcAuthService);

chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc

@@ -339,11 +339,6 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
         base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
             &test_url_loader_factory_);
     auth_service_->SetURLLoaderFactoryForTesting(test_shared_loader_factory_);
-    // It is non-trivial to navigate through the merge session in a testing
-    // context; currently we just skip it.
-    // TODO(blundell): Figure out how to enable this flow.
-    auth_service_->SkipMergeSessionForTesting();
-
     arc_bridge_service_ = ArcServiceManager::Get()->arc_bridge_service();
     DCHECK(arc_bridge_service_);
     arc_bridge_service_->auth()->SetInstance(&auth_instance_);

chrome/browser/chromeos/arc/auth/arc_background_auth_code_fetcher.cc

@@ -237,8 +237,4 @@ void ArcBackgroundAuthCodeFetcher::ReportResult(
   std::move(callback_).Run(!auth_code.empty(), auth_code);
 }
 
-void ArcBackgroundAuthCodeFetcher::SkipMergeSessionForTesting() {
-  context_.SkipMergeSessionForTesting();
-}
-
 }  // namespace arc

chrome/browser/chromeos/arc/auth/arc_background_auth_code_fetcher.h

@@ -49,8 +49,6 @@ class ArcBackgroundAuthCodeFetcher : public ArcAuthCodeFetcher {
   // ArcAuthCodeFetcher:
   void Fetch(FetchCallback callback) override;
 
-  void SkipMergeSessionForTesting();
-
  private:
   void ResetFetchers();
   void OnPrepared(bool success);