From 8570f7440780db5c9b410e033e843b0e80e2fd27 Mon Sep 17 00:00:00 2001 From: Fred Quintana Date: Thu, 18 Feb 2010 10:32:54 -0800 Subject: [PATCH] - fix the AccountManager documentation. http://b/2401790 - only pass the authtoken through from the authenticator to the client for getAuthToken() and strip it out from the other calls, like addAccount(). http://b/2332762 - beef up the documentation to indicate what calls are allowed to be made from the main thread and which are not allowed. http://b/2384961 - wait a bit before retrying syncs that failed because one was already in progress. http://b/2414235 --- .../AbstractAccountAuthenticator.java | 8 +- .../java/android/accounts/AccountManager.java | 219 +++++++++++++++++- .../accounts/AccountManagerService.java | 32 ++- core/java/android/content/SyncManager.java | 13 ++ 4 files changed, 251 insertions(+), 21 deletions(-) diff --git a/core/java/android/accounts/AbstractAccountAuthenticator.java b/core/java/android/accounts/AbstractAccountAuthenticator.java index be2bdbefb59d0..8bc7428a6fa82 100644 --- a/core/java/android/accounts/AbstractAccountAuthenticator.java +++ b/core/java/android/accounts/AbstractAccountAuthenticator.java @@ -296,8 +296,7 @@ public abstract class AbstractAccountAuthenticator { * @@ -368,8 +367,7 @@ public abstract class AbstractAccountAuthenticator { * @@ -378,7 +376,7 @@ public abstract class AbstractAccountAuthenticator { */ public abstract Bundle updateCredentials(AccountAuthenticatorResponse response, Account account, String authTokenType, Bundle options) throws NetworkErrorException; - + /** * Checks if the account supports all the specified authenticator specific features. * @param response to send the result back to the AccountManager, will never be null diff --git a/core/java/android/accounts/AccountManager.java b/core/java/android/accounts/AccountManager.java index be15ac9968346..43a0f303dedee 100644 --- a/core/java/android/accounts/AccountManager.java +++ b/core/java/android/accounts/AccountManager.java @@ -67,6 +67,8 @@ import com.google.android.collect.Maps; * cause the running thread to block until the result is returned. Keep in mind that one * should not block the main thread in this way. Instead one should either use a callback, * thus making the call asynchronous, or make the blocking call on a separate thread. + * getResult() will throw an {@link IllegalStateException} if you call it from the main thread + * before the request has completed, i.e. before the callback has been invoked. *

