Revert of Immediately handle user-recoverable auth errors for child a…

…ccounts. (patchset #5 id:80001 of https://codereview.chromium.org/2724023002/ ) Reason for revert: Causes an UnsatisfiedLinkError: https://crbug.com/697979 Original issue's description: > Immediately handle user-recoverable auth errors for child accounts. > > This will open a reauth activity e.g. when the password has changed. For child > accounts, that is preferable to letting the user use Chrome unauthenticated. > > BUG=697412 > > Review-Url: https://codereview.chromium.org/2724023002 > Cr-Commit-Position: refs/heads/master@{#453981} > Committed: https://chromium.googlesource.com/chromium/src/+/94af3b0124e78f1f5f00a515e1f569a194c6b1dc TBR=gogerald@google.com,msarda@chromium.org,bsazonov@chromium.org,rogerta@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=697412 Review-Url: https://codereview.chromium.org/2729203002 Cr-Commit-Position: refs/heads/master@{#454642}
nwjs · Mar 3, 2017 · 2e588c4 · 2e588c4
1 parent 462ef8c
commit 2e588c4
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 70 deletions.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java b/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java
@@ -14,7 +14,6 @@
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
-import org.chromium.chrome.browser.childaccounts.ChildAccountService;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.components.signin.AccountManagerHelper;
 import org.chromium.components.signin.ChromeSigninController;
@@ -146,11 +145,9 @@ public void run() {
         }
         String oauth2Scope = OAUTH2_SCOPE_PREFIX + scope;
 
-        boolean handleUserRecoverableErrors = ChildAccountService.isChildAccount();
-
         AccountManagerHelper accountManagerHelper = AccountManagerHelper.get(context);
-        accountManagerHelper.getAuthToken(account, oauth2Scope, handleUserRecoverableErrors,
-                new AccountManagerHelper.GetAuthTokenCallback() {
+        accountManagerHelper.getAuthToken(
+                account, oauth2Scope, new AccountManagerHelper.GetAuthTokenCallback() {
                     @Override
                     public void tokenAvailable(String token) {
                         nativeOAuth2TokenFetched(token, false, nativeCallback);
@@ -173,9 +170,7 @@ public void tokenUnavailable(boolean isTransientError) {
     public static void getOAuth2AccessToken(Context context, Account account, String scope,
             AccountManagerHelper.GetAuthTokenCallback callback) {
         String oauth2Scope = OAUTH2_SCOPE_PREFIX + scope;
-        boolean handleUserRecoverableErrors = ChildAccountService.isChildAccount();
-        AccountManagerHelper.get(context).getAuthToken(
-                account, oauth2Scope, handleUserRecoverableErrors, callback);
+        AccountManagerHelper.get(context).getAuthToken(account, oauth2Scope, callback);
     }
 
     /**

diff --git a/.../core/browser/android/java/src/org/chromium/components/signin/AccountManagerDelegate.java b/.../core/browser/android/java/src/org/chromium/components/signin/AccountManagerDelegate.java
@@ -29,32 +29,15 @@ public interface AccountManagerDelegate {
     /**
      * Get an auth token. This should only be called on a background thread.
      *
-     * @param account        The {@link Account} for which the auth token is requested.
+     * @param account The {@link Account} for which the auth token is requested.
      * @param authTokenScope The scope of the authToken being requested.
-     *
      * @return The auth token fetched from the authenticator.
      * @throws AuthException Indicates a failure in fetching the auth token perhaps due to a
      * transient error or when user intervention is required (like confirming the credentials)
      * which is expressed as an {@link Intent} to the handler.
      */
     String getAuthToken(Account account, String authTokenScope) throws AuthException;
 
-    /**
-     * Get an auth token. If there is a user-recoverable auth error, immediately attempt to resolve
-     * it by launching the intent associated with it.
-     * This should only be called on a background thread.
-     *
-     * @param account        The {@link Account} for which the auth token is requested.
-     * @param authTokenScope The scope of the authToken being requested.
-     *
-     * @return The auth token fetched from the authenticator.
-     * @throws AuthException Indicates a failure in fetching the auth token perhaps due to a
-     *         transient error or when user intervention is required (like confirming the
-     *         credentials) which is expressed as an {@link Intent} to the handler.
-     */
-    String getAuthTokenAndFixUserRecoverableErrorIfNeeded(Account account, String authTokenScope)
-            throws AuthException;
-
     /**
      * @param authToken The auth token to invalidate.
      * @throws AuthException Indicates a failure clearing the auth token; can be transient.

diff --git a/...in/core/browser/android/java/src/org/chromium/components/signin/AccountManagerHelper.java b/...in/core/browser/android/java/src/org/chromium/components/signin/AccountManagerHelper.java
@@ -14,7 +14,6 @@
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.net.NetworkChangeNotifier;
-import org.chromium.net.NetworkChangeNotifier.ConnectionTypeObserver;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -292,23 +291,15 @@ public boolean hasGoogleAccountAuthenticator() {
      * Gets the auth token and returns the response asynchronously.
      * This should be called when we have a foreground activity that needs an auth token.
      * If encountered an IO error, it will attempt to retry when the network is back.
-     * If {@code handleUserRecoverableErrors} is true, attempt to immediately resolve
-     * user-recoverable errors. This should only be used if getting the auth token is absolutely
-     * necessary for the browser to function.
      *
      * - Assumes that the account is a valid account.
      */
     public void getAuthToken(final Account account, final String authTokenType,
-            final boolean handleUserRecoverableErrors, final GetAuthTokenCallback callback) {
+            final GetAuthTokenCallback callback) {
         ConnectionRetry.runAuthTask(new AuthTask<String>() {
             @Override
             public String run() throws AuthException {
-                if (handleUserRecoverableErrors) {
-                    return mAccountManager.getAuthTokenAndFixUserRecoverableErrorIfNeeded(
-                            account, authTokenType);
-                } else {
-                    return mAccountManager.getAuthToken(account, authTokenType);
-                }
+                return mAccountManager.getAuthToken(account, authTokenType);
             }
             @Override
             public void onSuccess(String token) {
@@ -329,7 +320,7 @@ public void onFailure(boolean isTransientError) {
     public void getNewAuthToken(Account account, String authToken, String authTokenType,
             GetAuthTokenCallback callback) {
         invalidateAuthToken(authToken);
-        getAuthToken(account, authTokenType, false /* handleUserRecoverableErrors */, callback);
+        getAuthToken(account, authTokenType, callback);
     }
 
     /**

diff --git a/...browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java b/...browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java
@@ -14,7 +14,6 @@
 import android.accounts.OperationCanceledException;
 import android.app.Activity;
 import android.content.Context;
-import android.content.Intent;
 import android.content.pm.PackageManager;
 import android.os.AsyncTask;
 import android.os.Bundle;
@@ -24,7 +23,6 @@
 import com.google.android.gms.auth.GoogleAuthException;
 import com.google.android.gms.auth.GoogleAuthUtil;
 import com.google.android.gms.auth.GooglePlayServicesAvailabilityException;
-import com.google.android.gms.auth.UserRecoverableAuthException;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.Callback;
@@ -38,8 +36,8 @@
 import java.util.concurrent.TimeUnit;
 
 /**
- * Default implementation of {@link AccountManagerDelegate} which delegates all calls to the Android
- * account manager.
+ * Default implementation of {@link AccountManagerDelegate} which delegates all calls to the
+ * Android account manager.
  */
 @MainDex
 public class SystemAccountManagerDelegate implements AccountManagerDelegate {
@@ -87,26 +85,8 @@ public String getAuthToken(Account account, String authTokenScope) throws AuthEx
             return GoogleAuthUtil.getTokenWithNotification(
                     mApplicationContext, account, authTokenScope, null);
         } catch (GoogleAuthException ex) {
-            // TODO(bauerb): Investigate integrating the callback with ConnectionRetry.
-            throw new AuthException(false /* isTransientError */, ex);
-        } catch (IOException ex) {
-            throw new AuthException(true /* isTransientError */, ex);
-        }
-    }
-
-    @Override
-    public String getAuthTokenAndFixUserRecoverableErrorIfNeeded(
-            Account account, String authTokenScope) throws AuthException {
-        assert !ThreadUtils.runningOnUiThread();
-        assert AccountManagerHelper.GOOGLE_ACCOUNT_TYPE.equals(account.type);
-        try {
-            return GoogleAuthUtil.getToken(mApplicationContext, account, authTokenScope, null);
-        } catch (UserRecoverableAuthException ex) {
-            Intent intent = ex.getIntent();
-            intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-            mApplicationContext.startActivity(intent);
-            throw new AuthException(false /* isTransientError */, ex);
-        } catch (GoogleAuthException ex) {
+            // This case includes a UserRecoverableNotifiedException, but most clients will have
+            // their own retry mechanism anyway.
             // TODO(bauerb): Investigate integrating the callback with ConnectionRetry.
             throw new AuthException(false /* isTransientError */, ex);
         } catch (IOException ex) {
@@ -162,8 +142,8 @@ public void run(AccountManagerFuture<Boolean> future) {
 
     /**
      * Records a histogram value for how long time an action has taken using
-     * {@link RecordHistogram#recordTimesHistogram(String, long, TimeUnit))} iff the browser process
-     * has been initialized.
+     * {@link RecordHistogram#recordTimesHistogram(String, long, TimeUnit))} iff the browser
+     * process has been initialized.
      *
      * @param histogramName the name of the histogram.
      * @param elapsedMs the elapsed time in milliseconds.

diff --git a/...er/android/javatests/src/org/chromium/components/signin/test/util/MockAccountManager.java b/...er/android/javatests/src/org/chromium/components/signin/test/util/MockAccountManager.java
@@ -158,12 +158,6 @@ public String getAuthToken(Account account, String authTokenScope) {
         return ah.getAuthToken(authTokenScope);
     }
 
-    @Override
-    public String getAuthTokenAndFixUserRecoverableErrorIfNeeded(
-            Account account, String authTokenScope) {
-        return getAuthToken(account, authTokenScope);
-    }
-
     @Override
     public void invalidateAuthToken(String authToken) {
         if (authToken == null) {