Skip to content

Commit

Permalink
[MerchantTrust] Record message impact on user browsing
Browse files Browse the repository at this point in the history 
This CL records user browsing time and navigation times on the same host
after the trust message shows. We add a parameter to set up the
counterfactuals. Also to better analyze the impact, we suffix the two
histograms with the star ratings.

(cherry picked from commit 12cd3e4)

Bug: 1298610
Change-Id: I985d61fe9b5e73406e8d929a11a9e95374dd0987
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3485003
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Wei-Yin Chen <wychen@chromium.org>
Commit-Queue: Zhiyuan Cai <zhiyuancai@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#978370}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3514893
Auto-Submit: Zhiyuan Cai <zhiyuancai@chromium.org>
Reviewed-by: Krishna Govind <govind@chromium.org>
Commit-Queue: Krishna Govind <govind@chromium.org>
Owners-Override: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/4896@{#427}
Cr-Branched-From: 1f63ff4-refs/heads/main@{#972766}
  • Loading branch information
Zhiyuan Cai authored and Chromium LUCI CQ committed Mar 9, 2022
1 parent c7336b7 commit 365ec89
Show file tree
Hide file tree
Showing 11 changed files with 376 additions and 61 deletions.
1 change: 1 addition & 0 deletions chrome/browser/commerce/merchant_viewer/android/BUILD.gn
View file
Expand Up @@ -88,6 +88,7 @@ java_library("junit") {
"javatests/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustBottomSheetMediatorTest.java",
"javatests/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustMessageContextTest.java",
"javatests/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustMessageSchedulerTest.java",
"javatests/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustMetricsTest.java",
"javatests/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustSignalsCallbackHelper.java",
"javatests/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustSignalsCoordinatorTest.java",
"javatests/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustSignalsDataProviderTest.java",
Expand Down
27 changes: 21 additions & 6 deletions ...d/java/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustMessageScheduler.java
View file
Expand Up @@ -47,21 +47,36 @@ void clear(@MessageClearReason int clearReason) {
clearScheduledMessage(clearReason);
}

/** Adds a message to the underlying {@link MessageDispatcher} queue. */
// TODO(crbug.com/1298610): Clean up this api in tests.
@Deprecated
void schedule(PropertyModel model, MerchantTrustMessageContext messageContext,
long delayInMillis, Callback<MerchantTrustMessageContext> messageEnqueuedCallback) {
schedule(model, 4.0, messageContext, delayInMillis, messageEnqueuedCallback);
}

/** Adds a message to the underlying {@link MessageDispatcher} queue. */
void schedule(PropertyModel model, double starRating,
MerchantTrustMessageContext messageContext, long delayInMillis,
Callback<MerchantTrustMessageContext> messageEnqueuedCallback) {
setScheduledMessage(
new Pair<MerchantTrustMessageContext, PropertyModel>(messageContext, model));
mMetrics.recordMetricsForMessagePrepared();
mEnqueueMessageTimer.postDelayed(() -> {
if (messageContext.isValid() && mTabSupplier.hasValue()
&& messageContext.getWebContents().equals(
mTabSupplier.get().getWebContents())) {
mMessageDispatcher.enqueueMessage(
model, messageContext.getWebContents(), MessageScopeType.NAVIGATION, false);
mMetrics.recordMetricsForMessageShown();
messageEnqueuedCallback.onResult(messageContext);
setScheduledMessage(null);
mMetrics.startRecordingMessageImpact(messageContext.getHostName(), starRating);
if (MerchantViewerConfig.isTrustSignalsMessageDisabledForImpactStudy()) {
messageEnqueuedCallback.onResult(null);
// TODO(crbug.com/1298610): Use a new message clear reason.
clearScheduledMessage(MessageClearReason.UNKNOWN);
} else {
mMessageDispatcher.enqueueMessage(model, messageContext.getWebContents(),
MessageScopeType.NAVIGATION, false);
mMetrics.recordMetricsForMessageShown();
messageEnqueuedCallback.onResult(messageContext);
setScheduledMessage(null);
}
} else {
messageEnqueuedCallback.onResult(null);
if (!messageContext.isValid()) {
Expand Down
80 changes: 80 additions & 0 deletions ...er/android/java/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustMetrics.java
View file
Expand Up @@ -4,7 +4,10 @@

package org.chromium.chrome.browser.merchant_viewer;

import android.text.format.DateUtils;

import androidx.annotation.IntDef;
import androidx.annotation.VisibleForTesting;

import org.chromium.base.TimeUtils;
import org.chromium.base.metrics.RecordHistogram;
Expand All @@ -19,6 +22,13 @@
* Metrics util class for merchant trust.
*/
public class MerchantTrustMetrics {
@VisibleForTesting
public static String MESSAGE_IMPACT_BROWSING_TIME_HISTOGRAM =
"MerchantTrust.MessageImpact.BrowsingTime";
@VisibleForTesting
public static String MESSAGE_IMPACT_NAVIGATION_COUNT_HISTOGRAM =
"MerchantTrust.MessageImpact.NavigationCount";

/**
* The reason why we clear the prepared message.
*
Expand Down Expand Up @@ -75,6 +85,12 @@ public class MerchantTrustMetrics {
private long mBottomSheetHalfOpenedNanoseconds;
private long mBottomSheetFullyOpenedNanoseconds;

// Metrics for message impact on user browsing.
private long mMessageVisibleNsForBrowsingTime;
private int mNavigationCountAfterMessageShown;
private double mMessageStarRating;
private String mCurrentHost;

/** Records metrics when the message is prepared. */
public void recordMetricsForMessagePrepared() {
startMessagePreparedTimer();
Expand Down Expand Up @@ -270,4 +286,68 @@ public void recordMetricsForBottomSheetOpenedSource(@BottomSheetOpenedSource int
RecordHistogram.recordEnumeratedHistogram("MerchantTrust.BottomSheet.OpenSource", source,
BottomSheetOpenedSource.MAX_VALUE + 1);
}

/** Start recording message impact on user browsing time and navigation times. */
public void startRecordingMessageImpact(String hostName, double starRating) {
mMessageVisibleNsForBrowsingTime = System.nanoTime();
mNavigationCountAfterMessageShown = 0;
mCurrentHost = hostName;
mMessageStarRating = starRating;
}

/** Update the message impact data. */
public void updateRecordingMessageImpact(String hostName) {
if (mCurrentHost != null) {
if (mCurrentHost.equals(hostName)) {
mNavigationCountAfterMessageShown++;
} else {
finishRecordingMessageImpact();
}
}
}

/** Finish recording message impact for this host and reset the data. */
public void finishRecordingMessageImpact() {
if (mCurrentHost != null) {
long browsingTime = (System.nanoTime() - mMessageVisibleNsForBrowsingTime)
/ TimeUtils.NANOSECONDS_PER_MILLISECOND;
RecordHistogram.recordCustomTimesHistogram(MESSAGE_IMPACT_BROWSING_TIME_HISTOGRAM,
browsingTime, 10, DateUtils.MINUTE_IN_MILLIS * 10, 50);
RecordHistogram.recordCustomTimesHistogram(
MESSAGE_IMPACT_BROWSING_TIME_HISTOGRAM + getStarRatingSuffixForMessageImpact(),
browsingTime, 10, DateUtils.MINUTE_IN_MILLIS * 10, 50);

RecordHistogram.recordCount100Histogram(
MESSAGE_IMPACT_NAVIGATION_COUNT_HISTOGRAM, mNavigationCountAfterMessageShown);
RecordHistogram.recordCount100Histogram(MESSAGE_IMPACT_NAVIGATION_COUNT_HISTOGRAM
+ getStarRatingSuffixForMessageImpact(),
mNavigationCountAfterMessageShown);
}
mMessageVisibleNsForBrowsingTime = 0;
mNavigationCountAfterMessageShown = 0;
mCurrentHost = null;
mMessageStarRating = 0.0;
}

/**
* To better analyze the message impact, we add a suffix to each histogram based on the shown
* star rating.
*/
private String getStarRatingSuffixForMessageImpact() {
// Only keep one decimal to avoid inaccurate double value such as 4.49999.
double ratingValue = Math.round(mMessageStarRating * 10) / 10.0;
String ratingSuffix;
if (ratingValue >= 4.5) {
ratingSuffix = "AboveFourPointFive";
} else if (ratingValue >= 4.0) {
ratingSuffix = "AboveFour";
} else if (ratingValue >= 3.0) {
ratingSuffix = "AboveThree";
} else if (ratingValue >= 2.0) {
ratingSuffix = "AboveTwo";
} else {
ratingSuffix = "BelowTwo";
}
return ".Rating" + ratingSuffix;
}
}
5 changes: 3 additions & 2 deletions ...java/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustSignalsCoordinator.java
View file
Expand Up @@ -102,7 +102,8 @@ tabSupplier, new MerchantTrustSignalsDataProvider(), profileSupplier, metrics,
mProfileSupplier = profileSupplier;
mWindowAndroid = windowAndroid;

mMediator = new MerchantTrustSignalsMediator(tabSupplier, this::onFinishEligibleNavigation);
mMediator = new MerchantTrustSignalsMediator(
tabSupplier, this::onFinishEligibleNavigation, metrics);
mMessageScheduler = messageScheduler;
mDetailsTabCoordinator = detailsTabCoordinator;
}
Expand Down Expand Up @@ -179,7 +180,7 @@ private void scheduleMessage(MerchantTrustSignalsV2 trustSignals,
assert (trustSignals != null) && (item != null);
mMessageScheduler.schedule(
MerchantTrustMessageViewModel.create(mContext, trustSignals, item.getUrl(), this),
item,
trustSignals.getMerchantStarRating(), item,
shouldExpediteMessage ? MerchantTrustMessageScheduler.MESSAGE_ENQUEUE_NO_DELAY
: MerchantViewerConfig.getDefaultTrustSignalsMessageDelay(),
this::onMessageEnqueued);
Expand Down
16 changes: 14 additions & 2 deletions ...id/java/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustSignalsMediator.java
View file
Expand Up @@ -10,6 +10,7 @@
import org.chromium.chrome.browser.tab.CurrentTabObserver;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabHidingType;
import org.chromium.content_public.browser.NavigationHandle;

/**
Expand All @@ -28,8 +29,8 @@ interface MerchantTrustSignalsCallback {

private final CurrentTabObserver mCurrentTabObserver;

MerchantTrustSignalsMediator(
ObservableSupplier<Tab> tabSupplier, MerchantTrustSignalsCallback delegate) {
MerchantTrustSignalsMediator(ObservableSupplier<Tab> tabSupplier,
MerchantTrustSignalsCallback delegate, MerchantTrustMetrics metrics) {
mCurrentTabObserver = new CurrentTabObserver(tabSupplier, new EmptyTabObserver() {
@Override
public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
Expand All @@ -41,9 +42,20 @@ public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
return;
}

metrics.updateRecordingMessageImpact(navigation.getUrl().getHost());
delegate.onFinishEligibleNavigation(
new MerchantTrustMessageContext(navigation, tab.getWebContents()));
}

@Override
public void onHidden(Tab tab, @TabHidingType int type) {
metrics.finishRecordingMessageImpact();
}

@Override
public void onDestroyed(Tab tab) {
metrics.finishRecordingMessageImpact();
}
});
}

Expand Down
13 changes: 13 additions & 0 deletions ...er/android/java/src/org/chromium/chrome/browser/merchant_viewer/MerchantViewerConfig.java
View file
Expand Up @@ -55,6 +55,9 @@ public class MerchantViewerConfig {
@VisibleForTesting
public static final String TRUST_SIGNALS_MESSAGE_DESCRIPTION_UI_PARAM =
"trust_signals_message_description_ui";
@VisibleForTesting
public static final String TRUST_SIGNALS_MESSAGE_DISABLED_FOR_IMPACT_STUDY_PARAM =
"trust_signals_message_disabled_for_impact_study";

public static int getDefaultTrustSignalsMessageDelay() {
int defaultDelay = (int) TimeUnit.SECONDS.toMillis(30);
Expand Down Expand Up @@ -194,4 +197,14 @@ public static int getTrustSignalsMessageDescriptionUI() {
}
return defaultUI;
}

public static boolean isTrustSignalsMessageDisabledForImpactStudy() {
boolean defaultValue = false;
if (FeatureList.isInitialized()) {
return ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
ChromeFeatureList.COMMERCE_MERCHANT_VIEWER,
TRUST_SIGNALS_MESSAGE_DISABLED_FOR_IMPACT_STUDY_PARAM, defaultValue);
}
return defaultValue;
}
}
45 changes: 44 additions & 1 deletion ...ts/src/org/chromium/chrome/browser/merchant_viewer/MerchantTrustMessageSchedulerTest.java
View file
Expand Up @@ -27,8 +27,10 @@
import org.mockito.MockitoAnnotations;
import org.robolectric.annotation.Config;

import org.chromium.base.FeatureList;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.merchant_viewer.MerchantTrustMetrics.MessageClearReason;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.util.browser.Features;
Expand Down Expand Up @@ -97,12 +99,13 @@ public void testSchedule() throws TimeoutException {
doReturn(true).when(mockMessagesContext).isValid();
doReturn(mMockWebContents).when(mockMessagesContext).getWebContents();
doReturn(mMockWebContents).when(mMockTab).getWebContents();
doReturn("fake_host").when(mockMessagesContext).getHostName();

scheduler.setHandlerForTesting(mMockHandler);

int callCount = callbackHelper.getCallCount();
scheduler.schedule(
mockPropteryModel, mockMessagesContext, 2000, callbackHelper::notifyCalled);
mockPropteryModel, 4.7, mockMessagesContext, 2000, callbackHelper::notifyCalled);
callbackHelper.waitForCallback(callCount);

Assert.assertNotNull(callbackHelper.getResult());
Expand All @@ -111,10 +114,50 @@ public void testSchedule() throws TimeoutException {
verify(mMockMessageDispatcher, times(1))
.enqueueMessage(eq(mockPropteryModel), eq(mMockWebContents),
eq(MessageScopeType.NAVIGATION), eq(false));
verify(mMockMetrics, times(1)).startRecordingMessageImpact(eq("fake_host"), eq(4.7));
verify(mMockMetrics, times(1)).recordMetricsForMessageShown();
Assert.assertNull(scheduler.getScheduledMessageContext());
}

@Test
public void testSchedule_DisableMessageForImpactStudy() throws TimeoutException {
FeatureList.TestValues testValues = new FeatureList.TestValues();
testValues.addFieldTrialParamOverride(ChromeFeatureList.COMMERCE_MERCHANT_VIEWER,
MerchantViewerConfig.TRUST_SIGNALS_MESSAGE_DISABLED_FOR_IMPACT_STUDY_PARAM, "true");
FeatureList.setTestValues(testValues);

MerchantTrustSignalsCallbackHelper callbackHelper =
new MerchantTrustSignalsCallbackHelper();
MerchantTrustMessageScheduler scheduler = getSchedulerUnderTest();
PropertyModel mockPropteryModel = mock(PropertyModel.class);
doReturn(false).when(mMockWebContents).isDestroyed();

MerchantTrustMessageContext mockMessagesContext = mock(MerchantTrustMessageContext.class);
doReturn(true).when(mockMessagesContext).isValid();
doReturn(mMockWebContents).when(mockMessagesContext).getWebContents();
doReturn(mMockWebContents).when(mMockTab).getWebContents();
doReturn("fake_host").when(mockMessagesContext).getHostName();

scheduler.setHandlerForTesting(mMockHandler);

int callCount = callbackHelper.getCallCount();
scheduler.schedule(
mockPropteryModel, 4.7, mockMessagesContext, 2000, callbackHelper::notifyCalled);
callbackHelper.waitForCallback(callCount);

Assert.assertNull(callbackHelper.getResult());
verify(mMockMetrics, times(1)).recordMetricsForMessagePrepared();
verify(mMockHandler, times(1)).postDelayed(any(Runnable.class), eq(2000L));
verify(mMockMessageDispatcher, times(0))
.enqueueMessage(eq(mockPropteryModel), eq(mMockWebContents),
eq(MessageScopeType.NAVIGATION), eq(false));
verify(mMockMetrics, times(1)).startRecordingMessageImpact(eq("fake_host"), eq(4.7));
verify(mMockMetrics, times(0)).recordMetricsForMessageShown();
verify(mMockMetrics, times(1))
.recordMetricsForMessageCleared(eq(MessageClearReason.UNKNOWN));
Assert.assertNull(scheduler.getScheduledMessageContext());
}

@Test
public void testScheduleInvalidMessageContext() throws TimeoutException {
MerchantTrustSignalsCallbackHelper callbackHelper =
Expand Down

0 comments on commit 365ec89

Please sign in to comment.