Prevent invalidating refresh token on transient errors.

Problem:
When access token request fails with unexpected errors those get
translated to "invalid credentials". In response auth_sync_observer.cc
marks refresh token as invalid which prevents offline auth on next startup.

oauth2_access_token_fetcher_impl.cc:
 - Handle all 5xx http codes as transient errors.
 - Log error for unexpected codes.

profile_sync_service.cc:
 - handle REQUEST_CANCELLED and SERVICE_ERROR as transient error, retry instead of
   notifying observers.
 - Log error for unexpected auth errors.

BUG=394939
R=rogerta@chromium.org,zea@chromium.org

Review URL: https://codereview.chromium.org/472403002

Cr-Commit-Position: refs/heads/master@{#290158}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290158 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/sync/profile_sync_service.cc

@@ -759,6 +759,8 @@ void ProfileSyncService::OnGetTokenFailure(
   last_get_token_error_ = error;
   switch (error.state()) {
     case GoogleServiceAuthError::CONNECTION_FAILED:
+    case GoogleServiceAuthError::REQUEST_CANCELED:
+    case GoogleServiceAuthError::SERVICE_ERROR:
     case GoogleServiceAuthError::SERVICE_UNAVAILABLE: {
       // Transient error. Retry after some time.
       request_access_token_backoff_.InformOfRequest(false);
@@ -772,7 +774,6 @@ void ProfileSyncService::OnGetTokenFailure(
       NotifyObservers();
       break;
     }
-    case GoogleServiceAuthError::SERVICE_ERROR:
     case GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS: {
       if (!sync_prefs_.SyncHasAuthError()) {
         sync_prefs_.SetSyncAuthError(true);
@@ -783,6 +784,9 @@ void ProfileSyncService::OnGetTokenFailure(
       // Fallthrough.
     }
     default: {
+      if (error.state() != GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS) {
+        LOG(ERROR) << "Unexpected persistent error: " << error.ToString();
+      }
       // Show error to user.
       UpdateAuthErrorState(error);
     }

chrome/browser/sync/test/integration/sync_auth_test.cc

@@ -206,10 +206,10 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, InvalidGrant) {
             GetSyncService((0))->GetAuthError().state());
 }
 
-// Verify that ProfileSyncService ends up with an SERVICE_ERROR auth error when
+// Verify that ProfileSyncService retries after SERVICE_ERROR auth error when
 // an invalid_client error is returned by OAuth2TokenService with an
 // HTTP_BAD_REQUEST (400) response code.
-IN_PROC_BROWSER_TEST_F(SyncAuthTest, InvalidClient) {
+IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryInvalidClient) {
   ASSERT_TRUE(SetupSync());
   ASSERT_FALSE(AttemptToTriggerAuthError());
   GetFakeServer()->SetUnauthenticated();
@@ -218,13 +218,12 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, InvalidClient) {
                          net::HTTP_BAD_REQUEST,
                          net::URLRequestStatus::SUCCESS);
   ASSERT_TRUE(AttemptToTriggerAuthError());
-  ASSERT_EQ(GoogleServiceAuthError::SERVICE_ERROR,
-            GetSyncService((0))->GetAuthError().state());
+  ASSERT_TRUE(GetSyncService((0))->IsRetryingAccessTokenFetchForTest());
 }
 
-// Verify that ProfileSyncService ends up with a REQUEST_CANCELED auth error
-// when when OAuth2TokenService has encountered a URLRequestStatus of CANCELED.
-IN_PROC_BROWSER_TEST_F(SyncAuthTest, RequestCanceled) {
+// Verify that ProfileSyncService retries after REQUEST_CANCELED auth error
+// when OAuth2TokenService has encountered a URLRequestStatus of CANCELED.
+IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryRequestCanceled) {
   ASSERT_TRUE(SetupSync());
   ASSERT_FALSE(AttemptToTriggerAuthError());
   GetFakeServer()->SetUnauthenticated();
@@ -233,8 +232,7 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, RequestCanceled) {
                          net::HTTP_INTERNAL_SERVER_ERROR,
                          net::URLRequestStatus::CANCELED);
   ASSERT_TRUE(AttemptToTriggerAuthError());
-  ASSERT_EQ(GoogleServiceAuthError::REQUEST_CANCELED,
-            GetSyncService((0))->GetAuthError().state());
+  ASSERT_TRUE(GetSyncService((0))->IsRetryingAccessTokenFetchForTest());
 }
 
 // Verify that ProfileSyncService fails initial sync setup during backend

google_apis/gaia/oauth2_access_token_fetcher_impl.cc

@@ -183,9 +183,8 @@ void OAuth2AccessTokenFetcherImpl::EndGetAccessToken(
     case net::HTTP_OK:
       break;
     case net::HTTP_FORBIDDEN:
-    case net::HTTP_INTERNAL_SERVER_ERROR:
       // HTTP_FORBIDDEN (403) is treated as temporary error, because it may be
-      // '403 Rate Limit Exeeded.' 500 is always treated as transient.
+      // '403 Rate Limit Exeeded.'
       OnGetTokenFailure(
           GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_UNAVAILABLE));
       return;
@@ -212,11 +211,20 @@ void OAuth2AccessTokenFetcherImpl::EndGetAccessToken(
               : GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_ERROR));
       return;
     }
-    default:
-      // The other errors are treated as permanent error.
-      OnGetTokenFailure(GoogleServiceAuthError(
-          GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
+    default: {
+      if (source->GetResponseCode() >= net::HTTP_INTERNAL_SERVER_ERROR) {
+        // 5xx is always treated as transient.
+        OnGetTokenFailure(GoogleServiceAuthError(
+            GoogleServiceAuthError::SERVICE_UNAVAILABLE));
+      } else {
+        // The other errors are treated as permanent error.
+        DLOG(ERROR) << "Unexpected persistent error: http_status="
+                    << source->GetResponseCode();
+        OnGetTokenFailure(GoogleServiceAuthError(
+            GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS));
+      }
       return;
+    }
   }
 
   // The request was successfully fetched and it returned OK.