Google accounts in android phone do not get pushed into content area. The code already handles several cases correctly:

- when signing to chrome after mirror is enabled, the accounts will be pushed into the content area correctly
- if accounts are added while chrome is still running in the background, the accounts will be pushed into the content area correctly

The missing case is when the user is already signed in and then updates to the latest version.

BUG=381881
TBR=sky@chromium.org

Review URL: https://codereview.chromium.org/326723003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276963 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 135 insertions, 28 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
index 9adf0cc..deca8f5 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java
@@ -20,11 +20,11 @@ import org.chromium.sync.signin.AccountManagerHelper;
 import org.chromium.sync.signin.ChromeSigninController;
 
 import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.HashSet;
-import java.util.Set;
 
 import javax.annotation.Nullable;
 
@@ -42,6 +42,10 @@ public final class OAuth2TokenService {
     @VisibleForTesting
     public static final String STORED_ACCOUNTS_KEY = "google.services.stored_accounts";
 
+    /**
+     * Classes that want to listen for refresh token availability should
+     * implement this interface and register with {@link #addObserver}.
+     */
     public interface OAuth2TokenServiceObserver {
         void onRefreshTokenAvailable(Account account);
         void onRefreshTokenRevoked(Account account);
@@ -219,11 +223,21 @@ public final class OAuth2TokenService {
         }
     }
 
+    /**
+     * TODO(rogerta): This overload exists until a CL lands in the clank repo to use the
+     * version that takes a boolean second arg.
+     */
     public void validateAccounts(Context context) {
+      validateAccounts(context, false);
+    }
+
+    @CalledByNative
+    public void validateAccounts(Context context, boolean forceNotifications) {
         ThreadUtils.assertOnUiThread();
         String currentlySignedInAccount =
                 ChromeSigninController.get(context).getSignedInAccountName();
-        nativeValidateAccounts(mNativeProfileOAuth2TokenService, currentlySignedInAccount);
+        nativeValidateAccounts(mNativeProfileOAuth2TokenService, currentlySignedInAccount,
+                               forceNotifications);
     }
 
     /**
@@ -300,7 +314,8 @@ public final class OAuth2TokenService {
             String authToken, boolean result, long nativeCallback);
     private native void nativeValidateAccounts(
             long nativeAndroidProfileOAuth2TokenService,
-            String currentlySignedInAccount);
+            String currentlySignedInAccount,
+            boolean forceNotifications);
     private native void nativeFireRefreshTokenAvailableFromJava(
             long nativeAndroidProfileOAuth2TokenService, String accountName);
     private native void nativeFireRefreshTokenRevokedFromJava(
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
index 12d965c..4ae56c3 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
@@ -52,7 +52,10 @@ public class OAuth2TokenServiceIntegrationTest extends ChromeShellTestBase {
         mContext = new AdvancedMockContext(getInstrumentation().getTargetContext());
         mAccountManager = new MockAccountManager(mContext, getInstrumentation().getContext());
         AccountManagerHelper.overrideAccountManagerHelperForTests(mContext, mAccountManager);
+
+        // Make sure there is no account signed in yet.
         mChromeSigninController = ChromeSigninController.get(mContext);
+        mChromeSigninController.setSignedInAccountName(null);
 
         // Get a reference to the service.
         mOAuth2TokenService = getOAuth2TokenServiceOnUiThread();
@@ -193,10 +196,22 @@ public class OAuth2TokenServiceIntegrationTest extends ChromeShellTestBase {
         // Run test.
         mOAuth2TokenService.validateAccounts(mContext);
 
-        // Ensure no calls have been made to the observer.
+        // Ensure one call for the signed in account.
         assertEquals(1, mObserver.getAvailableCallCount());
         assertEquals(0, mObserver.getRevokedCallCount());
         assertEquals(0, mObserver.getLoadedCallCount());
+
+        // Validate again and make sure no new calls are made.
+        mOAuth2TokenService.validateAccounts(mContext);
+        assertEquals(1, mObserver.getAvailableCallCount());
+        assertEquals(0, mObserver.getRevokedCallCount());
+        assertEquals(0, mObserver.getLoadedCallCount());
+
+        // Validate again with force notifications and make sure one new calls is made.
+        mOAuth2TokenService.validateAccounts(mContext, true);
+        assertEquals(2, mObserver.getAvailableCallCount());
+        assertEquals(0, mObserver.getRevokedCallCount());
+        assertEquals(0, mObserver.getLoadedCallCount());
     }
 
     @MediumTest
@@ -216,7 +231,7 @@ public class OAuth2TokenServiceIntegrationTest extends ChromeShellTestBase {
 
         // Re-run validation.
         mOAuth2TokenService.validateAccounts(mContext);
-        assertEquals(2, mObserver.getAvailableCallCount());
+        assertEquals(1, mObserver.getAvailableCallCount());
         assertEquals(0, mObserver.getRevokedCallCount());
         assertEquals(0, mObserver.getLoadedCallCount());
     }
@@ -241,7 +256,7 @@ public class OAuth2TokenServiceIntegrationTest extends ChromeShellTestBase {
 
         // Re-run validation.
         mOAuth2TokenService.validateAccounts(mContext);
-        assertEquals(3, mObserver.getAvailableCallCount());
+        assertEquals(2, mObserver.getAvailableCallCount());
         assertEquals(0, mObserver.getRevokedCallCount());
         assertEquals(0, mObserver.getLoadedCallCount());
     }
@@ -263,7 +278,7 @@ public class OAuth2TokenServiceIntegrationTest extends ChromeShellTestBase {
         mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
         mOAuth2TokenService.validateAccounts(mContext);
 
-        assertEquals(3, mObserver.getAvailableCallCount());
+        assertEquals(2, mObserver.getAvailableCallCount());
         assertEquals(1, mObserver.getRevokedCallCount());
         assertEquals(0, mObserver.getLoadedCallCount());
     }
@@ -346,7 +361,7 @@ public class OAuth2TokenServiceIntegrationTest extends ChromeShellTestBase {
 
         // Ensure no calls have been made to the observer.
         assertEquals(0, mObserver.getAvailableCallCount());
-        assertEquals(1, mObserver.getRevokedCallCount());
+        assertEquals(0, mObserver.getRevokedCallCount());
         assertEquals(0, mObserver.getLoadedCallCount());
     }
 
diff --git a/chrome/browser/android/signin/signin_manager_android.cc b/chrome/browser/android/signin/signin_manager_android.cc
index 0213309..37cd951 100644
--- a/chrome/browser/android/signin/signin_manager_android.cc
+++ b/chrome/browser/android/signin/signin_manager_android.cc
@@ -233,7 +233,7 @@ void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env, jobject obj) {
             profile_);
     const std::string& primary_acct =
         signin_manager->GetAuthenticatedAccountId();
-    token_service->ValidateAccounts(primary_acct);
+    token_service->ValidateAccounts(primary_acct, true);
 
   } else {
     DVLOG(1) << "SigninManagerAndroid::LogInSignedInUser "
diff --git a/chrome/browser/signin/android_profile_oauth2_token_service.cc b/chrome/browser/signin/android_profile_oauth2_token_service.cc
index 6922489..cb2dea7 100644
--- a/chrome/browser/signin/android_profile_oauth2_token_service.cc
+++ b/chrome/browser/signin/android_profile_oauth2_token_service.cc
@@ -48,7 +48,10 @@ typedef base::Callback<void(
 
 }  // namespace
 
+bool AndroidProfileOAuth2TokenService::is_testing_profile_ = false;
+
 AndroidProfileOAuth2TokenService::AndroidProfileOAuth2TokenService() {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::ctor";
   JNIEnv* env = AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jobject> local_java_ref =
       Java_OAuth2TokenService_create(env, reinterpret_cast<intptr_t>(this));
@@ -73,6 +76,17 @@ static jobject GetForProfile(JNIEnv* env,
       env, clazz, j_profile_android);
 }
 
+void AndroidProfileOAuth2TokenService::Initialize(SigninClient* client) {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::Initialize";
+  ProfileOAuth2TokenService::Initialize(client);
+
+  if (!is_testing_profile_) {
+    Java_OAuth2TokenService_validateAccounts(
+        AttachCurrentThread(), java_ref_.obj(),
+        base::android::GetApplicationContext(), JNI_TRUE);
+  }
+}
+
 bool AndroidProfileOAuth2TokenService::RefreshTokenIsAvailable(
     const std::string& account_id) const {
   JNIEnv* env = AttachCurrentThread();
@@ -82,7 +96,7 @@ bool AndroidProfileOAuth2TokenService::RefreshTokenIsAvailable(
       Java_OAuth2TokenService_hasOAuth2RefreshToken(
           env, base::android::GetApplicationContext(),
           j_account_id.obj());
-  return refresh_token_is_available != JNI_FALSE;
+  return refresh_token_is_available == JNI_TRUE;
 }
 
 std::vector<std::string> AndroidProfileOAuth2TokenService::GetAccounts() {
@@ -172,20 +186,29 @@ void AndroidProfileOAuth2TokenService::InvalidateOAuth2Token(
 void AndroidProfileOAuth2TokenService::ValidateAccounts(
     JNIEnv* env,
     jobject obj,
-    jstring j_current_acc) {
+    jstring j_current_acc,
+    jboolean j_force_notifications) {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts from java";
   std::string signed_in_account = ConvertJavaStringToUTF8(env, j_current_acc);
-  ValidateAccounts(signed_in_account);
+  ValidateAccounts(signed_in_account, j_force_notifications != JNI_FALSE);
 }
 
 void AndroidProfileOAuth2TokenService::ValidateAccounts(
-    const std::string& signed_in_account) {
+    const std::string& signed_in_account,
+    bool force_notifications) {
   std::vector<std::string> prev_ids = GetAccounts();
   std::vector<std::string> curr_ids = GetSystemAccounts();
   std::vector<std::string> refreshed_ids;
   std::vector<std::string> revoked_ids;
 
-  if (!ValidateAccounts(
-      signed_in_account, prev_ids, curr_ids, refreshed_ids, revoked_ids)) {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts:"
+          << " sigined_in_account=" << signed_in_account
+          << " prev_ids=" << prev_ids.size()
+          << " curr_ids=" << curr_ids.size()
+          << " force=" << (force_notifications ? "true" : "false");
+
+  if (!ValidateAccounts(signed_in_account, prev_ids, curr_ids, refreshed_ids,
+                        revoked_ids, force_notifications)) {
     curr_ids.clear();
   }
 
@@ -211,7 +234,8 @@ bool AndroidProfileOAuth2TokenService::ValidateAccounts(
     const std::vector<std::string>& prev_account_ids,
     const std::vector<std::string>& curr_account_ids,
     std::vector<std::string>& refreshed_ids,
-    std::vector<std::string>& revoked_ids) {
+    std::vector<std::string>& revoked_ids,
+    bool force_notifications) {
   if (std::find(curr_account_ids.begin(),
                 curr_account_ids.end(),
                 signed_in_account) != curr_account_ids.end()) {
@@ -225,30 +249,50 @@ bool AndroidProfileOAuth2TokenService::ValidateAccounts(
       if (std::find(curr_account_ids.begin(),
                     curr_account_ids.end(),
                     *it) == curr_account_ids.end()) {
+        VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts:"
+                << "revoked=" << *it;
         revoked_ids.push_back(*it);
       }
     }
 
-    // Always fire the primary signed in account first.
-    refreshed_ids.push_back(signed_in_account);
+    if (force_notifications ||
+        std::find(prev_account_ids.begin(), prev_account_ids.end(),
+                  signed_in_account) == prev_account_ids.end()) {
+      // Always fire the primary signed in account first.
+      VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts:"
+              << "refreshed=" << signed_in_account;
+      refreshed_ids.push_back(signed_in_account);
+    }
 
     for (std::vector<std::string>::const_iterator it = curr_account_ids.begin();
          it != curr_account_ids.end(); it++) {
       if (*it != signed_in_account) {
-        refreshed_ids.push_back(*it);
+        if (force_notifications ||
+            std::find(prev_account_ids.begin(),
+                      prev_account_ids.end(),
+                      *it) == prev_account_ids.end()) {
+          VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts:"
+                  << "refreshed=" << *it;
+          refreshed_ids.push_back(*it);
+        }
       }
     }
     return true;
   } else {
     // Currently signed in account does not any longer exist among accounts on
     // system together with all other accounts.
-    if (!signed_in_account.empty()) {
+    if (std::find(prev_account_ids.begin(), prev_account_ids.end(),
+                  signed_in_account) != prev_account_ids.end()) {
+      VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts:"
+              << "revoked=" << signed_in_account;
       revoked_ids.push_back(signed_in_account);
     }
     for (std::vector<std::string>::const_iterator it = prev_account_ids.begin();
          it != prev_account_ids.end(); it++) {
       if (*it == signed_in_account)
         continue;
+      VLOG(1) << "AndroidProfileOAuth2TokenService::ValidateAccounts:"
+              << "revoked=" << *it;
       revoked_ids.push_back(*it);
     }
     return false;
@@ -265,6 +309,9 @@ void AndroidProfileOAuth2TokenService::FireRefreshTokenAvailableFromJava(
 
 void AndroidProfileOAuth2TokenService::FireRefreshTokenAvailable(
     const std::string& account_id) {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::FireRefreshTokenAvailable id="
+          << account_id;
+
   // Notify native observers.
   OAuth2TokenService::FireRefreshTokenAvailable(account_id);
   // Notify Java observers.
@@ -285,6 +332,9 @@ void AndroidProfileOAuth2TokenService::FireRefreshTokenRevokedFromJava(
 
 void AndroidProfileOAuth2TokenService::FireRefreshTokenRevoked(
     const std::string& account_id) {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::FireRefreshTokenRevoked id="
+          << account_id;
+
   // Notify native observers.
   OAuth2TokenService::FireRefreshTokenRevoked(account_id);
   // Notify Java observers.
@@ -302,6 +352,7 @@ void AndroidProfileOAuth2TokenService::FireRefreshTokensLoadedFromJava(
 }
 
 void AndroidProfileOAuth2TokenService::FireRefreshTokensLoaded() {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::FireRefreshTokensLoaded";
   // Notify native observers.
   OAuth2TokenService::FireRefreshTokensLoaded();
   // Notify Java observers.
@@ -311,6 +362,7 @@ void AndroidProfileOAuth2TokenService::FireRefreshTokensLoaded() {
 }
 
 void AndroidProfileOAuth2TokenService::RevokeAllCredentials() {
+  VLOG(1) << "AndroidProfileOAuth2TokenService::RevokeAllCredentials";
   std::vector<std::string> accounts = GetAccounts();
   for (std::vector<std::string>::iterator it = accounts.begin();
        it != accounts.end(); it++) {
diff --git a/chrome/browser/signin/android_profile_oauth2_token_service.h b/chrome/browser/signin/android_profile_oauth2_token_service.h
index cab9314..f2cb8e2 100644
--- a/chrome/browser/signin/android_profile_oauth2_token_service.h
+++ b/chrome/browser/signin/android_profile_oauth2_token_service.h
@@ -36,10 +36,17 @@ class AndroidProfileOAuth2TokenService : public ProfileOAuth2TokenService {
   static jobject GetForProfile(
       JNIEnv* env, jclass clazz, jobject j_profile_android);
 
+  // Called by the TestingProfile class to disable account validation in
+  // tests.  This prevents the token service from trying to look up system
+  // accounts which requires special permission.
+  static void set_is_testing_profile() {
+    is_testing_profile_ = true;
+  }
+
+  // ProfileOAuth2TokenService overrides:
+  virtual void Initialize(SigninClient* client) OVERRIDE;
   virtual bool RefreshTokenIsAvailable(
       const std::string& account_id) const OVERRIDE;
-
-  // Lists account IDs of all accounts with a refresh token.
   virtual std::vector<std::string> GetAccounts() OVERRIDE;
 
   // Lists account at the OS level.
@@ -47,11 +54,15 @@ class AndroidProfileOAuth2TokenService : public ProfileOAuth2TokenService {
 
   void ValidateAccounts(JNIEnv* env,
                         jobject obj,
-                        jstring current_account);
+                        jstring current_account,
+                        jboolean force_notifications);
 
   // Takes a the signed in sync account as well as all the other
-  // android account ids and check the token status of each.
-  void ValidateAccounts(const std::string& signed_in_account);
+  // android account ids and check the token status of each.  If
+  // |force_notifications| is true, TokenAvailable notifications will
+  // be sent anyway, even if the account was already known.
+  void ValidateAccounts(const std::string& signed_in_account,
+                        bool force_notifications);
 
   // Triggers a notification to all observers of the OAuth2TokenService that a
   // refresh token is now available. This may cause observers to retry
@@ -110,16 +121,21 @@ class AndroidProfileOAuth2TokenService : public ProfileOAuth2TokenService {
   virtual void FireRefreshTokensLoaded() OVERRIDE;
 
   // Return whether |signed_in_account| is valid and we have access
-  // to all the tokens in |curr_account_ids|.
+  // to all the tokens in |curr_account_ids|. If |force_notifications| is true,
+  // TokenAvailable notifications will be sent anyway, even if the account was
+  // already known.
   bool ValidateAccounts(const std::string& signed_in_account,
                         const std::vector<std::string>& prev_account_ids,
                         const std::vector<std::string>& curr_account_ids,
                         std::vector<std::string>& refreshed_ids,
-                        std::vector<std::string>& revoked_ids);
+                        std::vector<std::string>& revoked_ids,
+                        bool force_notifications);
 
  private:
   base::android::ScopedJavaGlobalRef<jobject> java_ref_;
 
+  static bool is_testing_profile_;
+
   DISALLOW_COPY_AND_ASSIGN(AndroidProfileOAuth2TokenService);
 };
 
diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc
index 04c63ab..043d5ee 100644
--- a/chrome/test/base/testing_profile.cc
+++ b/chrome/test/base/testing_profile.cc
@@ -92,6 +92,10 @@
 #include "chrome/browser/guest_view/guest_view_manager.h"
 #endif
 
+#if defined(OS_ANDROID)
+#include "chrome/browser/signin/android_profile_oauth2_token_service.h"
+#endif
+
 #if defined(ENABLE_MANAGED_USERS)
 #include "chrome/browser/managed_mode/managed_user_settings_service.h"
 #include "chrome/browser/managed_mode/managed_user_settings_service_factory.h"
@@ -324,6 +328,11 @@ void TestingProfile::Init() {
              content::BrowserThread::UI) ||
          content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
+#if defined(OS_ANDROID)
+  // Make sure token service knows its running in tests.
+  AndroidProfileOAuth2TokenService::set_is_testing_profile();
+#endif
+
   // Normally this would happen during browser startup, but for tests
   // we need to trigger creation of Profile-related services.
   ChromeBrowserMainExtraPartsProfiles::