[Assistant Voice Search] Observe changes in Gsa account and update state

The account check can complete after startup completes, which leaves
the ui components out of sync with each other and the current assistant
capabilities. Adding an observer to GSAState to monitor for account
changes and updating AssistantVoiceSearchService clients accordingly.

Bug: 1163196
Change-Id: I02c17425458544cf5afaa9fdd434f661061b0014
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611655Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Nick Burris <nburris@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841540}

chrome/android/chrome_test_java_sources.gni

@@ -347,6 +347,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/omnibox/suggestions/tiles/TileSuggestionProcessorUnitTest.java",
   "javatests/src/org/chromium/chrome/browser/omnibox/voice/AssistantVoiceSearchConsentUiRenderTest.java",
   "javatests/src/org/chromium/chrome/browser/omnibox/voice/AssistantVoiceSearchConsentUiTest.java",
+  "javatests/src/org/chromium/chrome/browser/omnibox/voice/AssistantVoiceSearchServiceRenderTest.java",
   "javatests/src/org/chromium/chrome/browser/omnibox/voice/VoiceRecognitionHandlerTest.java",
   "javatests/src/org/chromium/chrome/browser/page_info/ConnectionInfoViewTest.java",
   "javatests/src/org/chromium/chrome/browser/page_info/CookieControlsViewTest.java",

chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAState.java

@@ -21,6 +21,7 @@ import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.Log;
+import org.chromium.base.ObserverList;
 import org.chromium.base.PackageManagerUtils;
 import org.chromium.base.PackageUtils;
 import org.chromium.base.ThreadUtils;
