Skip to content

Commit

Permalink
[Start] Build histograms for click rates of MV tiles on Start and NTP
Browse files Browse the repository at this point in the history
We created unique histograms to log click rates on each module on Start / NTP, and split by the surface hostname. These two histograms are named StartSurface.Module.Click and NewTabPage.Module.Click, with the modules including Omnibox, MV tiles, Single Tab Card, Feeds, and so on.
In this CL, we completed the two histograms by recording the clicking actions on MV tiles on Start/NTP and the tests for these 2 sub-histograms.
In order for readability, we also made a google sheet to summarize the behavior of different UMA histograms for clicking actions on MV tiles in Start Surface and NTP: https://docs.google.com/spreadsheets/d/1Ag2FGXfvPOccSC2PumnOg6R5exRfuxonrmRGaOlCUPU/edit#gid=0

Bug: 1384156
Change-Id: I263078726c146861d6d03b9c1245b46276073b11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4117337
Commit-Queue: Xinyi Ji <xinyiji@google.com>
Reviewed-by: Xi Han <hanxi@chromium.org>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Weilun Shi <sweilun@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1097463}
  • Loading branch information
Xinyi Ji authored and Chromium LUCI CQ committed Jan 26, 2023
1 parent 2904714 commit f74142c
Show file tree
Hide file tree
Showing 15 changed files with 429 additions and 17 deletions.
Expand Up @@ -46,6 +46,7 @@
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.share.ShareDelegate;
import org.chromium.chrome.browser.share.crow.CrowButtonDelegate;
import org.chromium.chrome.browser.suggestions.tile.TileGroupDelegateImpl;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
Expand Down Expand Up @@ -722,6 +723,11 @@ public boolean isMVTilesInitializedForTesting() {
return mTasksSurface.isMVTilesInitialized();
}

@VisibleForTesting
public TileGroupDelegateImpl getTileGroupDelegateForTesting() {
return mTasksSurface.getTileGroupDelegate();
}

/**
* Called only when Start Surface is enabled.
*/
Expand Down
Expand Up @@ -17,6 +17,7 @@
import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.feed.FeedReliabilityLogger;
import org.chromium.chrome.browser.omnibox.OmniboxStub;
import org.chromium.chrome.browser.suggestions.tile.TileGroupDelegateImpl;
import org.chromium.chrome.browser.tasks.tab_management.TabSwitcher;
import org.chromium.chrome.browser.tasks.tab_management.TabSwitcherCustomViewManager;

