Skip to content

Commit

Permalink
[Assistant] Rework eligibility reporting for voice search requests
Browse files Browse the repository at this point in the history
- Report eligibility/failure reasons only once per mic taps.
- Report all eligibility failure reasons instead of just one.
- Add Google default search engine status as an explicit condition.
- Remove deferred ContentProvider read and replaced it with a signed-in
  condition on the Chrome-side & sending the signed-in email over the
  intent.
- Add low-end device condition.
- Updating eligibility failure enum to deprecate values and updating
  histogram readme.

TBR=aliceywang@chromium.org

(cherry picked from commit 2ad1a22)

Bug: 1182496
Change-Id: Ie89ff06ca3a5dc56492c8ed3d0b0d6b75e1061ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2719828
Reviewed-by: Alice Wang <aliceywang@chromium.org>
Reviewed-by: Josh Simmons <jds@google.com>
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#860281}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2747477
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/4430@{#322}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}
  • Loading branch information
Brandon Wylie authored and Chromium LUCI CQ committed Mar 10, 2021
1 parent ccd8de7 commit 14c0344
Show file tree
Hide file tree
Showing 14 changed files with 293 additions and 318 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
import org.chromium.chrome.browser.omnibox.voice.AssistantVoiceSearchService;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
import org.chromium.components.browser_ui.styles.ChromeColors;
import org.chromium.components.externalauth.ExternalAuthUtils;
import org.chromium.ui.base.ViewUtils;
Expand Down Expand Up @@ -88,7 +90,9 @@ public void destroy() {
public void onFinishNativeInitialization() {
mAssistantVoiceSearchService = new AssistantVoiceSearchService(mContext,
ExternalAuthUtils.getInstance(), TemplateUrlServiceFactory.get(),
GSAState.getInstance(mContext), this, SharedPreferencesManager.getInstance());
GSAState.getInstance(mContext), this, SharedPreferencesManager.getInstance(),
IdentityServicesProvider.get().getIdentityManager(
Profile.getLastUsedRegularProfile()));
onAssistantVoiceSearchServiceChanged();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,6 @@ include_rules = [
"+components/omnibox/browser/android/java/src/org/chromium/components/omnibox/AutocompleteMatch.java",
"+components/omnibox/browser/android/java/src/org/chromium/components/omnibox/AutocompleteResult.java",
"+ui/android/java/src/org/chromium/ui/base",
"+components/signin/public/android",
"+chrome/browser/signin/services/android",
]
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.privacy.settings.PrivacyPreferencesManagerImpl;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.ui.native_page.NativePage;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
Expand Down Expand Up @@ -281,7 +282,9 @@ public void set(LocationBarMediator object, Float value) {
mOmniboxPrerender = new OmniboxPrerender();
mAssistantVoiceSearchServiceSupplier.set(new AssistantVoiceSearchService(mContext,
ExternalAuthUtils.getInstance(), mTemplateUrlServiceSupplier.get(),
GSAState.getInstance(mContext), this, SharedPreferencesManager.getInstance()));
GSAState.getInstance(mContext), this, SharedPreferencesManager.getInstance(),
IdentityServicesProvider.get().getIdentityManager(
Profile.getLastUsedRegularProfile())));
onAssistantVoiceSearchServiceChanged();
mLocationBarLayout.onFinishNativeInitialization();
setProfile(mProfileSupplier.get());
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ public class VoiceRecognitionHandler implements ProfileManager.Observer {
@VisibleForTesting
static final String EXTRA_INTENT_SENT_TIMESTAMP =
"com.android.chrome.voice.INTENT_SENT_TIMESTAMP";
/**
* Extra containing the email for the signed-in/syncing account on the device.
*/
@VisibleForTesting
static final String EXTRA_INTENT_USER_EMAIL = "com.android.chrome.voice.INTENT_USER_EMAIL";
/**
* Extra containing an integer that indicates which voice entrypoint the intent was initiated
* from.
Expand Down Expand Up @@ -653,7 +658,10 @@ public void startVoiceRecognition(@VoiceInteractionSource int source) {
}
}

if (startAGSAForAssistantVoiceSearch(activity, windowAndroid, source)) return;
if (mAssistantVoiceSearchServiceSupplier.hasValue()) {
mAssistantVoiceSearchServiceSupplier.get().reportUserEligibility();
if (startAGSAForAssistantVoiceSearch(activity, windowAndroid, source)) return;
}

if (!startSystemForVoiceSearch(activity, windowAndroid, source)) {
// TODO(wylieb): Emit histogram here to identify how many users are attempting to use
Expand All @@ -669,13 +677,13 @@ public void startVoiceRecognition(@VoiceInteractionSource int source) {
* and the result was a denial without an option to request them again, voice
* functionality will become unavailable.
*
* @param activity The current {@link Activity} that we're requesting permission for.
* @param windowAndroid Used to request audio permissions from the Android system.
* @param source The source of the mic button click, used to record metrics.
* @return Whether audio permissions are granted.
*/
@VisibleForTesting
boolean ensureAudioPermissionGranted(
WindowAndroid windowAndroid, @VoiceInteractionSource int source) {
private boolean ensureAudioPermissionGranted(
Activity activity, WindowAndroid windowAndroid, @VoiceInteractionSource int source) {
if (windowAndroid.hasPermission(Manifest.permission.RECORD_AUDIO)) return true;

// If we don't have permission and also can't ask, then there's no more work left other
Expand All @@ -691,7 +699,7 @@ boolean ensureAudioPermissionGranted(
}

if (grantResults[0] == PackageManager.PERMISSION_GRANTED) {
startVoiceRecognition(source);
startSystemForVoiceSearch(activity, windowAndroid, source);
} else if (!windowAndroid.canRequestPermission(Manifest.permission.RECORD_AUDIO)) {
notifyVoiceAvailabilityImpacted();
}
Expand All @@ -706,7 +714,7 @@ private boolean startSystemForVoiceSearch(
Activity activity, WindowAndroid windowAndroid, @VoiceInteractionSource int source) {
// Check if we need to request audio permissions. If we don't, then trigger a permissions
// prompt will appear and startVoiceRecognition will be called again.
if (!ensureAudioPermissionGranted(windowAndroid, source)) return false;
if (!ensureAudioPermissionGranted(activity, windowAndroid, source)) return false;

Intent intent = new Intent(RecognizerIntent.ACTION_RECOGNIZE_SPEECH);
intent.putExtra(
Expand Down Expand Up @@ -738,8 +746,11 @@ private boolean startAGSAForAssistantVoiceSearch(
mAssistantVoiceSearchServiceSupplier.get();
if (assistantVoiceSearchService == null) return false;

if (assistantVoiceSearchService.canRequestAssistantVoiceSearch()
&& assistantVoiceSearchService.needsEnabledCheck()) {
// Check if the device meets the requirements to use Assistant voice search.
if (!assistantVoiceSearchService.canRequestAssistantVoiceSearch()) return false;

// Check if the consent prompt needs to be shown.
if (assistantVoiceSearchService.needsEnabledCheck()) {
mDelegate.clearOmniboxFocus();
AssistantVoiceSearchConsentUi.show(windowAndroid,
SharedPreferencesManager.getInstance(), new SettingsLauncherImpl(),
Expand All @@ -761,14 +772,14 @@ private boolean startAGSAForAssistantVoiceSearch(
return true;
}

// Report the client's eligibility for Assistant voice search.
assistantVoiceSearchService.reportUserEligibility();
// Final check to see if Assistant voice search should be used.
if (!assistantVoiceSearchService.shouldRequestAssistantVoiceSearch()) return false;

Intent intent = assistantVoiceSearchService.getAssistantVoiceSearchIntent();
intent.putExtra(EXTRA_VOICE_ENTRYPOINT, source);
// Allows Assistant to track intent latency.
intent.putExtra(EXTRA_INTENT_SENT_TIMESTAMP, System.currentTimeMillis());
intent.putExtra(EXTRA_INTENT_USER_EMAIL, assistantVoiceSearchService.getUserEmail());

if (shouldAddPageUrl(source)) {
String url = getUrl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
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;

Expand Down Expand Up @@ -64,9 +63,7 @@ public class AssistantVoiceSearchServiceRenderTest {

@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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import static org.mockito.ArgumentMatchers.notNull;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

import android.app.Activity;
Expand Down Expand Up @@ -117,6 +118,7 @@ public void onSuggestionsReceived(

// The default Tab URL.
private static final String DEFAULT_URL = "https://example.com/";
private static final String DEFAULT_USER_EMAIL = "test@test.com";

/**
* An implementation of the real {@link VoiceRecognitionHandler} except instead of
Expand Down Expand Up @@ -555,6 +557,7 @@ public void setUp() throws InterruptedException, ExecutionException {
doReturn(false).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
doReturn(false).when(mAssistantVoiceSearchService).needsEnabledCheck();
doReturn(mIntent).when(mAssistantVoiceSearchService).getAssistantVoiceSearchIntent();
doReturn(DEFAULT_USER_EMAIL).when(mAssistantVoiceSearchService).getUserEmail();

doReturn(true).when(mTranslateBridgeWrapper).canManuallyTranslate(notNull());
doReturn("fr").when(mTranslateBridgeWrapper).getOriginalLanguage(notNull());
Expand Down Expand Up @@ -723,6 +726,7 @@ public void testStartVoiceRecognition_OnlyUpdateMicButtonStateIfCantRequestPermi
@Feature({"OmniboxAssistantVoiceSearch"})
@EnableFeatures({ChromeFeatureList.OMNIBOX_ASSISTANT_VOICE_SEARCH})
public void testStartVoiceRecognition_StartsAssistantVoiceSearch() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.OMNIBOX);

Expand All @@ -733,6 +737,23 @@ public void testStartVoiceRecognition_StartsAssistantVoiceSearch() {
eq(VoiceRecognitionHandler.EXTRA_INTENT_SENT_TIMESTAMP), anyLong());
verify(mIntent).putExtra(
VoiceRecognitionHandler.EXTRA_VOICE_ENTRYPOINT, VoiceInteractionSource.OMNIBOX);
verify(mIntent).putExtra(
VoiceRecognitionHandler.EXTRA_INTENT_USER_EMAIL, DEFAULT_USER_EMAIL);
}

@Test
@SmallTest
@Feature({"OmniboxAssistantVoiceSearch"})
@EnableFeatures({ChromeFeatureList.OMNIBOX_ASSISTANT_VOICE_SEARCH})
public void testStartVoiceRecognition_ShouldRequestConditionsFail() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(false).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.OMNIBOX);

verify(mAssistantVoiceSearchService).reportUserEligibility();
// We check for the consent dialog when canRequestAssistantVoiceSearch() is true.
verify(mAssistantVoiceSearchService).needsEnabledCheck();
verify(mAssistantVoiceSearchService, times(0)).getAssistantVoiceSearchIntent();
}

@Test
Expand All @@ -745,6 +766,7 @@ public void testStartVoiceRecognition_StartsAssistantVoiceSearch() {
+ VoiceRecognitionHandler.ASSISTANT_EXPERIMENT_ID_PARAM_NAME + "/test"})
public void
testStartVoiceRecognition_AssistantExperimentIdDisabled() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);

Expand All @@ -763,6 +785,7 @@ public void testStartVoiceRecognition_StartsAssistantVoiceSearch() {
+ VoiceRecognitionHandler.ASSISTANT_EXPERIMENT_ID_PARAM_NAME + "/test"})
public void
testStartVoiceRecognition_IncludeExperimentIdInAssistantIntent() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);

Expand All @@ -775,6 +798,7 @@ public void testStartVoiceRecognition_StartsAssistantVoiceSearch() {
@Feature("AssistantIntentPageUrl")
@EnableFeatures(ChromeFeatureList.ASSISTANT_INTENT_PAGE_URL)
public void testStartVoiceRecognition_ToolbarButtonIncludesPageUrl() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);

Expand All @@ -788,6 +812,7 @@ public void testStartVoiceRecognition_ToolbarButtonIncludesPageUrl() {
@Feature("AssistantIntentPageUrl")
@EnableFeatures(ChromeFeatureList.ASSISTANT_INTENT_PAGE_URL)
public void testStartVoiceRecognition_OmitPageUrlWhenAssistantVoiceSearchDisabled() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(false).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();

startVoiceRecognition(VoiceInteractionSource.TOOLBAR);
Expand All @@ -801,6 +826,7 @@ public void testStartVoiceRecognition_OmitPageUrlWhenAssistantVoiceSearchDisable
@Feature("AssistantIntentPageUrl")
@EnableFeatures(ChromeFeatureList.ASSISTANT_INTENT_PAGE_URL)
public void testStartVoiceRecognition_OmitPageUrlForNonToolbar() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();

startVoiceRecognition(VoiceInteractionSource.NTP);
Expand All @@ -813,6 +839,7 @@ public void testStartVoiceRecognition_OmitPageUrlForNonToolbar() {
@Feature("AssistantIntentPageUrl")
@EnableFeatures(ChromeFeatureList.ASSISTANT_INTENT_PAGE_URL)
public void testStartVoiceRecognition_OmitPageUrlForIncognito() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
doReturn(true).when(mTab).isIncognito();

Expand All @@ -826,6 +853,7 @@ public void testStartVoiceRecognition_OmitPageUrlForIncognito() {
@Feature("AssistantIntentPageUrl")
@EnableFeatures(ChromeFeatureList.ASSISTANT_INTENT_PAGE_URL)
public void testStartVoiceRecognition_OmitPageUrlForInternalPages() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
GURL url = new GURL("chrome://version");
doReturn(url).when(mTab).getUrl();
Expand Down Expand Up @@ -856,6 +884,7 @@ public void testStartVoiceRecognition_OmitPageUrlForNonHttp() {
ChromeFeatureList.ASSISTANT_INTENT_TRANSLATE_INFO})
public void
testStartVoiceRecognition_ToolbarButtonIncludesTranslateInfo() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);

Expand All @@ -873,6 +902,7 @@ public void testStartVoiceRecognition_OmitPageUrlForNonHttp() {
@EnableFeatures({ChromeFeatureList.OMNIBOX_ASSISTANT_VOICE_SEARCH})
@DisableFeatures({ChromeFeatureList.ASSISTANT_INTENT_TRANSLATE_INFO})
public void testStartVoiceRecognition_TranslateExtrasDisabled() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);

Expand All @@ -894,6 +924,7 @@ public void testStartVoiceRecognition_TranslateExtrasDisabled() {
ChromeFeatureList.ASSISTANT_INTENT_TRANSLATE_INFO})
public void
testStartVoiceRecognition_NoTranslateExtrasForNonToolbar() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
startVoiceRecognition(VoiceInteractionSource.OMNIBOX);

Expand All @@ -916,6 +947,7 @@ public void testStartVoiceRecognition_TranslateExtrasDisabled() {
ChromeFeatureList.ASSISTANT_INTENT_TRANSLATE_INFO})
public void
testStartVoiceRecognition_NoTranslateExtrasForNonTranslatePage() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
doReturn(false).when(mTranslateBridgeWrapper).canManuallyTranslate(notNull());
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);
Expand All @@ -939,6 +971,7 @@ public void testStartVoiceRecognition_TranslateExtrasDisabled() {
ChromeFeatureList.ASSISTANT_INTENT_TRANSLATE_INFO})
public void
testStartVoiceRecognition_NoTranslateExtrasWhenLanguagesUndetected() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
doReturn(null).when(mTranslateBridgeWrapper).getOriginalLanguage(notNull());
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);
Expand All @@ -962,6 +995,7 @@ public void testStartVoiceRecognition_TranslateExtrasDisabled() {
ChromeFeatureList.ASSISTANT_INTENT_TRANSLATE_INFO})
public void
testStartVoiceRecognition_TranslateInfoTargetLanguageOptional() {
doReturn(true).when(mAssistantVoiceSearchService).canRequestAssistantVoiceSearch();
doReturn(true).when(mAssistantVoiceSearchService).shouldRequestAssistantVoiceSearch();
doReturn(null).when(mTranslateBridgeWrapper).getTargetLanguage();
startVoiceRecognition(VoiceInteractionSource.TOOLBAR);
Expand Down
Loading

0 comments on commit 14c0344

Please sign in to comment.