[Remoting Android] Refactor OAuth Token Fetching Code

remoting/android/java/src/org/chromium/chromoting/OAuthTokenConsumer.java

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chromoting;
+
+import android.app.Activity;
+
+import org.chromium.chromoting.base.OAuthTokenFetcher;
+
+/**
+ * This helper guards same auth token requesting task shouldn't be run more than once at the same
+ * time.
+ */
+public class OAuthTokenConsumer {
+    private Activity mActivity;
+    private String mTokenScope;
+    private boolean mWaitingForAuthToken;
+    private String mLastToken;

[Lambros, 2016/05/13 01:11:31]

mLatestToken or mNewestToken or mCurrentToken or just mToken? In English, "last"
is ambiguous - it could mean either "latest" or "previous".

[Yuwei, 2016/05/13 17:20:29]

Done.

+
+    /**
+     * @param activity The Chromoting activity.
+     * @param tokenScope Scope to use when fetching the OAuth token.
+     */
+    public OAuthTokenConsumer(Activity activity, String tokenScope) {
+        mActivity = activity;
+        mTokenScope = tokenScope;
+        mWaitingForAuthToken = false;
+    }
+
+    /**
+     * 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.
+     * @param account User's account name (email).
+     * @param callback the callback to be called
+     * @return true if the same task has not been run and false if the task has already been
+     *         running.
+     */
+    public boolean consume(String account, final OAuthTokenFetcher.Callback callback) {

[Lambros, 2016/05/13 01:11:31]

If consume() is called several times with different |callback| parameters, only
one of them will actually get run?
I assume it's the caller's responsibility to supply the same (or equivalent)
callback each time?
Maybe update the JavaDoc to clarify this? The callback is used only when this
method returns true.

[Yuwei, 2016/05/13 03:53:58]

Right. Only the first callback will get run.
Yes. Except that the closure can capture changing variable like hostId.
I'll update the JavaDoc to clarify all of these.

[Yuwei, 2016/05/13 17:20:29]

Added more details.

+        if (mWaitingForAuthToken) {
+            return false;
+        }
+        mWaitingForAuthToken = true;
+
+        OAuthTokenFetcher fetcher = new OAuthTokenFetcher(mActivity, account, mTokenScope,

[Lambros, 2016/05/13 01:11:31]

Maybe just do:
new OAuthTokenFetcher(...).fetch();
to get rid of the temporary variable?

[Yuwei, 2016/05/13 17:20:30]

Done.

+                new OAuthTokenFetcher.Callback() {
+                    @Override
+                    public void onTokenFetched(String token) {
+                        mWaitingForAuthToken = false;
+                        mLastToken = token;
+                        callback.onTokenFetched(token);
+                    }
+
+                    @Override
+                    public void onError(OAuthTokenFetcher.Error error) {
+                        mWaitingForAuthToken = false;
+                        callback.onError(error);
+                    }
+                });
+        fetcher.fetch();
+        return true;
+    }
+
+    /**
+     * @return Last fetched auth token. This should be used right after calling consume() or
+     *         retry(). The token may become invalid after some amount of time.

[Yuwei, 2016/05/13 00:30:17]

Oops... forgot to remove retry()

[Yuwei, 2016/05/13 17:20:30]

Done.

+     */
+    public String getLastToken() {
+        return mLastToken;
+    }
+}