Expand Down Expand Up @@ -122,6 +123,10 @@ void updateFakeSearchBox(int height, int topMargin, int endPadding, float transl
/** Returns whether the MV tiles has been initialized. */
boolean isMVTilesInitialized();

@VisibleForTesting
/** Returns the tile group delegate. */
TileGroupDelegateImpl getTileGroupDelegate();

/**
* TODO(crbug.com/1315676): Remove this API after the bug is resolved.
*
Expand Down
Expand Up @@ -51,6 +51,7 @@
import org.chromium.chrome.browser.tasks.tab_management.TabSwitcherCustomViewManager;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.util.BrowserUiUtils;
import org.chromium.components.browser_ui.widget.MenuOrKeyboardActionController;
import org.chromium.components.browser_ui.widget.scrim.ScrimCoordinator;
import org.chromium.ui.base.DeviceFormFactor;
Expand Down Expand Up @@ -226,8 +227,8 @@ public void initializeMVTiles() {
new MostVisitedTileNavigationDelegate(mActivity, profile, mParentTabSupplier);
mSuggestionsUiDelegate =
new MostVisitedSuggestionsUiDelegate(navigationDelegate, profile, mSnackbarManager);
mTileGroupDelegate =
new TileGroupDelegateImpl(mActivity, profile, navigationDelegate, mSnackbarManager);
mTileGroupDelegate = new TileGroupDelegateImpl(mActivity, profile, navigationDelegate,
mSnackbarManager, BrowserUiUtils.HostSurface.START_SURFACE);

mMostVisitedCoordinator.initWithNative(
mSuggestionsUiDelegate, mTileGroupDelegate, enabled -> {});
Expand Down Expand Up @@ -330,6 +331,12 @@ public boolean isMVTilesInitialized() {
return mIsMVTilesInitialized;
}

@VisibleForTesting
@Override
public TileGroupDelegateImpl getTileGroupDelegate() {
return mTileGroupDelegate;
}

@Override
public @Nullable TabSwitcherCustomViewManager getTabSwitcherCustomViewManager() {
return (mTabSwitcher != null) ? mTabSwitcher.getTabSwitcherCustomViewManager() : null;
Expand Down
Expand Up @@ -4,28 +4,45 @@

package org.chromium.chrome.features.start_surface;

import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

import android.view.View;

import androidx.test.filters.MediumTest;
import androidx.test.filters.SmallTest;

import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.robolectric.annotation.Config;

import org.chromium.base.TimeUtils;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.UmaRecorder;
import org.chromium.base.metrics.UmaRecorderHolder;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.suggestions.SiteSuggestion;
import org.chromium.chrome.browser.suggestions.tile.Tile;
import org.chromium.chrome.browser.suggestions.tile.TileGroupDelegateImpl;
import org.chromium.chrome.browser.suggestions.tile.TileSectionType;
import org.chromium.chrome.browser.suggestions.tile.TileSource;
import org.chromium.chrome.browser.suggestions.tile.TileTitleSource;
import org.chromium.chrome.browser.tasks.ReturnToChromeUtil;
import org.chromium.chrome.browser.util.BrowserUiUtils;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.ui.mojom.WindowOpenDisposition;
import org.chromium.url.GURL;

/** Tests for {@link StartSurfaceCoordinator}. */
@RunWith(BaseRobolectricTestRunner.class)
Expand All @@ -36,6 +53,12 @@
public class StartSurfaceCoordinatorUnitTest {
private static final long MILLISECONDS_PER_MINUTE = TimeUtils.SECONDS_PER_MINUTE * 1000;
private static final String START_SURFACE_TIME_SPENT = "StartSurface.TimeSpent";
private static final String HISTOGRAM_START_SURFACE_MODULE_CLICK = "StartSurface.Module.Click";
private static final String USER_ACTION_START_SURFACE_MVT_CLICK =
"Suggestions.Tile.Tapped.StartSurface";

@Mock
private UmaRecorder mUmaRecorder;

@Rule
public StartSurfaceCoordinatorUnitTestRule mTestRule =
Expand Down Expand Up @@ -313,6 +336,114 @@ public void testRecordTimeSpendInStart() {
1, RecordHistogram.getHistogramTotalCountForTesting(START_SURFACE_TIME_SPENT));
}

/**
* Test whether the clicking action on MV tiles in {@link StartSurface} is been recorded in
* histogram correctly.
*/
@Test
@SmallTest
public void testRecordHistogramMostVisitedItemClick_StartSurface() {
Tile tileForTest =
new Tile(new SiteSuggestion("0 TOP_SITES", new GURL("https://www.foo.com"),
TileTitleSource.TITLE_TAG, TileSource.TOP_SITES,
TileSectionType.PERSONALIZED),
0);
TileGroupDelegateImpl tileGroupDelegate = mCoordinator.getTileGroupDelegateForTesting();

// Test clicking on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.CURRENT_TAB, tileForTest);
Assert.assertEquals(HISTOGRAM_START_SURFACE_MODULE_CLICK
+ " is not recorded correctly when click on MV tiles.",
1,
RecordHistogram.getHistogramValueCountForTesting(
HISTOGRAM_START_SURFACE_MODULE_CLICK,
BrowserUiUtils.ModuleTypeOnStartAndNTP.MOST_VISITED_TILES));

// Test long press then open in new tab on MV tiles.
tileGroupDelegate.openMostVisitedItem(
WindowOpenDisposition.NEW_BACKGROUND_TAB, tileForTest);
Assert.assertEquals(HISTOGRAM_START_SURFACE_MODULE_CLICK + " is not recorded "
+ "correctly when long press then open in new tab on MV tiles.",
2,
RecordHistogram.getHistogramValueCountForTesting(
HISTOGRAM_START_SURFACE_MODULE_CLICK,
BrowserUiUtils.ModuleTypeOnStartAndNTP.MOST_VISITED_TILES));

// Test long press then open in other window on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.NEW_WINDOW, tileForTest);
Assert.assertEquals(HISTOGRAM_START_SURFACE_MODULE_CLICK
+ " shouldn't be recorded when long press then open in other window "
+ "on MV tiles.",
2,
RecordHistogram.getHistogramValueCountForTesting(
HISTOGRAM_START_SURFACE_MODULE_CLICK,
BrowserUiUtils.ModuleTypeOnStartAndNTP.MOST_VISITED_TILES));

// Test long press then download link on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.SAVE_TO_DISK, tileForTest);
Assert.assertEquals(HISTOGRAM_START_SURFACE_MODULE_CLICK
+ " is not recorded correctly when long press then download link "
+ "on MV tiles.",
3,
RecordHistogram.getHistogramValueCountForTesting(
HISTOGRAM_START_SURFACE_MODULE_CLICK,
BrowserUiUtils.ModuleTypeOnStartAndNTP.MOST_VISITED_TILES));

// Test long press then open in Incognito tab on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.OFF_THE_RECORD, tileForTest);
Assert.assertEquals(HISTOGRAM_START_SURFACE_MODULE_CLICK + " is not recorded correctly "
+ "when long press then open in Incognito tab on MV tiles.",
4,
RecordHistogram.getHistogramValueCountForTesting(
HISTOGRAM_START_SURFACE_MODULE_CLICK,
BrowserUiUtils.ModuleTypeOnStartAndNTP.MOST_VISITED_TILES));
}

/**
* Test whether the clicking action on MV tiles in {@link StartSurface} is been recorded
* as user actions correctly.
*/
@Test
@SmallTest
public void testRecordUserActionMostVisitedItemClick_StartSurface() {
UmaRecorderHolder.setNonNativeDelegate(mUmaRecorder);

Tile tileForTest =
new Tile(new SiteSuggestion("0 TOP_SITES", new GURL("https://www.foo.com"),
TileTitleSource.TITLE_TAG, TileSource.TOP_SITES,
TileSectionType.PERSONALIZED),
0);
TileGroupDelegateImpl tileGroupDelegate = mCoordinator.getTileGroupDelegateForTesting();

// Test clicking on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.CURRENT_TAB, tileForTest);
verify(mUmaRecorder, times(1))
.recordUserAction(eq(USER_ACTION_START_SURFACE_MVT_CLICK), anyLong());

// Test long press then open in new tab on MV tiles.
tileGroupDelegate.openMostVisitedItem(
WindowOpenDisposition.NEW_BACKGROUND_TAB, tileForTest);
verify(mUmaRecorder, times(1))
.recordUserAction(eq(USER_ACTION_START_SURFACE_MVT_CLICK), anyLong());

// Test long press then open in other window on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.NEW_WINDOW, tileForTest);
verify(mUmaRecorder, times(1))
.recordUserAction(eq(USER_ACTION_START_SURFACE_MVT_CLICK), anyLong());

// Test long press then download link on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.SAVE_TO_DISK, tileForTest);
verify(mUmaRecorder, times(1))
.recordUserAction(eq(USER_ACTION_START_SURFACE_MVT_CLICK), anyLong());

// Test long press then open in Incognito tab on MV tiles.
tileGroupDelegate.openMostVisitedItem(WindowOpenDisposition.OFF_THE_RECORD, tileForTest);
verify(mUmaRecorder, times(2))
.recordUserAction(eq(USER_ACTION_START_SURFACE_MVT_CLICK), anyLong());

UmaRecorderHolder.resetForTesting();
}

/**
* Check that the next decision time is within |numOfDays| from now.
* @param numOfDays Number of days to check.
Expand Down
Expand Up @@ -82,6 +82,7 @@
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.ui.native_page.NativePage;
import org.chromium.chrome.browser.ui.native_page.NativePageHost;
import org.chromium.chrome.browser.util.BrowserUiUtils;
import org.chromium.chrome.browser.xsurface.FeedLaunchReliabilityLogger.SurfaceType;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.settings.SettingsLauncher;
Expand Down Expand Up @@ -283,7 +284,8 @@ public void onLoadingComplete() {
private class NewTabPageTileGroupDelegate extends TileGroupDelegateImpl {
private NewTabPageTileGroupDelegate(Context context, Profile profile,
SuggestionsNavigationDelegate navigationDelegate, SnackbarManager snackbarManager) {
super(context, profile, navigationDelegate, snackbarManager);
super(context, profile, navigationDelegate, snackbarManager,
BrowserUiUtils.HostSurface.NEW_TAB_PAGE);
}

@Override
Expand Down Expand Up @@ -992,6 +994,11 @@ public NewTabPageManager getNewTabPageManagerForTesting() {
return mNewTabPageManager;
}

@VisibleForTesting
public TileGroup.Delegate getTileGroupDelegateForTesting() {
return mTileGroupDelegate;
}

/**
* @param isTopMargin True to return the top margin; False to return bottom margin.
* @return The top margin or bottom margin of the logo.
Expand Down
Expand Up @@ -20,31 +20,42 @@
import org.chromium.chrome.browser.ui.messages.snackbar.Snackbar;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager.SnackbarController;
import org.chromium.chrome.browser.util.BrowserUiUtils;
import org.chromium.ui.mojom.WindowOpenDisposition;
import org.chromium.url.GURL;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

/**
* Reusable implementation of {@link TileGroup.Delegate}. Performs work in parts of the system that
* the {@link TileGroup} should not know about.
*/
public class TileGroupDelegateImpl implements TileGroup.Delegate {
private static final Set<Integer> MVTilesClickForUserAction = new HashSet<>(
Arrays.asList(WindowOpenDisposition.CURRENT_TAB, WindowOpenDisposition.OFF_THE_RECORD));

private final Context mContext;
private final SnackbarManager mSnackbarManager;
private final SuggestionsNavigationDelegate mNavigationDelegate;
private final MostVisitedSites mMostVisitedSites;

private boolean mIsDestroyed;
private SnackbarController mTileRemovedSnackbarController;
@BrowserUiUtils.HostSurface
private int mHostSurface;

public TileGroupDelegateImpl(Context context, Profile profile,
SuggestionsNavigationDelegate navigationDelegate, SnackbarManager snackbarManager) {
SuggestionsNavigationDelegate navigationDelegate, SnackbarManager snackbarManager,
@BrowserUiUtils.HostSurface int hostSurface) {
mContext = context;
mSnackbarManager = snackbarManager;
mNavigationDelegate = navigationDelegate;
mMostVisitedSites =
SuggestionsDependencyFactory.getInstance().createMostVisitedSites(profile);
mHostSurface = hostSurface;
}

@Override
Expand All @@ -59,6 +70,8 @@ public void removeMostVisitedItem(Tile item, Callback<GURL> removalUndoneCallbac
public void openMostVisitedItem(int windowDisposition, Tile item) {
assert !mIsDestroyed;

recordClickMvTiles(windowDisposition, mHostSurface);

String url = item.getUrl().getSpec();

// TODO(treib): Should we call recordOpenedMostVisitedItem here?
Expand All @@ -74,6 +87,8 @@ public void openMostVisitedItem(int windowDisposition, Tile item) {
public void openMostVisitedItemInGroup(int windowDisposition, Tile item) {
assert !mIsDestroyed;

recordClickMvTiles(windowDisposition, mHostSurface);

String url = item.getUrl().getSpec();

recordOpenedTile(item);
Expand Down Expand Up @@ -148,4 +163,23 @@ private void recordOpenedTile(Tile tile) {
ReturnToChromeUtil.onMVTileOpened();
mMostVisitedSites.recordOpenedMostVisitedItem(tile);
}

/**
* Records user clicking on MV tiles in New tab page or Start surface.
* @param windowDisposition How to open (new window, current tab, etc).
* @param hostSurface The corresponding item of the host name in {@link
* BrowserUiUtils.HostSurface} which indicates the page
* where the Mv tiles located.
*/
private void recordClickMvTiles(
int windowDisposition, @BrowserUiUtils.HostSurface int hostSurface) {
if (windowDisposition != WindowOpenDisposition.NEW_WINDOW) {
BrowserUiUtils.recordModuleClickHistogram(
hostSurface, BrowserUiUtils.ModuleTypeOnStartAndNTP.MOST_VISITED_TILES);
}
if (MVTilesClickForUserAction.contains(windowDisposition)) {
RecordUserAction.record(
"Suggestions.Tile.Tapped." + BrowserUiUtils.getHostName(hostSurface));
}
}
}
Expand Up @@ -51,7 +51,6 @@
import org.chromium.chrome.features.start_surface.StartSurfaceState;
import org.chromium.chrome.features.start_surface.StartSurfaceUserData;
import org.chromium.components.browser_ui.widget.gesture.BackPressHandler;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.components.segmentation_platform.SegmentSelectionResult;
import org.chromium.components.segmentation_platform.SegmentationPlatformService;
Expand Down Expand Up @@ -287,17 +286,12 @@ private static Tab handleLoadUrlWithPostDataFromStartSurface(LoadUrlParams param
StartSurfaceUserData.setOpenedFromStart(newTab);
}

if (params.getTransitionType() == PageTransition.AUTO_BOOKMARK) {
if (!TextUtils.equals(UrlConstants.RECENT_TABS_URL, params.getUrl())
&& params.getReferrer() == null) {
RecordUserAction.record("Suggestions.Tile.Tapped.StartSurface");
}
} else if (url == null) {
RecordUserAction.record("MobileMenuNewTab.StartSurfaceFinale");
} else {
int transitionAfterMask = params.getTransitionType() & PageTransition.CORE_MASK;
if (transitionAfterMask == PageTransition.TYPED
|| transitionAfterMask == PageTransition.GENERATED) {
RecordUserAction.record("MobileOmniboxUse.StartSurface");

// These are duplicated here but would have been recorded by LocationBarLayout#loadUrl.
// These are not duplicated here with the recording in LocationBarLayout#loadUrl.
RecordUserAction.record("MobileOmniboxUse");
LocaleManager.getInstance().recordLocaleBasedSearchMetrics(
false, url, params.getTransitionType());
Expand Down

0 comments on commit f74142c

Please sign in to comment.