Skip to content

Commit

Permalink
[TTS] Delete SmallTextSuppression.java and ShortTextRunSuppression.java
Browse files Browse the repository at this point in the history
Delete SmallTextSuppression.java and ShortTextRunSuppression.java since
we are no longer using them.

This CL does not update blink::mojom::UnhandledTapInfoPtr since we can
delete it once we remove TapFarFromPreviousSuppression.java.

Bug: 1324486
Change-Id: Ia37a0d5a189c1152903d5a489193c267e88c7655
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3642332
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002487}
  • Loading branch information
Gang Wu authored and Chromium LUCI CQ committed May 12, 2022
1 parent 988d63a commit 5d5667b
Show file tree
Hide file tree
Showing 13 changed files with 25 additions and 202 deletions.
2 changes: 0 additions & 2 deletions chrome/android/chrome_java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/contextualsearch/RelatedSearchesUma.java",
"java/src/org/chromium/chrome/browser/contextualsearch/ResolvedSearchTerm.java",
"java/src/org/chromium/chrome/browser/contextualsearch/SelectionClientManager.java",
"java/src/org/chromium/chrome/browser/contextualsearch/ShortTextRunSuppression.java",
"java/src/org/chromium/chrome/browser/contextualsearch/SmallTextSuppression.java",
"java/src/org/chromium/chrome/browser/contextualsearch/TapFarFromPreviousSuppression.java",
"java/src/org/chromium/chrome/browser/contextualsearch/TapSuppressionHeuristics.java",
"java/src/org/chromium/chrome/browser/crash/ChromeMinidumpUploadJobServiceImpl.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,15 @@ public class ContextualSearchFieldTrial {
int IS_NOT_AN_ENTITY_SUPPRESSION_ENABLED = 10;
/** Whether triggering is suppressed due to lack of engagement with the feature. */
int IS_ENGAGEMENT_SUPPRESSION_ENABLED = 11;
/** Whether triggering is suppressed for a tap that has a short element run-length. */
/**
* @deprecated
* Whether triggering is suppressed for a tap that has a short element run-length.
*/
int IS_SHORT_TEXT_RUN_SUPPRESSION_ENABLED = 12;
/** Whether triggering is suppressed for a tap on small-looking text. */
/**
* @deprecated
* Whether triggering is suppressed for a tap on small-looking text.
*/
int IS_SMALL_TEXT_SUPPRESSION_ENABLED = 13;
/**
* Whether to disable auto-promotion of clicks in the AMP carousel into a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1559,8 +1559,8 @@ public void cancelAllRequests() {}
}

/** Shows the Unhandled Tap UI. Called by {@link ContextualSearchTabHelper}. */
void onShowUnhandledTapUIIfNeeded(int x, int y, int fontSizeDips, int textRunLength) {
mSelectionController.handleShowUnhandledTapUIIfNeeded(x, y, fontSizeDips, textRunLength);
void onShowUnhandledTapUIIfNeeded(int x, int y) {
mSelectionController.handleShowUnhandledTapUIIfNeeded(x, y);
}

// ============================================================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@ public class ContextualSearchSelectionController {
private float mX;
private float mY;

// Additional tap info from Mojo.
int mFontSizeDips;
int mTextRunLength;

// The time of the most last scroll activity, or 0 if none.
private long mLastScrollTimeNs;

Expand Down Expand Up @@ -403,8 +399,6 @@ private void resetAllStates() {
mLastScrollTimeNs = 0;
mTapTimeNanoseconds = 0;
mDidExpandSelection = false;
mFontSizeDips = 0;
mTextRunLength = 0;
}

/**
Expand Down Expand Up @@ -432,10 +426,8 @@ void onTabSelected() {
* Handles an unhandled tap gesture.
* @param x The x coordinate in px.
* @param y The y coordinate in px.
* @param fontSizeDips The font size in DPs.
* @param textRunLength The run-length of the text of the tapped element.
*/
void handleShowUnhandledTapUIIfNeeded(int x, int y, int fontSizeDips, int textRunLength) {
void handleShowUnhandledTapUIIfNeeded(int x, int y) {
mWasTapGestureDetected = false;
// TODO(donnd): refactor to avoid needing a new handler API method as suggested by Pedro.
if (mSelectionType != SelectionType.LONG_PRESS && !mAreSelectionHandlesShown
Expand All @@ -445,8 +437,6 @@ void handleShowUnhandledTapUIIfNeeded(int x, int y, int fontSizeDips, int textRu
mSelectionType = SelectionType.TAP;
mX = x;
mY = y;
mFontSizeDips = fontSizeDips;
mTextRunLength = textRunLength;
mHandler.handleValidTap();
} else {
// Long press, or long-press selection handles shown; reset last tap state.
Expand All @@ -469,9 +459,8 @@ void handleShouldSuppressTap(ContextualSearchContext contextualSearchContext,
int x = (int) mX;
int y = (int) mY;

TapSuppressionHeuristics tapHeuristics =
new TapSuppressionHeuristics(this, mLastTapState, x, y, contextualSearchContext,
mWasSelectionEmptyBeforeTap, mFontSizeDips, mTextRunLength);
TapSuppressionHeuristics tapHeuristics = new TapSuppressionHeuristics(
this, mLastTapState, x, y, contextualSearchContext, mWasSelectionEmptyBeforeTap);
// TODO(donnd): Move to be called when the panel closes to work with states that change.
tapHeuristics.logConditionState();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,11 +380,10 @@ void onContextualSearchPrefChanged() {
* coordinates.
*/
@CalledByNative
void onShowUnhandledTapUIIfNeeded(int x, int y, int fontSizeDips, int textRunLength) {
void onShowUnhandledTapUIIfNeeded(int x, int y) {
// Only notify the manager if we currently have a valid listener.
if (mGestureStateListener != null && getContextualSearchManager(mTab) != null) {
getContextualSearchManager(mTab).onShowUnhandledTapUIIfNeeded(
x, y, fontSizeDips, textRunLength);
getContextualSearchManager(mTab).onShowUnhandledTapUIIfNeeded(x, y);
}
}

Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,16 @@ public class TapSuppressionHeuristics extends ContextualSearchHeuristics {
* @param y The y position of the Tap.
* @param contextualSearchContext The {@link ContextualSearchContext} of this tap.
* @param wasSelectionEmptyBeforeTap Whether the selection was empty before this tap.
* @param fontSizeDips The font size from Blink in dips.
* @param elementRunLength The length of the text in the element tapped, in characters.
*/
TapSuppressionHeuristics(ContextualSearchSelectionController selectionController,
@Nullable ContextualSearchTapState previousTapState, int x, int y,
ContextualSearchContext contextualSearchContext, boolean wasSelectionEmptyBeforeTap,
int fontSizeDips, int elementRunLength) {
ContextualSearchContext contextualSearchContext, boolean wasSelectionEmptyBeforeTap) {
super();
mHeuristics.add(new EngagementSuppression());
mHeuristics.add(new RecentScrollTapSuppression(selectionController));
mHeuristics.add(new TapFarFromPreviousSuppression(
selectionController, previousTapState, x, y, wasSelectionEmptyBeforeTap));
mHeuristics.add(new ContextualSearchEntityHeuristic(contextualSearchContext));
mHeuristics.add(new ShortTextRunSuppression(contextualSearchContext, elementRunLength));
mHeuristics.add(new SmallTextSuppression(fontSizeDips));
// Quick Answer that appears in the Caption via the JS API.
QuickAnswersHeuristic quickAnswersHeuristic = new QuickAnswersHeuristic();
setQuickAnswersHeuristic(quickAnswersHeuristic);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ protected void mockTapText(String text) {
mContextualSearchManager.getBaseSelectionPopupController().setSelectedText(text);
TestThreadUtils.runOnUiThreadBlocking(() -> {
mContextualSearchManager.getGestureStateListener().onTouchDown();
mContextualSearchManager.onShowUnhandledTapUIIfNeeded(0, 0, 12, 100);
mContextualSearchManager.onShowUnhandledTapUIIfNeeded(0, 0);
});
}

Expand All @@ -246,7 +246,7 @@ protected void mockTapText(String text) {
*/
protected void mockTapEmptySpace() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mContextualSearchManager.onShowUnhandledTapUIIfNeeded(0, 0, 0, 0);
mContextualSearchManager.onShowUnhandledTapUIIfNeeded(0, 0);
mContextualSearchClient.onSelectionEvent(
SelectionEventType.SELECTION_HANDLES_CLEARED, 0, 0);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,12 @@ void ContextualSearchTabHelper::OnContextualSearchPrefChanged() {
Java_ContextualSearchTabHelper_onContextualSearchPrefChanged(env, jobj);
}

void ContextualSearchTabHelper::OnShowUnhandledTapUIIfNeeded(
int x_px,
int y_px,
int font_size_dips,
int text_run_length) {
void ContextualSearchTabHelper::OnShowUnhandledTapUIIfNeeded(int x_px,
int y_px) {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> jobj = weak_java_ref_.get(env);
Java_ContextualSearchTabHelper_onShowUnhandledTapUIIfNeeded(
env, jobj, x_px, y_px, font_size_dips, text_run_length);
Java_ContextualSearchTabHelper_onShowUnhandledTapUIIfNeeded(env, jobj, x_px,
y_px);
}

void ContextualSearchTabHelper::InstallUnhandledTapNotifierIfNeeded(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,7 @@ class ContextualSearchTabHelper {
// Call when an unhandled tap needs to show the UI for a tap at the given
// position, with the given |font_size_dips|, and |text_run_length| of the
// enclosing element.
void OnShowUnhandledTapUIIfNeeded(int x_px,
int y_px,
int font_size_dips,
int text_run_length);
void OnShowUnhandledTapUIIfNeeded(int x_px, int y_px);

JavaObjectWeakGlobalRef weak_java_ref_;
std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,9 @@ void UnhandledTapNotifierImpl::ShowUnhandledTapUIIfNeeded(
float x_px = unhandled_tap_info->tapped_position_in_viewport.x();
float y_px = unhandled_tap_info->tapped_position_in_viewport.y();

// Pixel from Blink are DIPs.
int font_size_dips = unhandled_tap_info->font_size_in_pixels;

// Call back through the callback if possible. (The callback uses a weakptr
// that might make this a NOP).
unhandled_tap_callback_.Run(x_px, y_px, font_size_dips,
unhandled_tap_info->element_text_run_length);
unhandled_tap_callback_.Run(x_px, y_px);
}

// static
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@

namespace contextual_search {

typedef base::RepeatingCallback<
void(int x_px, int y_px, int font_size_dips, int text_run_length)>
UnhandledTapCallback;
typedef base::RepeatingCallback<void(int x_px, int y_px)> UnhandledTapCallback;

// Binds a Mojo unhandled-tap notifier message-handler to the frame host
// observed by this observer.
Expand Down

0 comments on commit 5d5667b

Please sign in to comment.