Commit e8f9faf5 authored by yuweih's avatar yuweih Committed by Commit bot

[Remoting Android] Fix OAuthTokenConsumer's thread safety issue

OAuthTokenConsumer.revokeLatestToken() revokes the token on the background
thread and calls the callback directly on that thread once it is done. This
would be potentially thread unsafe if the caller doesn't realize the callback
will be called on a non-main thread. Currently we don't see any bug related
to this problem since the only caller of revokeLatestToken() passes in null
as the callback.

This CL makes revokeLatestToken() post a task to the main thread to run the
callback when the token is revoked.

Review-Url: https://codereview.chromium.org/2277723003
Cr-Commit-Position: refs/heads/master@{#414268}
parent 5a02d174
...@@ -6,6 +6,8 @@ package org.chromium.chromoting; ...@@ -6,6 +6,8 @@ package org.chromium.chromoting;
import android.content.Context; import android.content.Context;
import android.os.AsyncTask; import android.os.AsyncTask;
import android.os.Handler;
import android.os.Looper;
import com.google.android.gms.auth.GoogleAuthException; import com.google.android.gms.auth.GoogleAuthException;
import com.google.android.gms.auth.GoogleAuthUtil; import com.google.android.gms.auth.GoogleAuthUtil;
...@@ -37,7 +39,8 @@ public class OAuthTokenConsumer { ...@@ -37,7 +39,8 @@ public class OAuthTokenConsumer {
/** /**
* Retrieves the auth token and call the callback when it is done. callback.onTokenFetched() * Retrieves the auth token and call the callback when it is done. callback.onTokenFetched()
* will be called if the retrieval succeeds, otherwise callback.onError() will be called. * will be called on the main thread if the retrieval succeeds, otherwise callback.onError()
* will be called.
* The callback will not be run if the task is already running and false will be returned in * The callback will not be run if the task is already running and false will be returned in
* that case. * that case.
* Each OAuthTokenConsumer is supposed to work for one specific task. It is the caller's * Each OAuthTokenConsumer is supposed to work for one specific task. It is the caller's
...@@ -86,9 +89,10 @@ public class OAuthTokenConsumer { ...@@ -86,9 +89,10 @@ public class OAuthTokenConsumer {
/** /**
* Revokes the latest token fetched by the consumer. * Revokes the latest token fetched by the consumer.
* @param callback onTokenFetch(null) will be called if the token is cleared successfully. * @param callback onTokenFetch(null) will be called on the main thread if the token is cleared
* onError(error) will be called if any error occurs. |callback| can be null, * successfully. onError(error) will be called if any error occurs. |callback|
* in which case token will be cleared without running the callback. * can be null, in which case token will be cleared without running the
* callback.
*/ */
public void revokeLatestToken(final OAuthTokenFetcher.Callback callback) { public void revokeLatestToken(final OAuthTokenFetcher.Callback callback) {
new AsyncTask<Void, Void, Void>() { new AsyncTask<Void, Void, Void>() {
...@@ -98,19 +102,35 @@ public class OAuthTokenConsumer { ...@@ -98,19 +102,35 @@ public class OAuthTokenConsumer {
GoogleAuthUtil.clearToken(mContext, mLatestToken); GoogleAuthUtil.clearToken(mContext, mLatestToken);
mLatestToken = null; mLatestToken = null;
if (callback != null) { if (callback != null) {
callback.onTokenFetched(null); new Handler(Looper.getMainLooper()).post(new Runnable() {
@Override
public void run() {
callback.onTokenFetched(null);
}
});
} }
} catch (GoogleAuthException e) { } catch (GoogleAuthException e) {
if (callback != null) { if (callback != null) {
handleErrorOnMainThread(callback, OAuthTokenFetcher.Error.UNEXPECTED);
callback.onError(OAuthTokenFetcher.Error.UNEXPECTED); callback.onError(OAuthTokenFetcher.Error.UNEXPECTED);
} }
} catch (IOException e) { } catch (IOException e) {
if (callback != null) { if (callback != null) {
callback.onError(OAuthTokenFetcher.Error.NETWORK); handleErrorOnMainThread(callback, OAuthTokenFetcher.Error.NETWORK);
} }
} }
return null; return null;
} }
}.execute(); }.execute();
} }
private void handleErrorOnMainThread(final OAuthTokenFetcher.Callback callback,
final OAuthTokenFetcher.Error error) {
new Handler(Looper.getMainLooper()).post(new Runnable() {
@Override
public void run() {
callback.onError(error);
}
});
}
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment