Skip to content

Commit

Permalink
Follow-up changes for Camera assisted search with Google Lens on Omni…
Browse files Browse the repository at this point in the history
…box and NTP

- Check and set Lens button visibility on the NTP first load
- Add action reporting for the Lens button clicks.
- Add a UMA histogram for LensSupportStatus for omnibox entry points.
- Add a UMA histogram for 'time spent in Lens'.
- Add unit tests for the Lens button (TasksViewBinderTest, LocationBarTest LocationBarLayoutTest)

Change-Id: Ib9a4d5ecaa31a1b6fbd8b35c230f001eb1a8ef8f
Bug: 1183382
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2726098
Commit-Queue: Yu Su <yusuyoutube@google.com>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Ben Goldberger <benwgold@google.com>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859161}
  • Loading branch information
Yu Su authored and Chromium LUCI CQ committed Mar 2, 2021
1 parent d20a5bf commit b9cac71
Show file tree
Hide file tree
Showing 16 changed files with 341 additions and 23 deletions.
Expand Up @@ -352,6 +352,8 @@ void initWithNative(@Nullable FakeboxDelegate fakeboxDelegate,
// Note that isVoiceSearchEnabled will return false in incognito mode.
mPropertyModel.set(IS_VOICE_RECOGNITION_BUTTON_VISIBLE,
mFakeboxDelegate.getVoiceRecognitionHandler().isVoiceSearchEnabled());
mPropertyModel.set(IS_LENS_BUTTON_VISIBLE,
mFakeboxDelegate.isLensEnabled(LensEntryPoint.TASKS_SURFACE));

if (mController.overviewVisible()) {
mFakeboxDelegate.addUrlFocusChangeListener(mUrlFocusChangeListener);
Expand Down Expand Up @@ -397,6 +399,7 @@ void setSecondaryTasksSurfacePropertyModel(PropertyModel propertyModel) {
// tiles and voice recognition button should be invisible.
mSecondaryTasksSurfacePropertyModel.set(MV_TILES_VISIBLE, false);
mSecondaryTasksSurfacePropertyModel.set(IS_VOICE_RECOGNITION_BUTTON_VISIBLE, false);
mSecondaryTasksSurfacePropertyModel.set(IS_LENS_BUTTON_VISIBLE, false);
}

void addStateChangeObserver(StartSurface.StateObserver observer) {
Expand Down Expand Up @@ -919,9 +922,11 @@ private void setFakeBoxVisibility(boolean isVisible) {
// earlier than the VoiceRecognitionHandler, so isVoiceSearchEnabled returns
// incorrect state if check synchronously.
ThreadUtils.postOnUiThread(() -> {
if (mFakeboxDelegate != null && mFakeboxDelegate.getVoiceRecognitionHandler() != null) {
mPropertyModel.set(IS_VOICE_RECOGNITION_BUTTON_VISIBLE,
mFakeboxDelegate.getVoiceRecognitionHandler().isVoiceSearchEnabled());
if (mFakeboxDelegate != null) {
if (mFakeboxDelegate.getVoiceRecognitionHandler() != null) {
mPropertyModel.set(IS_VOICE_RECOGNITION_BUTTON_VISIBLE,
mFakeboxDelegate.getVoiceRecognitionHandler().isVoiceSearchEnabled());
}
mPropertyModel.set(IS_LENS_BUTTON_VISIBLE,
mFakeboxDelegate.isLensEnabled(LensEntryPoint.TASKS_SURFACE));
}
Expand Down
Expand Up @@ -110,8 +110,8 @@ public void onClick(View v) {
mModel.set(LENS_BUTTON_CLICK_LISTENER, new View.OnClickListener() {
@Override
public void onClick(View v) {
// TODO(b/181067692): Report user action for this click.
mFakeboxDelegate.startLens(LensEntryPoint.TASKS_SURFACE);
RecordUserAction.record("TasksSurface.FakeBox.Lens");
}
});

Expand Down
Expand Up @@ -21,8 +21,10 @@
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGNITO;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGNITO_DESCRIPTION_INITIALIZED;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGNITO_DESCRIPTION_VISIBLE;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_LENS_BUTTON_VISIBLE;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL_VISIBLE;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_VOICE_RECOGNITION_BUTTON_VISIBLE;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.LENS_BUTTON_CLICK_LISTENER;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MORE_TABS_CLICK_LISTENER;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_CONTAINER_TOP_MARGIN;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_VISIBLE;
Expand Down Expand Up @@ -179,6 +181,29 @@ public void testSetVoiceSearchButtonVisibilityAndClickListener() {
assertFalse(isViewVisible(R.id.voice_search_button));
}

@Test
@SmallTest
public void testSetLensButtonVisibilityAndClickListener() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mTasksViewPropertyModel.set(IS_FAKE_SEARCH_BOX_VISIBLE, true);
mTasksViewPropertyModel.set(IS_LENS_BUTTON_VISIBLE, true);
});
assertTrue(isViewVisible(R.id.lens_camera_button));

mViewClicked.set(false);
onView(withId(R.id.lens_camera_button)).perform(click());
assertFalse(mViewClicked.get());
TestThreadUtils.runOnUiThreadBlocking(() -> {
mTasksViewPropertyModel.set(LENS_BUTTON_CLICK_LISTENER, mViewOnClickListener);
});
onView(withId(R.id.lens_camera_button)).perform(click());
assertTrue(mViewClicked.get());

TestThreadUtils.runOnUiThreadBlocking(
() -> mTasksViewPropertyModel.set(IS_LENS_BUTTON_VISIBLE, false));
assertFalse(isViewVisible(R.id.lens_camera_button));
}

@Test
@UiThreadTest
@SmallTest
Expand Down
Expand Up @@ -31,6 +31,14 @@ public static LensController getInstance() {
return sInstance;
}

/**
* Set the Lens Controller instance for testing.
* @param lensController the instance to be set.
*/
public static void setInstanceForTesting(LensController lensController) {
sInstance = lensController;
}

/**
* Whether the Lens SDK is available.
* @return Whether the Lens SDK is available.
Expand Down
Expand Up @@ -15,16 +15,15 @@
* Static utility methods to support UMA logging for Lens entry points.
*/
public class LensUma {
// Note: these values must match the ContextMenuLensSupportStatus enum in enums.xml.
// Note: these values must match the LensSupportStatus enum in enums.xml.
// Only add new values at the end, right before NUM_ENTRIES.
@IntDef({LensSupportStatus.LENS_SEARCH_SUPPORTED, LensSupportStatus.NON_GOOGLE_SEARCH_ENGINE,
LensSupportStatus.ACTIVITY_NOT_ACCESSIBLE, LensSupportStatus.OUT_OF_DATE,
LensSupportStatus.SEARCH_BY_IMAGE_UNAVAILABLE, LensSupportStatus.LEGACY_OS,
LensSupportStatus.INVALID_PACKAGE, LensSupportStatus.LENS_SHOP_SUPPORTED,
LensSupportStatus.LENS_SHOP_AND_SEARCH_SUPPORTED,
LensSupportStatus.CAMERA_NOT_AVAILABLE, LensSupportStatus.LOW_END_DEVICE,
LensSupportStatus.LENS_CAMERA_ASSISTED_SEARCH_SUPPORTED,
LensSupportStatus.LENS_TRANSLATE_SUPPORTED})
LensSupportStatus.CAMERA_NOT_AVAILABLE, LensSupportStatus.DISABLED_ON_LOW_END_DEVICE,
LensSupportStatus.AGSA_VERSION_NOT_SUPPORTED, LensSupportStatus.DISABLED_ON_INCOGNITO})
@Retention(RetentionPolicy.SOURCE)
public static @interface LensSupportStatus {
int LENS_SEARCH_SUPPORTED = 0;
Expand All @@ -37,9 +36,9 @@ public class LensUma {
int LENS_SHOP_SUPPORTED = 7;
int LENS_SHOP_AND_SEARCH_SUPPORTED = 8;
int CAMERA_NOT_AVAILABLE = 9;
int LOW_END_DEVICE = 10;
int LENS_CAMERA_ASSISTED_SEARCH_SUPPORTED = 11;
int LENS_TRANSLATE_SUPPORTED = 12;
int DISABLED_ON_LOW_END_DEVICE = 10;
int AGSA_VERSION_NOT_SUPPORTED = 11;
int DISABLED_ON_INCOGNITO = 12;
int NUM_ENTRIES = 13;
}

Expand All @@ -48,4 +47,32 @@ public static void recordLensSupportStatus(
RecordHistogram.recordEnumeratedHistogram(
histogramName, reason, LensSupportStatus.NUM_ENTRIES);
}

public static String getSupportStatusHistogramNameByEntryPoint(
@LensEntryPoint int lensEntryPoint) {
String histogramName = null;
switch (lensEntryPoint) {
case LensEntryPoint.NEW_TAB_PAGE:
histogramName = "NewTabPage.LensSupportStatus";
break;
case LensEntryPoint.OMNIBOX:
histogramName = "Omnibox.LensSupportStatus";
break;
case LensEntryPoint.TASKS_SURFACE:
histogramName = "NewTabPage.TasksSurface.LensSupportStatus";
break;
case LensEntryPoint.CONTEXT_MENU_SEARCH_MENU_ITEM:
case LensEntryPoint.CONTEXT_MENU_SHOP_MENU_ITEM:
histogramName = "ContextMenu.LensSupportStatus";
break;
case LensEntryPoint.CONTEXT_MENU_CHIP:
default:
assert false : "Method not implemented.";
}
return histogramName;
}

public static void recordTimeSpentInLens(String histogramName, long timeSpentInLensMs) {
RecordHistogram.recordLongTimesHistogram(histogramName, timeSpentInLensMs);
}
}
Expand Up @@ -27,6 +27,7 @@
import org.chromium.base.CallbackController;
import org.chromium.base.MathUtils;
import org.chromium.base.TraceEvent;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.app.video_tutorials.NewTabPageVideoIPHManager;
Expand Down Expand Up @@ -354,9 +355,12 @@ private void initializeVoiceSearchButton() {

private void initializeLensButton() {
TraceEvent.begin(TAG + ".initializeLensButton()");
updateLensButtonVisibility();
// TODO(b/181067692): Report user action for this click.
mSearchBoxCoordinator.addLensButtonClickListener(
v -> mSearchBoxCoordinator.startLens(LensEntryPoint.NEW_TAB_PAGE));
mSearchBoxCoordinator.addLensButtonClickListener(v -> {
mSearchBoxCoordinator.startLens(LensEntryPoint.NEW_TAB_PAGE);
RecordUserAction.record("NewTabPage.SearchBox.Lens");
});
if (SearchEngineLogoUtils.getInstance().isSearchEngineLogoEnabled()) {
// View is 48dp, image is 24dp. Increasing the padding from 4dp -> 8dp will split the
// remaining 16dp evenly between start/end resulting in a paddingEnd of 8dp.
Expand Down
Expand Up @@ -19,6 +19,7 @@
import org.chromium.base.supplier.OneshotSupplierImpl;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.lens.LensController;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.Destroyable;
import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
Expand Down Expand Up @@ -134,7 +135,8 @@ public LocationBarCoordinator(View locationBarLayout, View autocompleteAnchorVie
mLocationBarLayout, locationBarDataProvider, profileObservableSupplier,
PrivacyPreferencesManagerImpl.getInstance(), overrideUrlLoadingDelegate,
LocaleManager.getInstance(), mTemplateUrlServiceSupplier, backKeyBehavior,
windowAndroid, isTablet() && isTabletLayout(), searchEngineLogoUtils);
windowAndroid, isTablet() && isTabletLayout(), searchEngineLogoUtils,
LensController.getInstance());
mUrlCoordinator =
new UrlBarCoordinator((UrlBar) mUrlBar, windowDelegate, actionModeCallback,
mCallbackController.makeCancelable(mLocationBarMediator::onUrlFocusChange),
Expand Down Expand Up @@ -571,6 +573,10 @@ public void onUrlChangedForTesting() {
mLocationBarMediator.onUrlChanged();
}

public void setLensControllerForTesting(LensController lensController) {
mLocationBarMediator.setLensControllerForTesting(lensController);
}

private boolean isPhoneLayout() {
return mLocationBarLayout instanceof LocationBarPhone;
}
Expand Down
Expand Up @@ -154,6 +154,7 @@ public void set(LocationBarMediator object, Float value) {
private boolean mShouldShowButtonsWhenUnfocused;
private float mUrlFocusChangeFraction;
private boolean mUrlHasFocus;
private LensController mLensController;

/*package */ LocationBarMediator(@NonNull Context context,
@NonNull LocationBarLayout locationBarLayout,
Expand All @@ -164,7 +165,8 @@ public void set(LocationBarMediator object, Float value) {
@NonNull LocaleManager localeManager,
@NonNull OneshotSupplier<TemplateUrlService> templateUrlServiceSupplier,
@NonNull BackKeyBehaviorDelegate backKeyBehavior, @NonNull WindowAndroid windowAndroid,
boolean isTablet, @NonNull SearchEngineLogoUtils searchEngineLogoUtils) {
boolean isTablet, @NonNull SearchEngineLogoUtils searchEngineLogoUtils,
@NonNull LensController lensController) {
mContext = context;
mLocationBarLayout = locationBarLayout;
mLocationBarDataProvider = locationBarDataProvider;
Expand All @@ -183,6 +185,7 @@ public void set(LocationBarMediator object, Float value) {
mIsTablet = isTablet;
mSearchEngineLogoUtils = searchEngineLogoUtils;
mShouldShowButtonsWhenUnfocused = isTablet;
mLensController = lensController;
}

/**
Expand Down Expand Up @@ -328,6 +331,10 @@ public void set(LocationBarMediator object, Float value) {
onAssistantVoiceSearchServiceChanged();
}

/* package */ void setLensControllerForTesting(LensController lensController) {
mLensController = lensController;
}

/* package */ OneshotSupplier<AssistantVoiceSearchService>
getAssistantVoiceSearchServiceSupplierForTesting() {
return mAssistantVoiceSearchServiceSupplier;
Expand Down Expand Up @@ -1362,15 +1369,15 @@ public void onTemplateURLServiceChanged() {

@Override
public boolean isLensEnabled(@LensEntryPoint int lensEntryPoint) {
return LensController.getInstance().isLensEnabled(
return mLensController.isLensEnabled(
new LensQueryParams.Builder(lensEntryPoint, mLocationBarDataProvider.isIncognito())
.build());
}

@Override
public void startLens(@LensEntryPoint int lensEntryPoint) {
// TODO(b/181067692): Report user action for this click.
LensController.getInstance().startLens(mWindowAndroid,
mLensController.startLens(mWindowAndroid,
new LensIntentParams.Builder(lensEntryPoint, mLocationBarDataProvider.isIncognito())
.build());
}
Expand Down
Expand Up @@ -12,8 +12,10 @@
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withId;

import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doReturn;

Expand Down Expand Up @@ -46,11 +48,14 @@
import org.chromium.chrome.R;
import org.chromium.chrome.browser.dom_distiller.DomDistillerTabUtils;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.lens.LensController;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.toolbar.LocationBarModel;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.OmniboxTestUtils;
import org.chromium.chrome.test.util.ViewUtils;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.security_state.ConnectionSecurityLevel;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.test.util.ClickUtils;
Expand Down Expand Up @@ -80,6 +85,8 @@ public class LocationBarLayoutTest {
AndroidPermissionDelegate mAndroidPermissionDelegate;
@Mock
SearchEngineLogoUtils mSearchEngineLogoUtils;
@Mock
LensController mLensController;

private TestLocationBarModel mTestLocationBarModel;

Expand Down Expand Up @@ -140,6 +147,7 @@ public UrlBarData getUrlBarData() {

@Before
public void setUp() throws InterruptedException {
LensController.setInstanceForTesting(mLensController);
mActivityTestRule.startMainActivityOnBlankPage();
setupModelsForCurrentTab();

Expand Down Expand Up @@ -190,6 +198,10 @@ private ImageButton getMicButton() {
return mActivityTestRule.getActivity().findViewById(R.id.mic_button);
}

private ImageButton getLensButton() {
return mActivityTestRule.getActivity().findViewById(R.id.lens_camera_button);
}

private View getStatusIconView() {
return mActivityTestRule.getActivity().findViewById(R.id.location_bar_status_icon);
}
Expand Down Expand Up @@ -234,6 +246,65 @@ public void testShowingVoiceSearchButtonIfUrlBarIsEmpty() throws ExecutionExcept
onView(withId(R.id.delete_button)).check(matches(not(isDisplayed())));
}

@Test
@SmallTest
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public void testLensButtonVisibilityOnStartNtp_enabled() throws ExecutionException {
doReturn(true).when(mLensController).isLensEnabled(any());
loadUrlInNewTabAndUpdateModels(UrlConstants.NTP_URL, false);
ViewUtils.waitForView(allOf(withId(R.id.lens_camera_button), isDisplayed()));
}

@Test
@SmallTest
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public void testLensButtonVisibilityOnStartNtp_disabled() throws ExecutionException {
doReturn(false).when(mLensController).isLensEnabled(any());
loadUrlInNewTabAndUpdateModels(UrlConstants.NTP_URL, false);
ViewUtils.waitForView(allOf(withId(R.id.lens_camera_button), not(isDisplayed())));
}

@Test
@SmallTest
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public void testNotShowingLensButtonIfUrlBarContainsText() throws ExecutionException {
doReturn(true).when(mLensController).isLensEnabled(any());
loadUrlInNewTabAndUpdateModels("", false);
// When there is text, the delete button should be visible.
setUrlBarTextAndFocus("testing");
onView(withId(R.id.delete_button)).check(matches(isDisplayed()));
onView(withId(R.id.lens_camera_button)).check(matches(not(isDisplayed())));
}

@Test
@SmallTest
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public void testShowingLensButtonIfUrlBarIsEmpty() throws ExecutionException {
doReturn(true).when(mLensController).isLensEnabled(any());
loadUrlInNewTabAndUpdateModels("", false);
// When there's no text, the lens button should be visible.
setUrlBarTextAndFocus("");
onView(withId(R.id.delete_button)).check(matches(not(isDisplayed())));
onView(withId(R.id.lens_camera_button)).check(matches(isDisplayed()));
}

@Test
@SmallTest
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public void testShowingLensButtonAfterTextDeleted() throws ExecutionException {
doReturn(true).when(mLensController).isLensEnabled(any());
loadUrlInNewTabAndUpdateModels("", false);
// When there is text, the delete button should be visible.
setUrlBarTextAndFocus("testing");
onView(withId(R.id.delete_button)).check(matches(isDisplayed()));
onView(withId(R.id.lens_camera_button)).check(matches(not(isDisplayed())));
// When there's no text, the lens button should be visible.
ClickUtils.clickButton(getDeleteButton());
onView(withId(R.id.delete_button)).check(matches(not(isDisplayed())));
onView(withId(R.id.lens_camera_button)).check(matches(isDisplayed()));
Assert.assertEquals("", getUrlText(getUrlBar()));
}

@Test
@SmallTest
public void testDeleteButton() throws ExecutionException {
Expand Down

0 comments on commit b9cac71

Please sign in to comment.