@@ -35,9 +36,15 @@ import org.chromium.components.signin.identitymanager.IdentityManager;
 import java.util.List;
 
 /**
- * A class responsible fore representing the current state of Chrome's integration with GSA.
+ * A class responsible for representing the current state of Chrome's integration with GSA.
  */
 public class GSAState {
+    /** Used to observe state changes in the class. */
+    public interface Observer {
+        /** Called when the GSA account name is set. */
+        void onSetGsaAccount();
+    }
+
     private static final String TAG = "GSAState";
 
     private static final int GSA_VERSION_FOR_DOCUMENT = 300401021;
@@ -67,6 +74,7 @@ public class GSAState {
      * The application context to use.
      */
     private final Context mContext;
+    private final ObserverList<Observer> mObserverList = new ObserverList<>();
 
     /**
      * Caches the result of a computation on whether GSA is available.
@@ -74,10 +82,10 @@ public class GSAState {
     private Boolean mGsaAvailable;
 
     /**
-     * The Google account being used by GSA according to the latest update we have received.
-     * This may be null.
+     * The Google account email address being used by GSA according to the latest update we have
+     * received.
      */
-    private String mGsaAccount;
+    private @Nullable String mGsaAccount;
 
     /**
      * Returns the singleton instance of GSAState and creates one if necessary.
@@ -110,6 +118,10 @@ public class GSAState {
      */
     public void setGsaAccount(String gsaAccount) {
         mGsaAccount = gsaAccount;
+
+        for (Observer observer : mObserverList) {
+            observer.onSetGsaAccount();
+        }
     }
 
     /**
@@ -276,4 +288,28 @@ public class GSAState {
 
         return Boolean.parseBoolean(cursor.getString(0));
     }
+
+    /**
+     * Adds an observer.
+     * @param observer The observer to add.
+     */
+    public void addObserver(@NonNull Observer observer) {
+        mObserverList.addObserver(observer);
+    }
+
+    /**
+     * Removes an observer.
+     * @param observer The observer to remove.
+     */
+    public void removeObserver(@NonNull Observer observer) {
+        mObserverList.removeObserver(observer);
+    }
+
+    /**
+     * Sets an instance for testing.
+     * @param gsaState The instance to set for testing.
+     */
+    public static void setInstanceForTesting(GSAState gsaState) {
+        sGSAState = gsaState;
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java

@@ -5,8 +5,6 @@
 package org.chromium.chrome.browser.omnibox;
 
 import android.content.Context;
-import android.content.res.ColorStateList;
-import android.graphics.drawable.Drawable;
 import android.os.Parcelable;
 import android.text.TextUtils;
 import android.util.AttributeSet;
@@ -169,6 +167,8 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
         mLocationBarDataProvider = locationBarDataProvider;
         mVoiceRecognitionHandler = voiceRecognitionHandler;
         mAssistantVoiceSearchServiceSupplier = assistantVoiceSearchSupplier;
+        mAssistantVoiceSearchServiceSupplier.onAvailable(
+                (assistantVoiceSearchService) -> onAssistantVoiceSearchServiceChanged());
 
         updateButtonVisibility();
         updateShouldAnimateIconChanges();
@@ -315,7 +315,7 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
     /** Updates visuals after the primary color has changed. */
     @CallSuper
     public void onPrimaryColorChanged() {
-        updateAssistantVoiceSearchColors();
+        updateAssistantVoiceSearchDrawableAndColors();
         updateUseDarkColors();
     }
 
@@ -369,27 +369,18 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
     }
 
     /* package */ void onAssistantVoiceSearchServiceChanged() {
-        AssistantVoiceSearchService assistantVoiceSearchService =
-                mAssistantVoiceSearchServiceSupplier.get();
-        assert assistantVoiceSearchService != null;
-        Drawable drawable = assistantVoiceSearchService.getCurrentMicDrawable();
-        mMicButton.setImageDrawable(drawable);
-        updateAssistantVoiceSearchColors();
+        updateAssistantVoiceSearchDrawableAndColors();
     }
 
-    private void updateAssistantVoiceSearchColors() {
+    private void updateAssistantVoiceSearchDrawableAndColors() {
         AssistantVoiceSearchService assistantVoiceSearchService =
                 mAssistantVoiceSearchServiceSupplier.get();
-        ColorStateList colorStateList;
-        // This will be called between inflation and initialization. For those calls, using a null
-        // ColorStateList should have no visible impact to the user.
-        if (assistantVoiceSearchService == null) {
-            colorStateList = null;
-        } else {
-            colorStateList = assistantVoiceSearchService.getMicButtonColorStateList(
-                    getPrimaryBackgroundColor(), getContext());
-        }
-        ApiCompatibilityUtils.setImageTintList(mMicButton, colorStateList);
+        if (assistantVoiceSearchService == null) return;
+
+        ApiCompatibilityUtils.setImageTintList(mMicButton,
+                assistantVoiceSearchService.getMicButtonColorStateList(
+                        getPrimaryBackgroundColor(), getContext()));
+        mMicButton.setImageDrawable(assistantVoiceSearchService.getCurrentMicDrawable());
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/omnibox/voice/AssistantVoiceSearchService.java

@@ -30,6 +30,8 @@ import org.chromium.chrome.browser.IntentHandler;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.gsa.GSAState;
 import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.profiles.ProfileManager;
 import org.chromium.components.browser_ui.styles.ChromeColors;
 import org.chromium.components.externalauth.ExternalAuthUtils;
 import org.chromium.components.search_engines.TemplateUrlService;
@@ -42,7 +44,8 @@ import java.lang.annotation.RetentionPolicy;
  * Service for state tracking and event delivery to classes that need to observe the state
  * of Assistant Voice Search.
  **/
-public class AssistantVoiceSearchService implements TemplateUrlService.TemplateUrlServiceObserver {
+public class AssistantVoiceSearchService implements TemplateUrlService.TemplateUrlServiceObserver,
+                                                    GSAState.Observer, ProfileManager.Observer {
     private static final String USER_ELIGIBILITY_HISTOGRAM =
             "Assistant.VoiceSearch.UserEligibility";
     @VisibleForTesting
@@ -114,22 +117,16 @@ public class AssistantVoiceSearchService implements TemplateUrlService.TemplateU
         mSharedPrefsManager = sharedPrefsManager;
         mObserver = observer;
 
+        ProfileManager.addObserver(this);
+        mGsaState.addObserver(this);
         mTemplateUrlService.addObserver(this);
         initializeAssistantVoiceSearchState();
     }
 
     public void destroy() {
         mTemplateUrlService.removeObserver(this);
-    }
-
-    @Override
-    public void onTemplateURLServiceChanged() {
-        boolean searchEngineGoogle = mTemplateUrlService.isDefaultSearchEngineGoogle();
-        if (mIsDefaultSearchEngineGoogle == searchEngineGoogle) return;
-
-        mIsDefaultSearchEngineGoogle = searchEngineGoogle;
-        mShouldShowColorfulMic = isColorfulMicEnabled();
-        notifyObserver();
+        mGsaState.removeObserver(this);
+        ProfileManager.removeObserver(this);
     }
 
     private void notifyObserver() {
@@ -202,6 +199,11 @@ public class AssistantVoiceSearchService implements TemplateUrlService.TemplateU
         return AppCompatResources.getColorStateList(context, id);
     }
 
+    /** Called from {@link VoiceRecognitionHandler} after the consent flow has completed. */
+    public void onAssistantConsentDialogComplete(boolean useAssistant) {
+        if (useAssistant) updateColorfulMicState();
+    }
+
     /**
      * @return Whether the user has enabled the feature, ensure {@link needsEnabledCheck} is
      *         called first.
@@ -222,6 +224,7 @@ public class AssistantVoiceSearchService implements TemplateUrlService.TemplateU
             sAgsaSupportsAssistantVoiceSearch =
                     mSharedPrefsManager.readBoolean(ASSISTANT_VOICE_SEARCH_SUPPORTED,
                             /* default= */ false);
+            updateColorfulMicState();
         } else {
             DeferredStartupHandler.getInstance().addDeferredTask(() -> {
                 // Only do this once per browser start.
@@ -240,6 +243,7 @@ public class AssistantVoiceSearchService implements TemplateUrlService.TemplateU
                                 sAgsaSupportsAssistantVoiceSearch);
                         mSharedPrefsManager.writeString(
                                 ASSISTANT_LAST_VERSION, currentAgsaVersion);
+                        updateColorfulMicState();
                     }
                 }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
             });
@@ -326,14 +330,61 @@ public class AssistantVoiceSearchService implements TemplateUrlService.TemplateU
                 USER_ELIGIBILITY_HISTOGRAM, canRequestAssistantVoiceSearch());
     }
 
