Simplyfing the authentication dance by handling both success and failure together, and both of them outside onActivityResult.

MyTracks/src/com/google/android/apps/mytracks/MyMapsList.java

@@ -19,6 +19,7 @@ import static com.google.android.apps.mytracks.Constants.TAG;
 
 import com.google.android.accounts.Account;
 import com.google.android.apps.mytracks.io.AuthManager;
+import com.google.android.apps.mytracks.io.AuthManager.AuthCallback;
 import com.google.android.apps.mytracks.io.AuthManagerFactory;
 import com.google.android.apps.mytracks.io.mymaps.MapsFacade;
 import com.google.android.apps.mytracks.io.mymaps.MyMapsConstants;
@@ -104,8 +105,15 @@ public class MyMapsList extends Activity implements MapsFacade.MapsListCallback
 
   private void doLogin(final Account account) {
     // Starts in the UI thread.
-    auth.doLogin(new Runnable() {
+    auth.doLogin(new AuthCallback() {
-      public void run() {
+      @Override
+      public void onAuthResult(boolean success) {
+        if (!success) {
+          setResult(RESULT_CANCELED);
+          finish();
+          return;
+        }
+
         // Runs in UI thread.
         mapsClient = new MapsFacade(MyMapsList.this, auth);
 

MyTracks/src/com/google/android/apps/mytracks/io/AuthManager.java

@@ -25,15 +25,27 @@ import android.content.Intent;
  */
 public interface AuthManager {
   /**
-   * Initializes the login process. The user should be asked to login if they
+   * Callback for authentication token retrieval operations.
-   * haven't already. The {@link Runnable} provided will be executed when the
-   * auth token is successfully fetched.
-   *
-   * @param whenFinished A {@link Runnable} to execute when the auth token
-   *        has been successfully fetched and is available via
-   *        {@link #getAuthToken()}
    */
-  void doLogin(Runnable whenFinished, Object o);
+  public interface AuthCallback {
+    /**
+     * Indicates that we're done fetching an auth token.
+     *
+     * @param success if true, indicates we have the requested auth token available
+     *        to be retrieved using {@link AuthManager#getAuthToken}
+     */
+    void onAuthResult(boolean success);
+  }
+
+  /**
+   * Initializes the login process. The user should be asked to login if they
+   * haven't already. The {@link AuthCallback} provided will be executed when the
+   * auth token fetching is done (successfully or not).
+   *
+   * @param whenFinished A {@link AuthCallback} to execute when the auth token
+   *        fetching is done
+   */
+  void doLogin(AuthCallback whenFinished, Object o);
 
   /**
    * The {@link android.app.Activity} owner of this class should call this
@@ -48,11 +60,8 @@ public interface AuthManager {
    *        {@link android.app.Activity#onActivityResult} function
    * @param results The data passed in to the {@link android.app.Activity}'s
    *        {@link android.app.Activity#onActivityResult} function
-   * @return True if the auth token was fetched or we aren't done fetching
-   *         the auth token, or False if there was an error or the request was
-   *         canceled
    */
-  boolean authResult(int resultCode, Intent results);
+  void authResult(int resultCode, Intent results);
 
   /**
    * Returns the current auth token. Response may be null if no valid auth
@@ -71,5 +80,5 @@ public interface AuthManager {
    * @param whenFinished A {@link Runnable} to execute when a new auth token
    *        is successfully fetched
    */
-  void invalidateAndRefresh(Runnable whenFinished);
+  void invalidateAndRefresh(AuthCallback whenFinished);
 }

MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerFactory.java

@@ -1,12 +1,12 @@
 /*
  * Copyright 2010 Google Inc.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -24,7 +24,7 @@ import android.util.Log;
 
 /**
  * A factory for getting the platform specific AuthManager.
- * 
+ *
  * @author Sandor Dornbush
  */
 public class AuthManagerFactory {
@@ -46,12 +46,10 @@ public class AuthManagerFactory {
   public static AuthManager getAuthManager(Activity activity, int code,
       Bundle extras, boolean requireGoogle, String service) {
     if (useModernAuthManager()) {
-      Log.i(Constants.TAG,
+      Log.i(Constants.TAG, "Creating modern auth manager: " + service);
-          "Creating modern auth manager: " + service);
+      return new ModernAuthManager(activity, service);
-      return new ModernAuthManager(activity, code, extras, requireGoogle, service);
     } else {
-      Log.i(Constants.TAG,
+      Log.i(Constants.TAG, "Creating legacy auth manager: " + service);
-          "Creating legacy auth manager: " + service);
       return new AuthManagerOld(activity, code, extras, requireGoogle, service);
     }
   }

MyTracks/src/com/google/android/apps/mytracks/io/AuthManagerOld.java

@@ -22,7 +22,6 @@ import android.app.Activity;
 import android.content.Intent;
 import android.os.Bundle;
 
-import java.util.Iterator;
 import java.util.Vector;
 
 /**
@@ -48,17 +47,11 @@ public class AuthManagerOld implements AuthManager {
   private final String service;
 
   /** A list of handlers to call when a new auth token is fetched. */
-  private final Vector<Runnable> newTokenListeners = new Vector<Runnable>();
+  private final Vector<AuthCallback> newTokenListeners = new Vector<AuthCallback>();
 
   /** The most recently fetched auth token or null if none is available. */
   private String authToken;
 
-  /**
-   * The number of handlers at the beginning of the above list that shouldn't
-   * be removed after they are called.
-   */
-  private int stickyNewTokenListenerCount;
-
   /**
    * AuthManager requires many of the same parameters as
    * {@link GoogleLoginServiceHelper#getCredentials(Activity, int, Bundle,
@@ -88,10 +81,8 @@ public class AuthManagerOld implements AuthManager {
     this.service = service;
   }
 
-  /* (non-Javadoc)
+  @Override
-   * @see com.google.android.apps.mytracks.io.AuthManager#doLogin(java.lang.Runnable)
+  public void doLogin(AuthCallback whenFinished, Object o) {
-   */
-  public void doLogin(Runnable whenFinished, Object o) {
     synchronized (newTokenListeners) {
       if (whenFinished != null) {
         newTokenListeners.add(whenFinished);
@@ -111,45 +102,43 @@ public class AuthManagerOld implements AuthManager {
     }
   }
 
-  /* (non-Javadoc)
+  @Override
-   * @see com.google.android.apps.mytracks.io.AuthManager#authResult(int, android.content.Intent)
+  public void authResult(int resultCode, Intent results) {
-   */
+    if (resultCode != Activity.RESULT_OK) {
-  public boolean authResult(int resultCode, Intent results) {
+      notifyListeners(false);
-    if (resultCode == Activity.RESULT_OK) {
+      return;
-      authToken = results.getStringExtra(
+    }
-          GoogleLoginServiceConstants.AUTHTOKEN_KEY);
+
-      if (authToken == null) {
+    authToken = results.getStringExtra(
-        GoogleLoginServiceHelper.getCredentials(
+        GoogleLoginServiceConstants.AUTHTOKEN_KEY);
-            activity, code, extras, requireGoogle, service, false);
+    if (authToken == null) {
-        return true;
+      // Retry, without prompting the user.
-      } else {
+      GoogleLoginServiceHelper.getCredentials(
-        // Notify all active listeners that we have a new auth token.
+          activity, code, extras, requireGoogle, service, false);
-        synchronized (newTokenListeners) {
+    } else {
-          Iterator<Runnable> iter = newTokenListeners.iterator();
+      // Notify all active listeners that we have a new auth token.
-          while (iter.hasNext()) {
+      notifyListeners(true);
-            iter.next().run();
-          }
-          iter = null;
-          // Remove anything not in the sticky part of the list.
-          newTokenListeners.setSize(stickyNewTokenListenerCount);
-        }
-        return true;
-      }
     }
-    return false;
   }
 
-  /* (non-Javadoc)
+  private void notifyListeners(boolean success) {
-   * @see com.google.android.apps.mytracks.io.AuthManager#getAuthToken()
+    synchronized (newTokenListeners) {
-   */
+      for (AuthCallback callback : newTokenListeners) {
+        callback.onAuthResult(success);
+      }
+
+      // Remove anything not in the sticky part of the list.
+      newTokenListeners.clear();
+    }
+  }
+
+  @Override
   public String getAuthToken() {
     return authToken;
   }
 
-  /* (non-Javadoc)
+  @Override
-   * @see com.google.android.apps.mytracks.io.AuthManager#invalidateAndRefresh(java.lang.Runnable)
+  public void invalidateAndRefresh(AuthCallback whenFinished) {
-   */
-  public void invalidateAndRefresh(Runnable whenFinished) {
     synchronized (newTokenListeners) {
       if (whenFinished != null) {
         newTokenListeners.add(whenFinished);
@@ -161,38 +150,4 @@ public class AuthManagerOld implements AuthManager {
       }
     });
   }
-
-  /**
-   * Adds a {@link Runnable} to be executed every time the auth token is
-   * updated. The {@link Runnable} will not be removed until manually removed
-   * with {@link #removeStickyNewTokenListener(Runnable)}.
-   *
-   * @param listener The {@link Runnable} to execute every time a new auth
-   *        token is fetched
-   */
-  public void addStickyNewTokenListener(Runnable listener) {
-    synchronized (newTokenListeners) {
-      newTokenListeners.add(0, listener);
-      stickyNewTokenListenerCount++;
-    }
-  }
-
-  /**
-   * Stops executing the given {@link Runnable} every time the auth token is
-   * updated. This {@link Runnable} must have been added with
-   * {@link #addStickyNewTokenListener(Runnable)} above. If the
-   * {@link Runnable} was added more than once, only the first occurrence
-   * will be removed.
-   *
-   * @param listener The {@link Runnable} to stop executing every time a new
-   *        auth token is fetched
-   */
-  public void removeStickyNewTokenListener(Runnable listener) {
-    synchronized (newTokenListeners) {
-      if (stickyNewTokenListenerCount > 0
-          && newTokenListeners.remove(listener)) {
-        stickyNewTokenListenerCount--;
-      }
-    }
-  }
 }

MyTracks/src/com/google/android/apps/mytracks/io/ModernAuthManager.java

@@ -15,6 +15,8 @@
  */
 package com.google.android.apps.mytracks.io;
 
+import static com.google.android.apps.mytracks.Constants.TAG;
+
 import com.google.android.accounts.Account;
 import com.google.android.accounts.AccountManager;
 import com.google.android.accounts.AccountManagerCallback;
@@ -49,7 +51,9 @@ public class ModernAuthManager implements AuthManager {
 
   private final AccountManager accountManager;
 
-  private Runnable whenFinished;
+  private AuthCallback whenFinished;
+
+  private Account lastAccount;
 
   /**
    * AuthManager requires many of the same parameters as
@@ -63,16 +67,9 @@ public class ModernAuthManager implements AuthManager {
    *        {@link Activity#onActivityResult} that calls
    *        {@link #authResult(int, Intent)} when {@literal code} is the request
    *        code
-   * @param code The request code to pass to
-   *        {@link Activity#onActivityResult} when
-   *        {@link #authResult(int, Intent)} should be called
-   * @param extras A {@link Bundle} of extras for
-   *        {@link com.google.android.googlelogindist.GoogleLoginServiceHelper}
-   * @param requireGoogle True if the account must be a Google account
    * @param service The name of the service to authenticate as
    */
-  public ModernAuthManager(Activity activity, int code, Bundle extras,
+  public ModernAuthManager(Activity activity, String service) {
-      boolean requireGoogle, String service) {
     this.activity = activity;
     this.service = service;
     this.accountManager = AccountManager.get(activity);
@@ -87,12 +84,19 @@ public class ModernAuthManager implements AuthManager {
    *        has been successfully fetched and is available via
    *        {@link #getAuthToken()}
    */
-  public void doLogin(final Runnable runnable, Object o) {
+  public void doLogin(AuthCallback runnable, Object o) {
     this.whenFinished = runnable;
     if (!(o instanceof Account)) {
-      throw new IllegalArgumentException("FroyoAuthManager requires an account.");
+      throw new IllegalArgumentException("ModernAuthManager requires an account.");
     }
     Account account = (Account) o;
+    doLogin(account);
+  }
+
+  private void doLogin(Account account) {
+    // Keep the account in case we need to retry.
+    this.lastAccount = account;
+
     accountManager.getAuthToken(account, service, true,
             new AccountManagerCallback<Bundle>() {
         public void run(AccountManagerFuture<Bundle> future) {
@@ -109,7 +113,7 @@ public class ModernAuthManager implements AuthManager {
 
             authToken = result.getString(
                 AccountManager.KEY_AUTHTOKEN);
-            Log.e(Constants.TAG, "Got auth token.");
+            Log.i(Constants.TAG, "Got auth token.");
             runWhenFinished();
           } catch (OperationCanceledException e) {
             Log.e(Constants.TAG, "Operation Canceled", e);
@@ -144,16 +148,23 @@ public class ModernAuthManager implements AuthManager {
    *         the auth token, or False if there was an error or the request was
    *         canceled
    */
-  public boolean authResult(int resultCode, Intent results) {
+  public void authResult(int resultCode, Intent results) {
+    boolean retry = false;
     if (results != null) {
-      authToken = results.getStringExtra(
+      authToken = results.getStringExtra(AccountManager.KEY_AUTHTOKEN);
-          AccountManager.KEY_AUTHTOKEN);
+      retry = results.getBooleanExtra("retry", false);
-      Log.w(Constants.TAG, "authResult: " + authToken);
+      Log.w(Constants.TAG, "authResult: token=" + authToken + "; extras=" + results.getExtras());
     } else {
-      Log.e(Constants.TAG, "No auth result results!!");
+      Log.e(Constants.TAG, "No auth token!!");
     }
+
+    if (authToken == null && retry) {
+      Log.i(TAG, "Retrying to get auth result");
+      doLogin(lastAccount);
+      return;
+    }
+
     runWhenFinished();
-    return authToken != null;
   }
 
   /**
@@ -175,13 +186,14 @@ public class ModernAuthManager implements AuthManager {
    * @param runnable A {@link Runnable} to execute when a new auth token
    *        is successfully fetched
    */
-  public void invalidateAndRefresh(final Runnable runnable) {
+  public void invalidateAndRefresh(final AuthCallback runnable) {
     this.whenFinished = runnable;
 
     activity.runOnUiThread(new Runnable() {
       public void run() {
         accountManager.invalidateAuthToken(Constants.ACCOUNT_TYPE,
             authToken);
+        authToken = null;
 
         AccountChooser accountChooser = new AccountChooser();
         accountChooser.chooseAccount(activity,
@@ -189,7 +201,7 @@ public class ModernAuthManager implements AuthManager {
               @Override
               public void onAccountSelected(Account account) {
                 if (account != null) {
-                  doLogin(whenFinished, account);
+                  doLogin(account);
                 } else {
                   runWhenFinished();
                 }
@@ -200,11 +212,13 @@ public class ModernAuthManager implements AuthManager {
   }
 
   private void runWhenFinished() {
+    lastAccount = null;
+
     if (whenFinished != null) {
       (new Thread() {
         @Override
         public void run() {
-          whenFinished.run();
+          whenFinished.onAuthResult(authToken != null);
         }
       }).start();
     }

MyTracks/src/com/google/android/apps/mytracks/io/gdata/GDataWrapper.java

@@ -1,12 +1,12 @@
 /*
  * Copyright 2008 Google Inc.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -17,6 +17,7 @@ package com.google.android.apps.mytracks.io.gdata;
 
 import com.google.android.apps.mytracks.Constants;
 import com.google.android.apps.mytracks.io.AuthManager;
+import com.google.android.apps.mytracks.io.AuthManager.AuthCallback;
 
 import android.util.Log;
 
@@ -26,6 +27,7 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 
 /**
@@ -132,7 +134,7 @@ public class GDataWrapper<C> {
   public static final int ERROR_CLEANED_UP = 7;
   // An unknown error occurred.
   public static final int ERROR_UNKNOWN = 100;
-  
+
   private static final int AUTH_TOKEN_INVALIDATE_REFRESH_NUM_RETRIES = 1;
   private static final int AUTH_TOKEN_INVALIDATE_REFRESH_TIMEOUT = 5000;
 
@@ -141,7 +143,7 @@ public class GDataWrapper<C> {
   private C gdataServiceClient;
   private AuthManager auth;
   private boolean retryOnAuthFailure;
-  
+
   public GDataWrapper() {
     errorType = ERROR_NO_ERROR;
     errorMessage = null;
@@ -179,7 +181,7 @@ public class GDataWrapper<C> {
           return false;
         }
       }
-      
+
       Log.d(Constants.TAG, "retrying function/query");
     }
     return false;
@@ -189,7 +191,7 @@ public class GDataWrapper<C> {
    * Execute a given function or query.  If one is executed, errorType and
    * errorMessage will contain the result/status of the function/query.
    */
-  private void runOne(final AuthenticatedFunction function, 
+  private void runOne(final AuthenticatedFunction function,
       final QueryFunction<C> query) {
     try {
       if (function != null) {
@@ -210,7 +212,7 @@ public class GDataWrapper<C> {
       errorType = ERROR_AUTH;
       errorMessage = e.getMessage();
     } catch (HttpException e) {
-      Log.e(Constants.TAG, 
+      Log.e(Constants.TAG,
           "HttpException, code " + e.getStatusCode() + " message " + e.getMessage(), e);
       errorMessage = e.getMessage();
       if (e.getStatusCode() == 401) {
@@ -241,10 +243,10 @@ public class GDataWrapper<C> {
     }
   }
 
-  /** 
+  /**
    * Invalidates and refreshes the auth token.  Blocks until the refresh has
    * completed or until we deem the refresh as having timed out.
-   * 
+   *
    * @return true If the invalidate/refresh succeeds, false if it fails or
    *   times out.
    */
@@ -252,19 +254,26 @@ public class GDataWrapper<C> {
     Log.d(Constants.TAG, "Retrying due to auth failure");
     // This FutureTask doesn't do anything -- it exists simply to be
     // blocked upon using get().
-    FutureTask<?> whenFinishedFuture = new FutureTask<Object>(new Runnable() {
+    final FutureTask<?> whenFinishedFuture = new FutureTask<Object>(new Runnable() {
       public void run() {}
     }, null);
 
-    auth.invalidateAndRefresh(whenFinishedFuture);
+    final AtomicBoolean finalSuccess = new AtomicBoolean(false);
+    auth.invalidateAndRefresh(new AuthCallback() {
+      @Override
+      public void onAuthResult(boolean success) {
+        finalSuccess.set(success);
+        whenFinishedFuture.run();
+      }
+    });
 
     try {
       Log.d(Constants.TAG, "waiting for invalidate");
-      whenFinishedFuture.get(AUTH_TOKEN_INVALIDATE_REFRESH_TIMEOUT, 
+      whenFinishedFuture.get(AUTH_TOKEN_INVALIDATE_REFRESH_TIMEOUT,
           TimeUnit.MILLISECONDS);
-      Log.d(Constants.TAG, "invalidate finished");
+      boolean success = finalSuccess.get();
-      return true;
+      Log.d(Constants.TAG, "invalidate finished, success = " + success);
-
+      return success;
     } catch (InterruptedException e) {
       Log.e(Constants.TAG, "Failed to invalidate", e);
     } catch (ExecutionException e) {
@@ -274,7 +283,7 @@ public class GDataWrapper<C> {
     } finally {
       whenFinishedFuture.cancel(false);
     }
-    
+
     return false;
   }
 
@@ -289,11 +298,11 @@ public class GDataWrapper<C> {
   public void setAuthManager(AuthManager auth) {
     this.auth = auth;
   }
-  
+
   public AuthManager getAuthManager() {
     return auth;
   }
-  
+
   public void setRetryOnAuthFailure(boolean retry) {
     retryOnAuthFailure = retry;
   }

MyTracks/src/com/google/android/apps/mytracks/io/mymaps/MyMapsGDataWrapper.java

@@ -2,6 +2,7 @@
 package com.google.android.apps.mytracks.io.mymaps;
 
 import com.google.android.apps.mytracks.io.AuthManager;
+import com.google.android.apps.mytracks.io.AuthManager.AuthCallback;
 import com.google.android.apps.mytracks.io.gdata.GDataClientFactory;
 import com.google.android.common.gdata.AndroidXmlParserFactory;
 import com.google.wireless.gdata.client.GDataClient;
@@ -126,11 +127,12 @@ class MyMapsGDataWrapper {
     }
     Log.d(MyMapsConstants.TAG, "GData error encountered: " + errorMessage);
     if (errorType == ERROR_AUTH && auth != null) {
-      Runnable whenFinished = null;
+      AuthCallback whenFinished = null;
       if (retryOnAuthFailure) {
         retriesPending++;
-        whenFinished = new Runnable() {
+        whenFinished = new AuthCallback() {
-          public void run() {
+          @Override
+          public void onAuthResult(boolean success) {
             retriesPending--;
             retryOnAuthFailure = false;
             runQuery(query);