arc: Implement retry logic for transient errors

chrome/browser/chromeos/arc/arc_auth_context.cc

Index: chrome/browser/chromeos/arc/arc_auth_context.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_context.cc b/chrome/browser/chromeos/arc/arc_auth_context.cc
index d3aed73a06e879c85622a92543ec7f997b4630a4..feb72fb58e11ef9ea44baf970ecc2a5f858a32b4 100644
--- a/chrome/browser/chromeos/arc/arc_auth_context.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_context.cc
@@ -25,9 +25,13 @@ namespace arc {
 
 namespace {
 
+constexpr int kMaxRetryAttempts = 3;
+
 constexpr base::TimeDelta kRefreshTokenTimeout =
     base::TimeDelta::FromSeconds(10);
 
+constexpr base::TimeDelta kRetryTimeout = base::TimeDelta::FromSeconds(10);
+
 }  // namespace
 
 ArcAuthContext::ArcAuthContext(Profile* profile) {
@@ -63,6 +67,7 @@ void ArcAuthContext::Prepare(const PrepareCallback& callback) {
   callback_ = callback;
   token_service_->RemoveObserver(this);
   refresh_token_timeout_.Stop();
+  ResetFetchers();
 
   if (!token_service_->RefreshTokenIsAvailable(account_id_)) {
     token_service_->AddObserver(this);
@@ -71,6 +76,7 @@ void ArcAuthContext::Prepare(const PrepareCallback& callback) {
     return;
   }
 
+  retry_attempt_count_ = 0;

[xiyuan, 2017/05/17 22:09:16]

Do we add this before the StartFetchers() call in 
ArcAuthContext::OnRefreshTokensLoaded as well? Or move it up after
ResetFetchers()?

[khmel, 2017/05/17 22:43:55]

Good point. Moved here up.

   StartFetchers();
 }
 
@@ -101,6 +107,29 @@ void ArcAuthContext::StartFetchers() {
   ubertoken_fetcher_->StartFetchingToken(account_id_);
 }
 
+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()) {
+    if (retry_attempt_count_ < kMaxRetryAttempts) {
+      ++retry_attempt_count_;

[xiyuan, 2017/05/17 22:09:16]

Not needed for this CL but do you want to use net::BackoffEntry::Policy ?

One example:
https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_ser...

[khmel, 2017/05/17 22:43:55]

Nice example. Thanks

+      LOG(WARNING) << "Found transient error. Retry attempt "
+                   << retry_attempt_count_;
+      refresh_token_timeout_.Start(FROM_HERE, kRetryTimeout, this,
+                                   &ArcAuthContext::StartFetchers);
+      return;
+    }
+    LOG(WARNING) << "Too many transient errors. Stop retrying.";
+  }
+  base::ResetAndReturn(&callback_).Run(nullptr);
+}
+
 void ArcAuthContext::OnUbertokenSuccess(const std::string& token) {
   ResetFetchers();
   merger_fetcher_.reset(
@@ -111,11 +140,12 @@ void ArcAuthContext::OnUbertokenSuccess(const std::string& token) {
 
 void ArcAuthContext::OnUbertokenFailure(const GoogleServiceAuthError& error) {
   LOG(WARNING) << "Failed to get ubertoken " << error.ToString() << ".";
-  ResetFetchers();
-  base::ResetAndReturn(&callback_).Run(nullptr);
+  OnFetcherError(error);
 }
 
 void ArcAuthContext::OnMergeSessionSuccess(const std::string& data) {
+  if (retry_attempt_count_)
+    VLOG(1) << "Auth context was successfully prepared after retry.";

[xiyuan, 2017/05/17 22:09:16]

nit:  VLOG_IF(1, retry_attempt_count_) << ...

[khmel, 2017/05/17 22:43:55]

Done.

   context_prepared_ = true;
   ResetFetchers();
   base::ResetAndReturn(&callback_)
@@ -125,13 +155,7 @@ void ArcAuthContext::OnMergeSessionSuccess(const std::string& data) {
 void ArcAuthContext::OnMergeSessionFailure(
     const GoogleServiceAuthError& error) {
   LOG(WARNING) << "Failed to merge gaia session " << error.ToString() << ".";
-  ResetFetchers();
-  base::ResetAndReturn(&callback_).Run(nullptr);
-}
-
-void ArcAuthContext::ResetFetchers() {
-  merger_fetcher_.reset();
-  ubertoken_fetcher_.reset();
+  OnFetcherError(error);
 }
 
 }  // namespace arc