+    private void updateColorfulMicState() {
+        final boolean shouldShowColorfulMic = isColorfulMicEnabled();
+        // Execute the update/notification in an AsyncTask to prevent re-entrant calls.
+        new AsyncTask<Boolean>() {
+            @Override
+            protected Boolean doInBackground() {
+                return mShouldShowColorfulMic != shouldShowColorfulMic;
+            }
+            @Override
+            protected void onPostExecute(Boolean notify) {
+                mShouldShowColorfulMic = shouldShowColorfulMic;
+                if (notify) notifyObserver();
+            }
+        }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
+    }
+
+    // TemplateUrlService.TemplateUrlServiceObserver implementation
+
+    @Override
+    public void onTemplateURLServiceChanged() {
+        boolean searchEngineGoogle = mTemplateUrlService.isDefaultSearchEngineGoogle();
+        if (mIsDefaultSearchEngineGoogle == searchEngineGoogle) return;
+
+        mIsDefaultSearchEngineGoogle = searchEngineGoogle;
+        mShouldShowColorfulMic = isColorfulMicEnabled();
+        notifyObserver();
+    }
+
+    // GSAState.Observer implementation
+
+    @Override
+    public void onSetGsaAccount() {
+        updateColorfulMicState();
+    }
+
+    // ProfileManager.Observer implementation
+
+    @Override
+    public void onProfileAdded(Profile profile) {
+        updateColorfulMicState();
+    }
+
+    @Override
+    public void onProfileDestroyed(Profile profile) {}
+
+    // Test-only methods
+
     /** Enable the colorful mic for testing purposes. */
     void setColorfulMicEnabledForTesting(boolean enabled) {
         mIsColorfulMicEnabled = enabled;
         mShouldShowColorfulMic = enabled;
     }
 
