Skip to content

Commit

Permalink
[TTS] Remove support:mandatory promo
Browse files Browse the repository at this point in the history
Deprecate the IS_MANDATORY_PROMO_ENABLED in ContextualSearchFieldTrial,
and the metrics it records.
  Search.ContextualSearchFirstRunFlowOutcome
  Search.ContextualSearchMandatoryPromoOutcomeByGesture
  Search.ContextualSearchPromoOutcomeByGesture

Since the mendatory promo is deleted, we can display the content all the
time. Therefore, OverlayPanelBase#isSupportedState can be deleted
because all the states are supported now.


Bug:1323152

Change-Id: Ic36d6e4f5b805933011d718357a0b0030d305e11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3631263
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: Tomasz Wiszkowski <ender@google.com>
Cr-Commit-Position: refs/heads/main@{#1001196}
  • Loading branch information
Gang Wu authored and Chromium LUCI CQ committed May 9, 2022
1 parent 8fed528 commit 7d5a319
Show file tree
Hide file tree
Showing 12 changed files with 29 additions and 285 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -651,17 +651,6 @@ protected void setPanelState(@PanelState int state, @StateChangeReason int reaso
mPanelState = state;
}

/**
* Determines if a given {@code PanelState} is supported by the Panel. By default,
* all states are supported, but subclasses can override this class to inform
* custom supported states.
* @param state A given state.
* @return Whether the panel supports a given state.
*/
protected boolean isSupportedState(@PanelState int state) {
return true;
}

/**
* Determines if a given {@code PanelState} is a valid UI state. The UNDEFINED state
* should never be considered a valid UI state.
Expand All @@ -671,20 +660,7 @@ protected boolean isSupportedState(@PanelState int state) {
private boolean isValidUiState(@PanelState int state) {
// TODO(pedrosimonetti): consider removing the UNDEFINED state
// which would allow removing this method.
return isSupportedState(state) && state != PanelState.UNDEFINED;
}

/**
* @return The maximum state supported by the panel.
*/
private @PanelState int getMaximumSupportedState() {
if (isSupportedState(PanelState.MAXIMIZED)) {
return PanelState.MAXIMIZED;
} else if (isSupportedState(PanelState.EXPANDED)) {
return PanelState.EXPANDED;
} else {
return PanelState.PEEKED;
}
return state != PanelState.UNDEFINED;
}

/**
Expand All @@ -697,11 +673,7 @@ private boolean isValidUiState(@PanelState int state) {
@PanelState
Integer prevState =
state >= PanelState.PEEKED && state <= PanelState.MAXIMIZED ? state - 1 : null;
if (prevState != null && !isSupportedState(PanelState.EXPANDED)) {
prevState = prevState >= PanelState.PEEKED && prevState <= PanelState.MAXIMIZED
? prevState - 1
: null;
}

return prevState != null ? prevState : PanelState.UNDEFINED;
}

Expand Down Expand Up @@ -811,9 +783,9 @@ && desiredPanelHeight < getPanelHeightFromState(nextState)) {
* @param height The height of the panel in dps.
*/
protected void setClampedPanelHeight(float height) {
final float clampedHeight = MathUtils.clamp(height,
getPanelHeightFromState(getMaximumSupportedState()),
getPanelHeightFromState(PanelState.PEEKED));
final float clampedHeight =
MathUtils.clamp(height, getPanelHeightFromState(PanelState.MAXIMIZED),
getPanelHeightFromState(PanelState.PEEKED));
setPanelHeight(clampedHeight);
}

Expand Down Expand Up @@ -1011,22 +983,12 @@ protected void updatePanelForExpansion(float percentage) {
* @param percentage The completion percentage.
*/
protected void updatePanelForMaximization(float percentage) {
boolean supportsExpandedState = isSupportedState(PanelState.EXPANDED);

// Base page offset.
float startTargetY = supportsExpandedState ? getBasePageTargetY() : 0.0f;
mBasePageY = MathUtils.interpolate(
startTargetY,
getBasePageTargetY(),
percentage);
mBasePageY = getBasePageTargetY();

// Base page brightness.
float startBrightness = supportsExpandedState
? BASE_PAGE_BRIGHTNESS_STATE_EXPANDED : BASE_PAGE_BRIGHTNESS_STATE_PEEKED;
mBasePageBrightness = MathUtils.interpolate(
startBrightness,
BASE_PAGE_BRIGHTNESS_STATE_MAXIMIZED,
percentage);
mBasePageBrightness = MathUtils.interpolate(BASE_PAGE_BRIGHTNESS_STATE_EXPANDED,
BASE_PAGE_BRIGHTNESS_STATE_MAXIMIZED, percentage);

// Bar border.
mIsBarBorderVisible = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ interface ContextualSearchPanelSectionHost {
interface ContextualSearchPromoHost extends ContextualSearchPanelSectionHost {
/**
* Notifies that the user has opted in.
* @param wasMandatory Whether the Promo was mandatory.
*/
void onPromoOptIn(boolean wasMandatory);
void onPromoOptIn();

/**
* Notifies that the user has opted out.
Expand Down Expand Up @@ -263,11 +262,6 @@ public void setPanelState(@PanelState int toState, @StateChangeReason int reason
mDidStartCollapsing = false;
}

@Override
protected boolean isSupportedState(@PanelState int state) {
return canDisplayContentInPanel() || state != PanelState.MAXIMIZED;
}

@Override
protected @PanelState int getProjectedState(float velocity) {
@PanelState
Expand Down Expand Up @@ -418,9 +412,7 @@ public boolean canBeSuppressed() {

@Override
public void notifyBarTouched(float x) {
if (canDisplayContentInPanel()) {
getOverlayPanelContent().showContent();
}
getOverlayPanelContent().showContent();
}

@Override
Expand Down Expand Up @@ -526,9 +518,9 @@ public void setWasSearchContentViewSeen() {
* @param isActive Whether the promo is active.
*/
@Override
public void setIsPromoActive(boolean isActive, boolean isMandatory) {
public void setIsPromoActive(boolean isActive) {
if (isActive) {
getPromoControl().show(isMandatory);
getPromoControl().show();
} else {
getPromoControl().hide();
}
Expand Down Expand Up @@ -1125,12 +1117,7 @@ public void onPanelSectionSizeChange(boolean hasStarted) {
}

@Override
public void onPromoOptIn(boolean wasMandatory) {
if (wasMandatory) {
getOverlayPanelContent().showContent();
expandPanel(StateChangeReason.OPTIN);
}
}
public void onPromoOptIn() {}

@Override
public void onPromoOptOut() {
Expand Down Expand Up @@ -1320,13 +1307,6 @@ public void onSuggestionClicked(int selectionIndex) {
// Panel Content
// ============================================================================================

/**
* @return Whether the content can be displayed in the panel.
*/
public boolean canDisplayContentInPanel() {
return !getPromoControl().isMandatory();
}

@Override
public void onTouchSearchContentViewAck() {
mHasContentBeenTouched = true;
Expand All @@ -1346,7 +1326,7 @@ public void destroyContent() {
* @return Whether the panel content can be displayed in a new tab.
*/
public boolean canPromoteToNewTab() {
return mActivityType == ActivityType.TABBED && canDisplayContentInPanel();
return mActivityType == ActivityType.TABBED;
}

// ============================================================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public boolean didTouchContent() {
}

@Override
public void setIsPromoActive(boolean show, boolean isMandatory) {}
public void setIsPromoActive(boolean show) {}

@Override
public boolean wasPromoInteractive() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public interface ContextualSearchPanelInterface {

/** {@link ContextualSearchPanel} methods */
boolean didTouchContent();
void setIsPromoActive(boolean show, boolean isMandatory);
void setIsPromoActive(boolean show);
boolean wasPromoInteractive();
void destroyContent();
void setSearchTerm(String searchTerm);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ public class ContextualSearchPromoControl extends OverlayPanelInflater {
/** Whether the Promo is visible. */
private boolean mIsVisible;

/** Whether the Promo is mandatory. */
private boolean mIsMandatory;

/** The opacity of the Promo. */
private float mOpacity;

Expand Down Expand Up @@ -106,17 +103,15 @@ public class ContextualSearchPromoControl extends OverlayPanelInflater {

/**
* Shows the Promo. This includes inflating the View and setting its initial state.
* @param isMandatory Whether the Promo is mandatory.
*/
void show(boolean isMandatory) {
void show() {
if (mIsVisible) return;

// Invalidates the View in order to generate a snapshot, but do not show the View yet.
// The View should only be displayed when in the expanded state.
invalidate();

mIsVisible = true;
mIsMandatory = isMandatory;
mWasInteractive = false;

mHeightPx = mContentHeightPx;
Expand All @@ -131,7 +126,6 @@ void hide() {
hidePromoView();

mIsVisible = false;
mIsMandatory = false;

mHeightPx = 0.f;
mOpacity = 0.f;
Expand All @@ -145,13 +139,7 @@ void onContextualSearchPrefChanged(boolean isEnabled) {
if (!mIsVisible || !mOverlayPanel.isShowing()) return;

if (isEnabled) {
boolean wasMandatory = mIsMandatory;
// Set mandatory state to false right now because it controls whether the Content
// can be displayed. See {@link ContextualSearchPanel#canDisplayContentInPanel}.
// Now that the feature is enabled, the host will try to show the Contents.
// See {@link ContextualSearchPanel#getContextualSearchPromoHost}.
mIsMandatory = false;
mHost.onPromoOptIn(wasMandatory);
mHost.onPromoOptIn();
} else {
mHost.onPromoOptOut();
}
Expand All @@ -166,13 +154,6 @@ public boolean isVisible() {
return mIsVisible;
}

/**
* @return Whether the Promo is mandatory.
*/
public boolean isMandatory() {
return mIsMandatory;
}

/**
* @return Whether the Promo reached a state in which it could be interacted.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ public class ContextualSearchFieldTrial {
static final String CONTEXTUAL_SEARCH_PROMO_CARD_MAX_SHOWN_PARAM_NAME = "promo_card_max_shown";
private static final int PROMO_DEFAULT_LIMIT = 3;

// Deprecated.
private static final int MANDATORY_PROMO_DEFAULT_LIMIT = 10;

// Cached values to avoid repeated and redundant JNI operations.
private static Boolean sEnabled;
private static Boolean[] sSwitches = new Boolean[ContextualSearchSwitch.NUM_ENTRIES];
Expand Down Expand Up @@ -96,6 +93,7 @@ public class ContextualSearchFieldTrial {
int IS_ONLINE_DETECTION_DISABLED = 1;

int IS_SEARCH_TERM_RESOLUTION_DISABLED = 2;
/** @deprecated */
int IS_MANDATORY_PROMO_ENABLED = 3;

/**
Expand Down Expand Up @@ -204,7 +202,10 @@ public class ContextualSearchFieldTrial {
* have gaps.
*/
@interface ContextualSearchSetting {
/** The number of times the Promo should be seen before it becomes mandatory. */
/**
* @deprecated
* The number of times the Promo should be seen before it becomes mandatory.
*/
int MANDATORY_PROMO_LIMIT = 0;
/**
* A Y value limit that will suppress a Tap near the top of the screen.
Expand Down Expand Up @@ -279,10 +280,7 @@ static boolean getSwitch(@ContextualSearchSwitch int value) {

static int getValue(@ContextualSearchSetting int value) {
if (sSettings[value] == null) {
sSettings[value] = getIntParamValueOrDefault(ContextualSearchSettingNames[value],
value == ContextualSearchSetting.MANDATORY_PROMO_LIMIT
? MANDATORY_PROMO_DEFAULT_LIMIT
: 0);
sSettings[value] = getIntParamValueOrDefault(ContextualSearchSettingNames[value], 0);
}
return sSettings[value].intValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,6 @@ public class ContextualSearchManager
private boolean mShouldLoadDelayedSearch;

private boolean mIsShowingPromo;
private boolean mIsMandatoryPromo;
private boolean mDidLogPromoOutcome;

/**
* Whether contextual search manager is currently promoting a tab. We should be ignoring hide
Expand Down Expand Up @@ -370,7 +368,6 @@ public void initialize(@NonNull ViewGroup parentView, @NonNull LayoutManagerImpl
mRedirectHandler = RedirectHandler.create();

mIsShowingPromo = false;
mDidLogPromoOutcome = false;
mDidStartLoadingResolvedSearchRequest = false;
mWereSearchResultsSeen = false;
mIsInitialized = true;
Expand Down Expand Up @@ -500,13 +497,8 @@ public void onCloseContextualSearch(@StateChangeReason int reason) {
mRelatedSearches = null;
mIsRelatedSearchesSerp = false;

if (mIsShowingPromo && !mDidLogPromoOutcome && mSearchPanel.wasPromoInteractive()) {
ContextualSearchUma.logPromoOutcome(mWasActivatedByTap, mIsMandatoryPromo);
mDidLogPromoOutcome = true;
}

mIsShowingPromo = false;
mSearchPanel.setIsPromoActive(false, false);
mSearchPanel.setIsPromoActive(false);
mSearchPanel.clearRelatedSearches();
notifyHideContextualSearch();
}
Expand Down Expand Up @@ -580,9 +572,7 @@ private void showContextualSearch(@StateChangeReason int stateChangeReason) {
// Note: now that the contextual search has properly started, set the promo involvement.
if (mPolicy.isPromoAvailable()) {
mIsShowingPromo = true;
mIsMandatoryPromo = mPolicy.isMandatoryPromoAvailable();
mDidLogPromoOutcome = false;
mSearchPanel.setIsPromoActive(true, mIsMandatoryPromo);
mSearchPanel.setIsPromoActive(true);
mSearchPanel.setDidSearchInvolvePromo();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.chromium.base.Log;
import org.chromium.blink_public.input.SelectionGranularity;
import org.chromium.chrome.browser.compositor.bottombar.contextualsearch.ContextualSearchPanelInterface;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchFieldTrial.ContextualSearchSetting;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchFieldTrial.ContextualSearchSwitch;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchInternalStateController.InternalState;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchSelectionController.SelectionType;
Expand Down Expand Up @@ -134,8 +133,7 @@ boolean isTapSupported() {
* explicitly interacts with the feature.
*/
boolean shouldPrefetchSearchResult() {
if (isMandatoryPromoAvailable()
|| PreloadPagesSettingsBridge.getState() == PreloadPagesState.NO_PRELOADING) {
if (PreloadPagesSettingsBridge.getState() == PreloadPagesState.NO_PRELOADING) {
return false;
}

Expand All @@ -159,9 +157,8 @@ boolean isResolvingGesture() {
* @return Whether the previous gesture should resolve.
*/
boolean shouldPreviousGestureResolve() {
if (isMandatoryPromoAvailable()
|| ContextualSearchFieldTrial.getSwitch(
ContextualSearchSwitch.IS_SEARCH_TERM_RESOLUTION_DISABLED)) {
if (ContextualSearchFieldTrial.getSwitch(
ContextualSearchSwitch.IS_SEARCH_TERM_RESOLUTION_DISABLED)) {
return false;
}

Expand Down Expand Up @@ -198,20 +195,6 @@ boolean canSendSurroundings() {
return isContextualSearchFullyEnabled();
}

/**
* @return Whether the Mandatory Promo is enabled.
*/
boolean isMandatoryPromoAvailable() {
if (!isUserUndecided()
|| !ContextualSearchFieldTrial.getSwitch(
ContextualSearchSwitch.IS_MANDATORY_PROMO_ENABLED)) {
return false;
}

return getPromoOpenCount() >= ContextualSearchFieldTrial.getValue(
ContextualSearchSetting.MANDATORY_PROMO_LIMIT);
}

/**
* @return Whether the Opt-out promo is available to be shown in any panel.
*/
Expand Down

0 comments on commit 7d5a319

Please sign in to comment.