Skip to content

Commit

Permalink
[CS] Delete BarOverlapTapSuppression.java
Browse files Browse the repository at this point in the history
Delete BarOverlapTapSuppression.java and the metrics it records.
  Search.ContextualSearchBarNoOverlap.PeekDuration
  Search.ContextualSearchBarOverlap
  Search.ContextualSearchBarOverlap.PeekDuration
  Search.ContextualSearchBarOverlapSeen


Bug: 1321250
Change-Id: Id271dce891ff05ae48d4cb7772248548ea284828
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615533
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#998661}
  • Loading branch information
Gang Wu authored and Chromium LUCI CQ committed May 3, 2022
1 parent c0dd256 commit 0075a0f
Show file tree
Hide file tree
Showing 13 changed files with 10 additions and 301 deletions.
1 change: 0 additions & 1 deletion chrome/android/chrome_java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/contextmenu/ContextMenuUi.java",
"java/src/org/chromium/chrome/browser/contextmenu/ContextMenuUtils.java",
"java/src/org/chromium/chrome/browser/contextmenu/LensChipDelegate.java",
"java/src/org/chromium/chrome/browser/contextualsearch/BarOverlapTapSuppression.java",
"java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchContext.java",
"java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchEntityHeuristic.java",
"java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFieldTrial.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ public void onPanelStateChanged(@PanelState int fromState, @PanelState int toSta
if (mResultsSeenExperiments != null) {
mResultsSeenExperiments.logResultsSeen(
mWasSearchContentViewSeen, mWasActivatedByTap);
mResultsSeenExperiments.logPanelViewedDurations(
panelViewDurationMs, mPanelOpenedBeyondPeekDurationMs);
if (!isChained) mResultsSeenExperiments = null;
}
mPanelOpenedBeyondPeekDurationMs = 0;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,13 @@ public class ContextualSearchFieldTrial {
* Enables usage of English as the target language even when it's the primary UI language.
*/
int IS_ENGLISH_TARGET_TRANSLATION_ENABLED = 4;
/** Whether collecting data on Bar overlap is enabled. */
/**
* @deprecated
* Whether collecting data on Bar overlap is enabled.
*/
int IS_BAR_OVERLAP_COLLECTION_ENABLED = 5;
/**
* @deprecated
* Whether triggering is suppressed by a selection nearly overlapping the normal
* Bar peeking location.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,6 @@ protected void logConditionState() {}
*/
protected void logResultsSeen(boolean wasSearchContentViewSeen, boolean wasActivatedByTap) {}

/**
* Optionally logs data about the duration the panel was viewed and /or opened.
* Default is to not log anything.
* @param panelViewDurationMs The duration that the panel was viewed (Peek and opened) by the
* user. This should always be a positive number, since this method is only called when
* the panel has been viewed (Peeked).
* @param panelOpenDurationMs The duration that the panel was opened, or 0 if it was never
* opened.
*/
protected void logPanelViewedDurations(long panelViewDurationMs, long panelOpenDurationMs) {}

/**
* @return Whether this heuristic should be considered when logging aggregate metrics for Tap
* suppression.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,6 @@ public void logResultsSeen(boolean wasSearchContentViewSeen, boolean wasActivate
}
}

/**
* Optionally logs data about the duration the panel was viewed and /or opened.
* Default is to not log anything.
* @param panelViewDurationMs The duration that the panel was viewed (Peek and opened) by the
* user. This should always be a positive number, since this method is only called when
* the panel has been viewed (Peeked).
* @param panelOpenDurationMs The duration that the panel was opened, or 0 if it was never
* opened.
*/
public void logPanelViewedDurations(long panelViewDurationMs, long panelOpenDurationMs) {
for (ContextualSearchHeuristic heuristic : mHeuristics) {
heuristic.logPanelViewedDurations(panelViewDurationMs, panelOpenDurationMs);
}
}

/**
* Logs the condition state for all the Tap suppression heuristics.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,8 @@ public void onExitFullscreen(Tab tab) {
};

mFullscreenManager.addObserver(mFullscreenObserver);
mSelectionController = new ContextualSearchSelectionController(
activity, this, mTabSupplier, mBrowserControlsStateProvider);
mSelectionController =
new ContextualSearchSelectionController(activity, this, mTabSupplier);
mNetworkCommunicator = this;
mPolicy = new ContextualSearchPolicy(mSelectionController, mNetworkCommunicator);
mTranslateController = new ContextualSearchTranslationImpl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.chromium.base.Log;
import org.chromium.base.TimeUtils;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchFieldTrial.ContextualSearchSetting;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchFieldTrial.ContextualSearchSwitch;
Expand Down Expand Up @@ -74,9 +73,6 @@ public class ContextualSearchSelectionController {
/** A means of accessing the currently active tab. */
private final Supplier<Tab> mTabSupplier;

/** A means of interacting with the browser controls. */
private final BrowserControlsStateProvider mBrowserControlsStateProvider;

private ContextualSearchPolicy mPolicy;

/**
Expand Down Expand Up @@ -169,15 +165,12 @@ public void onTouchDown() {
* @param activity The activity for resource and view access.
* @param handler The handler for callbacks.
* @param tabSupplier Access to the currently active tab.
* @param browserControlsStateProvider Access to the browser controls system.
*/
public ContextualSearchSelectionController(Activity activity,
ContextualSearchSelectionHandler handler, Supplier<Tab> tabSupplier,
BrowserControlsStateProvider browserControlsStateProvider) {
ContextualSearchSelectionHandler handler, Supplier<Tab> tabSupplier) {
mActivity = activity;
mHandler = handler;
mTabSupplier = tabSupplier;
mBrowserControlsStateProvider = browserControlsStateProvider;
mPxToDp = 1.f / mActivity.getResources().getDisplayMetrics().density;
mContainsWordPattern = Pattern.compile(CONTAINS_WORD_PATTERN);
// TODO(donnd): remove when behind-the-flag bug fixed (crbug.com/786589).
Expand Down Expand Up @@ -232,11 +225,6 @@ Supplier<Tab> getTabSupplier() {
return mTabSupplier;
}

/** @return A means of interacting with the browser controls system. */
BrowserControlsStateProvider getBrowserControlsStateProvider() {
return mBrowserControlsStateProvider;
}

/**
* @return the type of the selection.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,28 +304,6 @@ public class ContextualSearchUma {
int NUM_ENTRIES = 6;
}

// Constants for "Bar Overlap" with triggering gesture, and whether the results were seen.
@IntDef({BarOverlapResults.BAR_OVERLAP_RESULTS_SEEN_FROM_TAP,
BarOverlapResults.BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_TAP,
BarOverlapResults.NO_BAR_OVERLAP_RESULTS_SEEN_FROM_TAP,
BarOverlapResults.NO_BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_TAP,
BarOverlapResults.BAR_OVERLAP_RESULTS_SEEN_FROM_LONG_PRESS,
BarOverlapResults.BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_LONG_PRESS,
BarOverlapResults.NO_BAR_OVERLAP_RESULTS_SEEN_FROM_LONG_PRESS,
BarOverlapResults.NO_BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_LONG_PRESS})
@Retention(RetentionPolicy.SOURCE)
private @interface BarOverlapResults {
int BAR_OVERLAP_RESULTS_SEEN_FROM_TAP = 0;
int BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_TAP = 1;
int NO_BAR_OVERLAP_RESULTS_SEEN_FROM_TAP = 2;
int NO_BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_TAP = 3;
int BAR_OVERLAP_RESULTS_SEEN_FROM_LONG_PRESS = 4;
int BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_LONG_PRESS = 5;
int NO_BAR_OVERLAP_RESULTS_SEEN_FROM_LONG_PRESS = 6;
int NO_BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_LONG_PRESS = 7;
int NUM_ENTRIES = 8;
}

// Constants for quick action intent resolution histogram.
@IntDef({QuickActionResolve.FAILED, QuickActionResolve.SINGLE, QuickActionResolve.MULTIPLE})
@Retention(RetentionPolicy.SOURCE)
Expand Down Expand Up @@ -937,41 +915,6 @@ public static void logSerpResultClicked(boolean isShowingRelatedSearchSerp) {
}
}

/**
* Logs the whether the panel was seen and the type of the trigger and if Bar nearly overlapped.
* If the panel was seen, logs the duration of the panel view into a BarOverlap or BarNoOverlap
* duration histogram.
* @param wasPanelSeen Whether the panel was seen.
* @param wasTap Whether the gesture was a Tap or not.
* @param wasBarOverlap Whether the trigger location overlapped the Bar area.
*/
public static void logBarOverlapResultsSeen(
boolean wasPanelSeen, boolean wasTap, boolean wasBarOverlap) {
RecordHistogram.recordEnumeratedHistogram("Search.ContextualSearchBarOverlapSeen",
getBarOverlapEnum(wasBarOverlap, wasPanelSeen, wasTap),
BarOverlapResults.NUM_ENTRIES);
}

/**
* Logs the duration of the panel viewed in its Peeked state before being opened.
* @param wasBarOverlap Whether the trigger location overlapped the Bar area.
* @param panelPeekDurationMs The duration that the panel was peeking before being opened
* by the user.
*/
public static void logBarOverlapPeekDuration(boolean wasBarOverlap, long panelPeekDurationMs) {
String histogram = wasBarOverlap ? "Search.ContextualSearchBarOverlap.PeekDuration"
: "Search.ContextualSearchBarNoOverlap.PeekDuration";
RecordHistogram.recordMediumTimesHistogram(histogram, panelPeekDurationMs);
}

/**
* Log whether the UX was suppressed due to Bar overlap.
* @param wasSuppressed Whether showing the UX was suppressed.
*/
public static void logBarOverlapSuppression(boolean wasSuppressed) {
RecordHistogram.recordBooleanHistogram("Search.ContextualSearchBarOverlap", wasSuppressed);
}

/**
* Logs the length of the selection in two histograms, one when results were seen and one when
* results were not seen.
Expand Down Expand Up @@ -1466,37 +1409,6 @@ public static void logOutcomesTimestamp(long durationMs) {
"Search.ContextualSearch.OutcomesDuration", durationInDays);
}

/**
* Get the encoded value to use for the Bar Overlap histogram by encoding all the input
* parameters.
* @param didBarOverlap Whether the selection overlapped the Bar position.
* @param wasPanelSeen Whether the panel content was seen.
* @param wasTap Whether the gesture was a Tap.
* @return The value for the enum histogram.
*/
private static @BarOverlapResults int getBarOverlapEnum(
boolean didBarOverlap, boolean wasPanelSeen, boolean wasTap) {
if (wasTap) {
if (didBarOverlap) {
return wasPanelSeen ? BarOverlapResults.BAR_OVERLAP_RESULTS_SEEN_FROM_TAP
: BarOverlapResults.BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_TAP;
} else {
return wasPanelSeen ? BarOverlapResults.NO_BAR_OVERLAP_RESULTS_SEEN_FROM_TAP
: BarOverlapResults.NO_BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_TAP;
}
} else {
if (didBarOverlap) {
return wasPanelSeen
? BarOverlapResults.BAR_OVERLAP_RESULTS_SEEN_FROM_LONG_PRESS
: BarOverlapResults.BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_LONG_PRESS;
} else {
return wasPanelSeen
? BarOverlapResults.NO_BAR_OVERLAP_RESULTS_SEEN_FROM_LONG_PRESS
: BarOverlapResults.NO_BAR_OVERLAP_RESULTS_NOT_SEEN_FROM_LONG_PRESS;
}
}
}

/**
* Logs whether Contextual Cards data was shown. Should be logged on tap if Contextual
* Cards integration is enabled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public class TapSuppressionHeuristics extends ContextualSearchHeuristics {
mHeuristics.add(new TapWordEdgeSuppression(contextualSearchContext));
mHeuristics.add(new ContextualSearchEntityHeuristic(contextualSearchContext));
mHeuristics.add(new NearTopTapSuppression(selectionController, y));
mHeuristics.add(new BarOverlapTapSuppression(selectionController, y));
mHeuristics.add(new ShortTextRunSuppression(contextualSearchContext, elementRunLength));
mHeuristics.add(new SmallTextSuppression(fontSizeDips));
// Quick Answer that appears in the Caption via the JS API.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,7 @@ private static class MockCSSelectionController extends ContextualSearchSelection

public MockCSSelectionController(
ChromeActivity activity, ContextualSearchSelectionHandler handler) {
super(activity, handler, activity.getActivityTabProvider(),
activity.getBrowserControlsManager());
super(activity, handler, activity.getActivityTabProvider());
mPopupController = new StubbedSelectionPopupController();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public final class ContextualSearchSelectionControllerTest {
public void setUp() throws Exception {
Activity activity = Robolectric.buildActivity(Activity.class).setup().get();
mSelectionControllerUnderTest =
new ContextualSearchSelectionController(activity, null, null, null);
new ContextualSearchSelectionController(activity, null, null);
mSelectionControllerUnderTest.setSelectedText(USER_SELECTION);
sSelectionSetByHandleSelection = null;
}
Expand Down
Loading

0 comments on commit 0075a0f

Please sign in to comment.