-    // Allows testing the NULL case for Boolean.
-    static void setAgsaSupportsAssistantVoiceSearchForTesting(Boolean value) {
+    /** Allows skipping the cross-app check for testing. */
+    public static void setAgsaSupportsAssistantVoiceSearchForTesting(Boolean value) {
         sAgsaSupportsAssistantVoiceSearch = value;
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/voice/VoiceRecognitionHandler.java

@@ -590,6 +590,9 @@ public class VoiceRecognitionHandler {
             AssistantVoiceSearchConsentUi.show(windowAndroid,
                     SharedPreferencesManager.getInstance(), new SettingsLauncherImpl(),
                     (useAssistant) -> {
+                        // Notify the service about the consent completion.
+                        assistantVoiceSearchService.onAssistantConsentDialogComplete(useAssistant);
+
                         if (useAssistant) {
                             if (!startAGSAForAssistantVoiceSearch(
                                         activity, windowAndroid, source)) {

chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/voice/AssistantVoiceSearchServiceRenderTest.java

+// Copyright 2021 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.chrome.browser.omnibox.voice;
+
+import static androidx.test.espresso.Espresso.onView;
+import static androidx.test.espresso.action.ViewActions.click;
+import static androidx.test.espresso.matcher.ViewMatchers.withId;
+
+import static org.mockito.ArgumentMatchers.anyObject;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+
+import static org.chromium.chrome.browser.preferences.ChromePreferenceKeys.ASSISTANT_VOICE_SEARCH_ENABLED;
+import static org.chromium.chrome.browser.preferences.ChromePreferenceKeys.ASSISTANT_VOICE_SEARCH_SUPPORTED;
+
+import android.support.test.filters.MediumTest;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.flags.ChromeSwitches;
+import org.chromium.chrome.browser.gsa.GSAState;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
+import org.chromium.chrome.test.util.ChromeRenderTestRule;
+import org.chromium.components.embedder_support.util.UrlConstants;
+import org.chromium.components.externalauth.ExternalAuthUtils;
+
+import java.io.IOException;
+
+/** Tests for AssistantVoiceSearchService */
+@RunWith(ChromeJUnit4ClassRunner.class)
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
+        "enable-features=" + ChromeFeatureList.OMNIBOX_ASSISTANT_VOICE_SEARCH + "<Study",
+        "force-fieldtrials=Study/Group"})
+public class AssistantVoiceSearchServiceRenderTest {
+    @Rule
+    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
+    @Rule
+    public ChromeRenderTestRule mRenderTestRule =
+            ChromeRenderTestRule.Builder.withPublicCorpus().build();
+    @Rule
+    public MockitoRule mMockitoRule = MockitoJUnit.rule();
+
+    @Mock
+    GSAState mGsaState;
+
+    @Before
+    public void setUp() {
+        AssistantVoiceSearchService.setAgsaSupportsAssistantVoiceSearchForTesting(true);
+        SharedPreferencesManager.getInstance().writeBoolean(ASSISTANT_VOICE_SEARCH_ENABLED, true);
+        SharedPreferencesManager.getInstance().writeBoolean(ASSISTANT_VOICE_SEARCH_SUPPORTED, true);
+
+        GSAState gsaState = Mockito.mock(GSAState.class);
+        doReturn(false).when(gsaState).isAgsaVersionBelowMinimum(anyString(), anyString());
+        doReturn(true).when(gsaState).doesGsaAccountMatchChrome();
+        doReturn(true).when(gsaState).canAgsaHandleIntent(anyObject());
+        GSAState.setInstanceForTesting(gsaState);
+
+        ExternalAuthUtils externalAuthUtils = Mockito.mock(ExternalAuthUtils.class);
+        doReturn(true).when(externalAuthUtils).isGoogleSigned(anyString());
+        doReturn(true).when(externalAuthUtils).isChromeGoogleSigned();
+        ExternalAuthUtils.setInstanceForTesting(externalAuthUtils);
+
+        mActivityTestRule.startMainActivityOnBlankPage();
+    }
+
+    @Test
+    @MediumTest
+    @CommandLineFlags.Add({"force-fieldtrial-params=Study.Group:colorful_mic/true"})
+    @Feature({"RenderTest"})
+    public void testAssistantColorfulMic() throws IOException {
+        mActivityTestRule.loadUrlInNewTab(UrlConstants.NTP_URL, /* incognito= */ false);
+
+        mRenderTestRule.render(mActivityTestRule.getActivity().findViewById(R.id.ntp_content),
+                "avs_colorful_mic_unfocused_ntp");
+
+        onView(withId(R.id.search_box)).perform(click());
+        mRenderTestRule.render(mActivityTestRule.getActivity().findViewById(R.id.toolbar),
+                "avs_colorful_mic_focused");
+    }
+
+    @Test
+    @MediumTest
+    @CommandLineFlags.Add({"force-fieldtrial-params=Study.Group:colorful_mic/false"})
+    @Feature({"RenderTest"})
+    public void testAssistantMic() throws IOException {
+        mActivityTestRule.loadUrlInNewTab(UrlConstants.NTP_URL, /* incognito= */ false);
+
+        mRenderTestRule.render(mActivityTestRule.getActivity().findViewById(R.id.ntp_content),
+                "avs__mic_unfocused_ntp");
+
+        onView(withId(R.id.search_box)).perform(click());
+        mRenderTestRule.render(
+                mActivityTestRule.getActivity().findViewById(R.id.toolbar), "avs_mic_focused");
+    }
+}
\ No newline at end of file

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/voice/AssistantVoiceSearchServiceUnitTest.java

@@ -187,6 +187,25 @@ public class AssistantVoiceSearchServiceUnitTest {
                         AssistantVoiceSearchService.EligibilityFailureReason.ACCOUNT_MISMATCH));
     }
 
+    @Test
+    @Feature("OmniboxAssistantVoiceSearch")
+    public void testStartVoiceRecognition_StartsAssistantVoiceSearch_TemporaryAccountMismatch() {
+        doReturn(false).when(mGsaState).doesGsaAccountMatchChrome();
+
+        Assert.assertFalse(mAssistantVoiceSearchService.shouldRequestAssistantVoiceSearch());
+        Assert.assertEquals(1,
+                ShadowRecordHistogram.getHistogramValueCountForTesting(
+                        AssistantVoiceSearchService.USER_ELIGIBILITY_FAILURE_REASON_HISTOGRAM,
+                        AssistantVoiceSearchService.EligibilityFailureReason.ACCOUNT_MISMATCH));
+
+        doReturn(true).when(mGsaState).doesGsaAccountMatchChrome();
+        Assert.assertTrue(mAssistantVoiceSearchService.shouldRequestAssistantVoiceSearch());
+        Assert.assertEquals(1,
+                ShadowRecordHistogram.getHistogramValueCountForTesting(
+                        AssistantVoiceSearchService.USER_ELIGIBILITY_FAILURE_REASON_HISTOGRAM,
+                        AssistantVoiceSearchService.EligibilityFailureReason.ACCOUNT_MISMATCH));
+    }
+
     @Test
     @Feature("OmniboxAssistantVoiceSearch")
     public void testAssistantEligibility_VersionTooLow() {

components/externalauth/android/java/src/org/chromium/components/externalauth/ExternalAuthUtils.java

@@ -286,4 +286,12 @@ public class ExternalAuthUtils {
     protected String describeError(final int errorCode) {
         return GoogleApiAvailability.getInstance().getErrorString(errorCode);
     }
+
+    /**
+     * Sets an instance for testing.
+     * @param externalAuthUtils The instance to set for testing.
+     */
+    public static void setInstanceForTesting(ExternalAuthUtils externalAuthUtils) {
+        sInstance = externalAuthUtils;
+    }
 }