* If one wants to ensure that the callback is invoked from a specific handler then they should * pass the handler to the request. This makes it easier to ensure thread-safety by running @@ -149,6 +151,8 @@ public class AccountManager { * Get the password that is associated with the account. Returns null if the account does * not exist. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission * {@link android.Manifest.permission#AUTHENTICATE_ACCOUNTS} and is running * with the same UID as the Authenticator for the account. @@ -166,6 +170,8 @@ public class AccountManager { * Get the user data named by "key" that is associated with the account. * Returns null if the account does not exist or if it does not have a value for key. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission * {@link android.Manifest.permission#AUTHENTICATE_ACCOUNTS} and is running * with the same UID as the Authenticator for the account. @@ -185,6 +191,8 @@ public class AccountManager { * @return an array that contains all the authenticators known to the AccountManager service. * This array will be empty if there are no authenticators and will never return null. *

+ * It is safe to call this method from the main thread. + *

* No permission is required to make this call. */ public AuthenticatorDescription[] getAuthenticatorTypes() { @@ -201,6 +209,8 @@ public class AccountManager { * @return an array that contains all the accounts known to the AccountManager service. * This array will be empty if there are no accounts and will never return null. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission {@link android.Manifest.permission#GET_ACCOUNTS} */ public Account[] getAccounts() { @@ -219,6 +229,8 @@ public class AccountManager { * @return an array that contains the accounts that match the specified type. This array * will be empty if no accounts match. It will never return null. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission {@link android.Manifest.permission#GET_ACCOUNTS} */ public Account[] getAccountsByType(String type) { @@ -243,6 +255,22 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Boolean result = hasFeatures(account, features, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * hasFeatures(account, features, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Boolean result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#GET_ACCOUNTS}. * * @param account The {@link Account} to test @@ -274,6 +302,8 @@ public class AccountManager { /** * Add an account to the AccountManager's set of known accounts. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission * {@link android.Manifest.permission#AUTHENTICATE_ACCOUNTS} and is running * with the same UID as the Authenticator for the account. @@ -304,6 +334,22 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Boolean result = removeAccount(account, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * removeAccount(account, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Boolean result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#MANAGE_ACCOUNTS}. * * @param account The {@link Account} to remove @@ -334,6 +380,8 @@ public class AccountManager { * Removes the given authtoken. If this authtoken does not exist for the given account type * then this call has no effect. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission {@link android.Manifest.permission#MANAGE_ACCOUNTS}. * @param accountType the account type of the authtoken to invalidate * @param authToken the authtoken to invalidate @@ -353,6 +401,8 @@ public class AccountManager { * asking the authenticaticor to generate one. If the account or the * authtoken do not exist then null is returned. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission * {@link android.Manifest.permission#AUTHENTICATE_ACCOUNTS} and is running * with the same UID as the Authenticator for the account. @@ -381,6 +431,8 @@ public class AccountManager { * Sets the password for the account. The password may be null. If the account does not exist * then this call has no affect. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission * {@link android.Manifest.permission#AUTHENTICATE_ACCOUNTS} and is running * with the same UID as the Authenticator for the account. @@ -404,6 +456,8 @@ public class AccountManager { * Sets the password for account to null. If the account does not exist then this call * has no effect. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission {@link android.Manifest.permission#MANAGE_ACCOUNTS}. * @param account the account whose password is to be cleared. Must not be null. */ @@ -424,6 +478,8 @@ public class AccountManager { * Sets account's userdata named "key" to the specified value. If the account does not * exist then this call has no effect. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission * {@link android.Manifest.permission#AUTHENTICATE_ACCOUNTS} and is running * with the same UID as the Authenticator for the account. @@ -452,6 +508,8 @@ public class AccountManager { * Sets the authtoken named by "authTokenType" to the value specified by authToken. * If the account does not exist then this call has no effect. *

+ * It is safe to call this method from the main thread. + *

* Requires that the caller has permission * {@link android.Manifest.permission#AUTHENTICATE_ACCOUNTS} and is running * with the same UID as the Authenticator for the account. @@ -473,6 +531,8 @@ public class AccountManager { * {@link #getAuthToken(Account, String, boolean, AccountManagerCallback, Handler)} * then extracts and returns the value of {@link #KEY_AUTHTOKEN} from its result. *

+ * It is not safe to call this method from the main thread. See {@link #getAuthToken}. + *

* Requires that the caller has permission {@link android.Manifest.permission#USE_CREDENTIALS}. * @param account the account whose authtoken is to be retrieved, must not be null * @param authTokenType the type of authtoken to retrieve @@ -505,9 +565,8 @@ public class AccountManager { * in the result. *

* If the authenticator needs to prompt the user for credentials it will return an intent to - * the activity that will do the prompting. If an activity is supplied then that activity - * will be used to launch the intent and the result will come from it. Otherwise a result will - * be returned that contains the intent. + * an activity that will do the prompting. The supplied activity will be used to launch the + * intent and the result will come from the launched activity. *

* This call returns immediately but runs asynchronously and the result is accessed via the * {@link AccountManagerFuture} that is returned. This future is also passed as the sole @@ -518,6 +577,23 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Bundle result = getAuthToken(
+     *   account, authTokenType, options, activity, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * getAuthToken(account, authTokenType, options, activity, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Bundle result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#USE_CREDENTIALS}. * * @param account The account whose credentials are to be updated. @@ -525,8 +601,9 @@ public class AccountManager { * May be null. * @param options authenticator specific options for the request * @param activity If the authenticator returns a {@link #KEY_INTENT} in the result then - * the intent will be started with this activity. If activity is null then the result will - * be returned as-is. + * the intent will be started with this activity. If you do not with to have the intent + * started automatically then use the other form, + * {@link #getAuthToken(Account, String, boolean, AccountManagerCallback, android.os.Handler)} * @param callback A callback to invoke when the request completes. If null then * no callback is invoked. * @param handler The {@link Handler} to use to invoke the callback. If null then the @@ -578,6 +655,23 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Bundle result = getAuthToken(
+     *   account, authTokenType, notifyAuthFailure, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * getAuthToken(account, authTokenType, notifyAuthFailure, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Bundle result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#USE_CREDENTIALS}. * * @param account The account whose credentials are to be updated. @@ -625,6 +719,23 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Bundle result = addAccount(
+     *   account, authTokenType, features, options, activity, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * addAccount(account, authTokenType, features, options, activity, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Bundle result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#MANAGE_ACCOUNTS}. * * @param accountType The type of account to add. This must not be null. @@ -646,7 +757,6 @@ public class AccountManager { *

*/ public AccountManagerFuture addAccount(final String accountType, @@ -667,6 +777,51 @@ public class AccountManager { }.start(); } + /** + * Queries for accounts that match the given account type and feature set. + *

+ * This call returns immediately but runs asynchronously and the result is accessed via the + * {@link AccountManagerFuture} that is returned. This future is also passed as the sole + * parameter to the {@link AccountManagerCallback}. If the caller wished to use this + * method asynchronously then they will generally pass in a callback object that will get + * invoked with the {@link AccountManagerFuture}. If they wish to use it synchronously then + * they will generally pass null for the callback and instead call + * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, + * which will then block until the request completes. + *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Account[] result =
+     *   getAccountsByTypeAndFeatures(accountType, features, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * getAccountsByTypeAndFeatures(accountType, features, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *         Account[] result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

+ * Requires that the caller has permission {@link android.Manifest.permission#GET_ACCOUNTS}. + * + * @param type The type of {@link Account} to return. If null is passed in then an empty + * array will be returned. + * @param features the features with which to filter the accounts list. Each returned account + * will have all specified features. This may be null, which will mean the account list will + * not be filtered by features, making this functionally identical to + * {@link #getAccountsByType(String)}. + * @param callback A callback to invoke when the request completes. If null then + * no callback is invoked. + * @param handler The {@link Handler} to use to invoke the callback. If null then the + * main thread's {@link Handler} is used. + * @return an {@link AccountManagerFuture} that represents the future result of the call. + * The future result is a an {@link Account} array that contains accounts of the specified + * type that match all the requested features. + */ public AccountManagerFuture getAccountsByTypeAndFeatures( final String type, final String[] features, AccountManagerCallback callback, Handler handler) { @@ -709,6 +864,23 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Bundle result = confirmCredentials(
+     *   account, options, activity, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * confirmCredentials(account, options, activity, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Bundle result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#MANAGE_ACCOUNTS}. * * @param account The account whose credentials are to be checked @@ -757,6 +929,23 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Bundle result = updateCredentials(
+     *   account, authTokenType, options, activity, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * updateCredentials(account, authTokenType, options, activity, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Bundle result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#MANAGE_ACCOUNTS}. * * @param account The account whose credentials are to be updated. @@ -775,7 +964,7 @@ public class AccountManager { *

* If the user presses "back" then the request will be canceled. */ @@ -807,6 +996,22 @@ public class AccountManager { * {@link android.accounts.AccountManagerFuture#getResult()} on this method's return value, * which will then block until the request completes. *

+ * Do not block the main thread waiting this method's result. + *

+ * Not allowed from main thread (but allowed from other threads): + * 

+     * Bundle result = editProperties(accountType, activity, callback, handler).getResult();
+     *
+ * Allowed from main thread: + * 
+     * editProperties(accountType, activity, new AccountManagerCallback() {
+     *    public void run(AccountManagerFuture future) {
+     *        Bundle result = future.getResult();
+     *        // use result
+     *    }
+     * }, handler);
+     *
+ *

* Requires that the caller has permission {@link android.Manifest.permission#MANAGE_ACCOUNTS}. * * @param accountType The account type of the authenticator whose properties are to be edited. diff --git a/core/java/android/accounts/AccountManagerService.java b/core/java/android/accounts/AccountManagerService.java index 785012418c267..770554eed67c0 100644 --- a/core/java/android/accounts/AccountManagerService.java +++ b/core/java/android/accounts/AccountManagerService.java @@ -466,7 +466,8 @@ public class AccountManagerService public TestFeaturesSession(IAccountManagerResponse response, Account account, String[] features) { - super(response, account.type, false /* expectActivityLaunch */); + super(response, account.type, false /* expectActivityLaunch */, + true /* stripAuthTokenFromResult */); mFeatures = features; mAccount = account; } @@ -520,7 +521,8 @@ public class AccountManagerService private class RemoveAccountSession extends Session { final Account mAccount; public RemoveAccountSession(IAccountManagerResponse response, Account account) { - super(response, account.type, false /* expectActivityLaunch */); + super(response, account.type, false /* expectActivityLaunch */, + true /* stripAuthTokenFromResult */); mAccount = account; } @@ -794,7 +796,8 @@ public class AccountManagerService } } - new Session(response, account.type, expectActivityLaunch) { + new Session(response, account.type, expectActivityLaunch, + false /* stripAuthTokenFromResult */) { protected String toDebugString(long now) { if (loginOptions != null) loginOptions.keySet(); return super.toDebugString(now) + ", getAuthToken" @@ -945,7 +948,8 @@ public class AccountManagerService checkManageAccountsPermission(); long identityToken = clearCallingIdentity(); try { - new Session(response, accountType, expectActivityLaunch) { + new Session(response, accountType, expectActivityLaunch, + true /* stripAuthTokenFromResult */) { public void run() throws RemoteException { mAuthenticator.addAccount(this, mAccountType, authTokenType, requiredFeatures, options); @@ -970,7 +974,8 @@ public class AccountManagerService checkManageAccountsPermission(); long identityToken = clearCallingIdentity(); try { - new Session(response, account.type, expectActivityLaunch) { + new Session(response, account.type, expectActivityLaunch, + true /* stripAuthTokenFromResult */) { public void run() throws RemoteException { mAuthenticator.confirmCredentials(this, account, options); } @@ -990,7 +995,8 @@ public class AccountManagerService checkManageAccountsPermission(); long identityToken = clearCallingIdentity(); try { - new Session(response, account.type, expectActivityLaunch) { + new Session(response, account.type, expectActivityLaunch, + true /* stripAuthTokenFromResult */) { public void run() throws RemoteException { mAuthenticator.updateCredentials(this, account, authTokenType, loginOptions); } @@ -1012,7 +1018,8 @@ public class AccountManagerService checkManageAccountsPermission(); long identityToken = clearCallingIdentity(); try { - new Session(response, accountType, expectActivityLaunch) { + new Session(response, accountType, expectActivityLaunch, + true /* stripAuthTokenFromResult */) { public void run() throws RemoteException { mAuthenticator.editProperties(this, mAccountType); } @@ -1034,7 +1041,8 @@ public class AccountManagerService public GetAccountsByTypeAndFeatureSession(IAccountManagerResponse response, String type, String[] features) { - super(response, type, false /* expectActivityLaunch */); + super(response, type, false /* expectActivityLaunch */, + true /* stripAuthTokenFromResult */); mFeatures = features; } @@ -1176,11 +1184,14 @@ public class AccountManagerService IAccountAuthenticator mAuthenticator = null; + private final boolean mStripAuthTokenFromResult; + public Session(IAccountManagerResponse response, String accountType, - boolean expectActivityLaunch) { + boolean expectActivityLaunch, boolean stripAuthTokenFromResult) { super(); if (response == null) throw new IllegalArgumentException("response is null"); if (accountType == null) throw new IllegalArgumentException("accountType is null"); + mStripAuthTokenFromResult = stripAuthTokenFromResult; mResponse = response; mAccountType = accountType; mExpectActivityLaunch = expectActivityLaunch; @@ -1319,6 +1330,9 @@ public class AccountManagerService response.onError(AccountManager.ERROR_CODE_INVALID_RESPONSE, "null bundle returned"); } else { + if (mStripAuthTokenFromResult) { + result.remove(AccountManager.KEY_AUTHTOKEN); + } response.onResult(result); } } catch (RemoteException e) { diff --git a/core/java/android/content/SyncManager.java b/core/java/android/content/SyncManager.java index 4ddb819a0f453..317e5a9beb71c 100644 --- a/core/java/android/content/SyncManager.java +++ b/core/java/android/content/SyncManager.java @@ -114,6 +114,11 @@ public class SyncManager implements OnAccountsUpdateListener { */ private static final long DEFAULT_MAX_SYNC_RETRY_TIME_IN_SECONDS = 60 * 60; // one hour + /** + * How long to wait before retrying a sync that failed due to one already being in progress. + */ + private static final int DELAY_RETRY_SYNC_IN_PROGRESS_IN_SECONDS = 10; + /** * An error notification is sent if sync of any of the providers has been failing for this long. */ @@ -807,6 +812,14 @@ public class SyncManager implements OnAccountsUpdateListener { + "it achieved some success"); } scheduleSyncOperation(operation); + } else if (syncResult.syncAlreadyInProgress) { + if (isLoggable) { + Log.d(TAG, "retrying sync operation that failed because there was already a " + + "sync in progress: " + operation); + } + scheduleSyncOperation(new SyncOperation(operation.account, operation.syncSource, + operation.authority, operation.extras, + DELAY_RETRY_SYNC_IN_PROGRESS_IN_SECONDS)); } else if (syncResult.hasSoftError()) { if (isLoggable) { Log.d(TAG, "retrying sync operation because it encountered a soft error: "