Commit 9c09335c authored by Donn Denman's avatar Donn Denman Committed by Commit Bot

[TTS][RSearches] Support Related Searches Tap w/o selection.

Adds support for displaying Related Searches using the existing
Contextual Search UX. The main differences are:
1) Tap does not select anything when Related Searches is enabled.
2) Clicks in the Overlay don't promote to a new tab when they are
   only one level deep but do promote to a separate tab after that.

When Related Searches are enabled the Long-press
triggering is automatically enabled so a Long-press resolves normally
but a Tap generates a Related Search without any selection.

The treatment of the selection is gated by the Feature with a
non-empty selection unless the Feature is active (allowing an
insertion point).

All significant functional changes are behind the RelatedSearches
Feature flag.  Functions that have been changed to accommodate this
Feature have tests: Context allows an insertion-point and the
SearchRequest allows non-Google URLs when the Feature is active.

Also removed some unused code.

BUG=1062737

Change-Id: I4b8f133398890c214d26e968c2ede6a1ba08618a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2200437
Commit-Queue: Donn Denman <donnd@chromium.org>
Auto-Submit: Donn Denman <donnd@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769510}
parent e85dbbe6
...@@ -10,10 +10,8 @@ import androidx.annotation.NonNull; ...@@ -10,10 +10,8 @@ import androidx.annotation.NonNull;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.base.Log;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.content_public.browser.WebContents;
/** /**
* Provides a context in which to search, and links to the native ContextualSearchContext. * Provides a context in which to search, and links to the native ContextualSearchContext.
...@@ -84,42 +82,18 @@ public abstract class ContextualSearchContext { ...@@ -84,42 +82,18 @@ public abstract class ContextualSearchContext {
@NonNull @NonNull
private String mTargetLanguage = ""; private String mTargetLanguage = "";
/**
* Support for Related Searches. When {@code true} this allows the context to resolve even
* when the selection is a simple insertion-point.
*/
private boolean mCanResolveInsertionPoint;
/** A {@link ContextualSearchContext} that ignores changes to the selection. */ /** A {@link ContextualSearchContext} that ignores changes to the selection. */
static class ChangeIgnoringContext extends ContextualSearchContext { static class ChangeIgnoringContext extends ContextualSearchContext {
@Override @Override
void onSelectionChanged() {} void onSelectionChanged() {}
} }
/**
* Returns a {@link ContextualSearchContext} given an insertion point in text.
* @param surroundingText The text to use for our context.
* @param insertionPointOffset The offset of the insertion point in characters from the start of
* the surrounding text.
* @return A {@link ContextualSearchContext} or {@code null} if the insertion point happens to
* miss a word (e.g. it has non-word characters on both sides).
*/
public static @Nullable ContextualSearchContext getContextForInsertionPoint(
String surroundingText, int insertionPointOffset) {
ContextualSearchContext context = new ChangeIgnoringContext();
context.setSurroundingText(
"UTF-8", surroundingText, insertionPointOffset, insertionPointOffset);
int start = context.findWordStartOffset(insertionPointOffset);
int end = context.findWordEndOffset(insertionPointOffset);
context.setSurroundingText("UTF-8", surroundingText, start, end, true);
if (start < end && start >= 0 && end <= surroundingText.length()) {
context.setInitialSelectedWord(surroundingText.substring(start, end));
}
if (context.hasValidSelection() && !TextUtils.isEmpty(context.getInitialSelectedWord())) {
Log.i(TAG, "identified default query: " + context.getWordTapped());
// TODO(donnd): figure out which of these parameters should be passed in.
context.setResolveProperties("US", true, 0, 0, "");
return context;
}
// TODO(donnd): Consider hunting around for a valid word instead of just giving up.
return null;
}
/** /**
* Constructs a context that tracks the selection and some amount of page content. * Constructs a context that tracks the selection and some amount of page content.
*/ */
...@@ -170,10 +144,25 @@ public abstract class ContextualSearchContext { ...@@ -170,10 +144,25 @@ public abstract class ContextualSearchContext {
* @param surroundingText The text from the base page surrounding the selection. * @param surroundingText The text from the base page surrounding the selection.
* @param startOffset The offset of start the selection. * @param startOffset The offset of start the selection.
* @param endOffset The offset of the end of the selection * @param endOffset The offset of the end of the selection
* @param canResolveInsertionPoint Whether an insertion-point selection is considered a valid
* selection to pass to the server Resolve request.
*/ */
void setSurroundingText( void setSurroundingText(String encoding, String surroundingText, int startOffset, int endOffset,
String encoding, String surroundingText, int startOffset, int endOffset) { boolean canResolveInsertionPoint) {
setSurroundingText(encoding, surroundingText, startOffset, endOffset, false); setSurroundingText(
encoding, surroundingText, startOffset, endOffset, canResolveInsertionPoint, false);
}
/**
* Sets the surrounding text and selection offsets assuming UTF-8 and no insertion-point
* support.
* @param surroundingText The text from the base page surrounding the selection.
* @param startOffset The offset of start the selection.
* @param endOffset The offset of the end of the selection
*/
@VisibleForTesting
void setSurroundingText(String surroundingText, int startOffset, int endOffset) {
setSurroundingText("UTF-8", surroundingText, startOffset, endOffset, false);
} }
/** /**
...@@ -182,15 +171,19 @@ public abstract class ContextualSearchContext { ...@@ -182,15 +171,19 @@ public abstract class ContextualSearchContext {
* @param surroundingText The text from the base page surrounding the selection. * @param surroundingText The text from the base page surrounding the selection.
* @param startOffset The offset of start the selection. * @param startOffset The offset of start the selection.
* @param endOffset The offset of the end of the selection. * @param endOffset The offset of the end of the selection.
* @param canResolveInsertionPoint Whether an insertion-point selection is considered a valid
* selection to pass to the server Resolve request.
* @param setNative Whether to set the native context too by passing it through JNI. * @param setNative Whether to set the native context too by passing it through JNI.
*/ */
@VisibleForTesting
void setSurroundingText(String encoding, String surroundingText, int startOffset, int endOffset, void setSurroundingText(String encoding, String surroundingText, int startOffset, int endOffset,
boolean setNative) { boolean canResolveInsertionPoint, boolean setNative) {
assert startOffset <= endOffset; assert startOffset <= endOffset;
mEncoding = encoding; mEncoding = encoding;
mSurroundingText = surroundingText; mSurroundingText = surroundingText;
mSelectionStartOffset = startOffset; mSelectionStartOffset = startOffset;
mSelectionEndOffset = endOffset; mSelectionEndOffset = endOffset;
mCanResolveInsertionPoint = canResolveInsertionPoint;
if (startOffset == endOffset && startOffset <= surroundingText.length() if (startOffset == endOffset && startOffset <= surroundingText.length()
&& !hasAnalyzedTap()) { && !hasAnalyzedTap()) {
analyzeTap(startOffset); analyzeTap(startOffset);
...@@ -208,15 +201,6 @@ public abstract class ContextualSearchContext { ...@@ -208,15 +201,6 @@ public abstract class ContextualSearchContext {
setTranslationLanguages(getDetectedLanguage(), mTargetLanguage); setTranslationLanguages(getDetectedLanguage(), mTargetLanguage);
} }
/**
* Sets the surrounding text to just identify the current selection.
* @param selection The current selection on the base page.
*/
void setSurroundingText(WebContents basePageWebContents, String selection) {
String encoding = basePageWebContents != null ? basePageWebContents.getEncoding() : null;
setSurroundingText(encoding, selection, 0, selection.length());
}
/** /**
* @return The text that surrounds the selection, or {@code null} if none yet known. * @return The text that surrounds the selection, or {@code null} if none yet known.
*/ */
...@@ -380,14 +364,16 @@ public abstract class ContextualSearchContext { ...@@ -380,14 +364,16 @@ public abstract class ContextualSearchContext {
} }
/** /**
* @return Whether this context has a valid selection. * @return Whether this context has a valid selection, which may be an insertion point.
*/ */
@VisibleForTesting @VisibleForTesting
boolean hasValidSelection() { boolean hasValidSelection() {
return !TextUtils.isEmpty(mSurroundingText) && mSelectionStartOffset != INVALID_OFFSET boolean validSelectionAllowingInsertionPoint = !TextUtils.isEmpty(mSurroundingText)
&& mSelectionEndOffset != INVALID_OFFSET && mSelectionStartOffset != INVALID_OFFSET && mSelectionEndOffset != INVALID_OFFSET
&& mSelectionStartOffset < mSelectionEndOffset && mSelectionStartOffset <= mSelectionEndOffset
&& mSelectionEndOffset < mSurroundingText.length(); && mSelectionEndOffset <= mSurroundingText.length();
return validSelectionAllowingInsertionPoint
&& (mCanResolveInsertionPoint || mSelectionStartOffset < mSelectionEndOffset);
} }
/** /**
......
...@@ -166,7 +166,7 @@ public class ContextualSearchManager ...@@ -166,7 +166,7 @@ public class ContextualSearchManager
private long mLoadedSearchUrlTimeMs; private long mLoadedSearchUrlTimeMs;
private boolean mWereSearchResultsSeen; private boolean mWereSearchResultsSeen;
private boolean mWereInfoBarsHidden; private boolean mWereInfoBarsHidden;
private boolean mDidPromoteSearchNavigation; private int mPromoteSearchNavigationCounter;
private boolean mWasActivatedByTap; private boolean mWasActivatedByTap;
private boolean mIsInitialized; private boolean mIsInitialized;
...@@ -629,7 +629,8 @@ public class ContextualSearchManager ...@@ -629,7 +629,8 @@ public class ContextualSearchManager
if (surroundingText.length() == 0) { if (surroundingText.length() == 0) {
mInternalStateController.reset(StateChangeReason.UNKNOWN); mInternalStateController.reset(StateChangeReason.UNKNOWN);
} else { } else {
mContext.setSurroundingText(encoding, surroundingText, startOffset, endOffset); mContext.setSurroundingText(encoding, surroundingText, startOffset, endOffset,
mPolicy.isProcessingRelatedSearch());
mInternalStateController.notifyFinishedWorkOn(InternalState.GATHERING_SURROUNDINGS); mInternalStateController.notifyFinishedWorkOn(InternalState.GATHERING_SURROUNDINGS);
} }
} }
...@@ -757,9 +758,10 @@ public class ContextualSearchManager ...@@ -757,9 +758,10 @@ public class ContextualSearchManager
// TODO(donnd): Instead of preloading, we should prefetch (ie the URL should not // TODO(donnd): Instead of preloading, we should prefetch (ie the URL should not
// appear in the user's history until the user views it). See crbug.com/406446. // appear in the user's history until the user views it). See crbug.com/406446.
boolean shouldPreload = !doPreventPreload && mPolicy.shouldPrefetchSearchResult(); boolean shouldPreload = !doPreventPreload && mPolicy.shouldPrefetchSearchResult();
boolean doRequireGoogleUrl = !mPolicy.isProcessingRelatedSearch();
mSearchRequest = new ContextualSearchRequest(searchTerm, alternateTerm, mSearchRequest = new ContextualSearchRequest(searchTerm, alternateTerm,
resolvedSearchTerm.mid(), shouldPreload, resolvedSearchTerm.searchUrlFull(), resolvedSearchTerm.mid(), shouldPreload, resolvedSearchTerm.searchUrlFull(),
resolvedSearchTerm.searchUrlPreload()); resolvedSearchTerm.searchUrlPreload(), doRequireGoogleUrl);
// Trigger translation, if enabled. // Trigger translation, if enabled.
mTranslateController.forceTranslateIfNeeded( mTranslateController.forceTranslateIfNeeded(
mSearchRequest, resolvedSearchTerm.contextLanguage()); mSearchRequest, resolvedSearchTerm.contextLanguage());
...@@ -1018,7 +1020,7 @@ public class ContextualSearchManager ...@@ -1018,7 +1020,7 @@ public class ContextualSearchManager
@Override @Override
public void onContentLoadStarted(String url) { public void onContentLoadStarted(String url) {
mDidPromoteSearchNavigation = false; mPromoteSearchNavigationCounter++;
} }
@Override @Override
...@@ -1182,15 +1184,16 @@ public class ContextualSearchManager ...@@ -1182,15 +1184,16 @@ public class ContextualSearchManager
* @param url The URL we are navigating to. * @param url The URL we are navigating to.
*/ */
public void onExternalNavigation(String url) { public void onExternalNavigation(String url) {
if (!mDidPromoteSearchNavigation && !BLACKLISTED_URL.equals(url) if (mSearchPanel != null && !BLACKLISTED_URL.equals(url)
&& !url.startsWith(INTENT_URL_PREFIX) && shouldPromoteSearchNavigation() && !url.startsWith(INTENT_URL_PREFIX) && shouldPromoteSearchNavigation()
&& mSearchPanel != null) { && mPromoteSearchNavigationCounter
> mPolicy.navigateWithoutPromotionLimitForRelatedSearches()) {
// Do not promote to a regular tab if we're loading our Resolved Search // Do not promote to a regular tab if we're loading our Resolved Search
// URL, otherwise we'll promote it when prefetching the Serp. // URL, otherwise we'll promote it when prefetching the Serp.
// Don't promote URLs when they are navigating to an intent - this is // Don't promote URLs when they are navigating to an intent - this is
// handled by the InterceptNavigationDelegate which uses a faster // handled by the InterceptNavigationDelegate which uses a faster
// maximizing animation. // maximizing animation.
mDidPromoteSearchNavigation = true; mPromoteSearchNavigationCounter = 0;
mSearchPanel.maximizePanelThenPromoteToTab(StateChangeReason.SERP_NAVIGATION); mSearchPanel.maximizePanelThenPromoteToTab(StateChangeReason.SERP_NAVIGATION);
} }
} }
...@@ -1619,6 +1622,15 @@ public class ContextualSearchManager ...@@ -1619,6 +1622,15 @@ public class ContextualSearchManager
/** Starts showing the Tap UI by selecting a word around the current caret. */ /** Starts showing the Tap UI by selecting a word around the current caret. */
@Override @Override
public void startShowingTapUi() { public void startShowingTapUi() {
if (mPolicy.isProcessingRelatedSearch()) {
// Skip showing the tap-ui (selecting the word) for Related Searches.
mInternalStateController.notifyStartingWorkOn(
InternalState.START_SHOWING_TAP_UI);
mInternalStateController.notifyFinishedWorkOn(
InternalState.START_SHOWING_TAP_UI);
return;
}
WebContents baseWebContents = getBaseWebContents(); WebContents baseWebContents = getBaseWebContents();
if (baseWebContents != null) { if (baseWebContents != null) {
mInternalStateController.notifyStartingWorkOn( mInternalStateController.notifyStartingWorkOn(
...@@ -1678,6 +1690,8 @@ public class ContextualSearchManager ...@@ -1678,6 +1690,8 @@ public class ContextualSearchManager
mInternalStateController.notifyStartingWorkOn(InternalState.RESOLVING); mInternalStateController.notifyStartingWorkOn(InternalState.RESOLVING);
String selection = mSelectionController.getSelectedText(); String selection = mSelectionController.getSelectedText();
selection = mPolicy.overrideSelectionIfProcessingRelatedSearches(
selection, mContext.getWordTapped());
assert !TextUtils.isEmpty(selection); assert !TextUtils.isEmpty(selection);
mNetworkCommunicator.startSearchTermResolutionRequest( mNetworkCommunicator.startSearchTermResolutionRequest(
selection, mSelectionController.isAdjustedSelection()); selection, mSelectionController.isAdjustedSelection());
......
...@@ -114,6 +114,8 @@ class ContextualSearchPolicy { ...@@ -114,6 +114,8 @@ class ContextualSearchPolicy {
* @return Whether a Tap gesture is currently supported as a trigger for the feature. * @return Whether a Tap gesture is currently supported as a trigger for the feature.
*/ */
boolean isTapSupported() { boolean isTapSupported() {
if (isRelatedSearchesEnabled()) return true;
if (isTapDisabledDueToLongpress()) return false; if (isTapDisabledDueToLongpress()) return false;
return (!isUserUndecided() return (!isUserUndecided()
...@@ -137,7 +139,8 @@ class ContextualSearchPolicy { ...@@ -137,7 +139,8 @@ class ContextualSearchPolicy {
// We never preload on a regular long-press so users can cut & paste without hitting the // We never preload on a regular long-press so users can cut & paste without hitting the
// servers. // servers.
return mSelectionController.getSelectionType() == SelectionType.TAP return mSelectionController.getSelectionType() == SelectionType.TAP
|| mSelectionController.getSelectionType() == SelectionType.RESOLVING_LONG_PRESS; || mSelectionController.getSelectionType() == SelectionType.RESOLVING_LONG_PRESS
|| isRelatedSearchesEnabled();
} }
/** /**
...@@ -328,6 +331,9 @@ class ContextualSearchPolicy { ...@@ -328,6 +331,9 @@ class ContextualSearchPolicy {
* to see if all privacy-related conditions are met to send the base page URL. * to see if all privacy-related conditions are met to send the base page URL.
*/ */
boolean maySendBasePageUrl() { boolean maySendBasePageUrl() {
// TODO(donnd): revisit for related searches privacy review. https://crbug.com/1064141.
if (isRelatedSearchesEnabled()) return true;
return !isUserUndecided(); return !isUserUndecided();
} }
...@@ -497,6 +503,52 @@ class ContextualSearchPolicy { ...@@ -497,6 +503,52 @@ class ContextualSearchPolicy {
return url != null && UrlConstants.HTTP_SCHEME.equals(url.getProtocol()); return url != null && UrlConstants.HTTP_SCHEME.equals(url.getProtocol());
} }
// --------------------------------------------------------------------------------------------
// Related Searches Support.
// --------------------------------------------------------------------------------------------
/**
* @return Whether the experimental Feature for Related Searches is enabled.
*/
boolean isRelatedSearchesEnabled() {
return ChromeFeatureList.isEnabled(ChromeFeatureList.RELATED_SEARCHES);
}
/**
* @return Whether we're currently processing a Related Search gesture.
*/
boolean isProcessingRelatedSearch() {
return isRelatedSearchesEnabled()
&& mSelectionController.getSelectionType() == SelectionType.TAP;
}
/**
* @return The number of times a navigation in the panel can be done without promoting the
* panel into a separate tab.
*/
int navigateWithoutPromotionLimitForRelatedSearches() {
if (ChromeFeatureList.isEnabled(ChromeFeatureList.RELATED_SEARCHES)
&& mSelectionController.getSelectionType() == SelectionType.TAP) {
// For Related Searches the returned page has a list of search-result pages
// that the user can choose from, so initial navigation should be done
// without promotion. We want normal behavior for Longpress.
return 2;
}
return 1;
}
/**
* Overrides the selection if we're processing a Related Searches gesture.
* @param selection The original selection. This is returned if not processing Related
Searches.
* @param relatedSearchesWord The word to show if we are processing Related Searches.
* @return the input or an override of the selection appropriate for experiments.
*/
String overrideSelectionIfProcessingRelatedSearches(
String selection, String relatedSearchesWord) {
return isProcessingRelatedSearch() ? relatedSearchesWord : selection;
}
// -------------------------------------------------------------------------------------------- // --------------------------------------------------------------------------------------------
// Testing helpers. // Testing helpers.
// -------------------------------------------------------------------------------------------- // --------------------------------------------------------------------------------------------
......
...@@ -62,7 +62,7 @@ class ContextualSearchRequest { ...@@ -62,7 +62,7 @@ class ContextualSearchRequest {
* @param isLowPriorityEnabled Whether the request can be made at a low priority. * @param isLowPriorityEnabled Whether the request can be made at a low priority.
*/ */
ContextualSearchRequest(String searchTerm, boolean isLowPriorityEnabled) { ContextualSearchRequest(String searchTerm, boolean isLowPriorityEnabled) {
this(searchTerm, null, null, isLowPriorityEnabled, null, null); this(searchTerm, null, null, isLowPriorityEnabled, null, null, true);
} }
/** /**
...@@ -78,17 +78,18 @@ class ContextualSearchRequest { ...@@ -78,17 +78,18 @@ class ContextualSearchRequest {
* @param isLowPriorityEnabled Whether the request can be made at a low priority. * @param isLowPriorityEnabled Whether the request can be made at a low priority.
* @param searchUrlFull The URL for the full search to present in the overlay, or empty. * @param searchUrlFull The URL for the full search to present in the overlay, or empty.
* @param searchUrlPreload The URL for the search to preload into the overlay, or empty. * @param searchUrlPreload The URL for the search to preload into the overlay, or empty.
* @param doRequireGoogleUrl Whether a Google URL is required when server-supplied.
*/ */
ContextualSearchRequest(String searchTerm, @Nullable String alternateTerm, @Nullable String mid, ContextualSearchRequest(String searchTerm, @Nullable String alternateTerm, @Nullable String mid,
boolean isLowPriorityEnabled, @Nullable String searchUrlFull, boolean isLowPriorityEnabled, @Nullable String searchUrlFull,
@Nullable String searchUrlPreload) { @Nullable String searchUrlPreload, boolean doRequireGoogleUrl) {
mWasPrefetch = isLowPriorityEnabled; mWasPrefetch = isLowPriorityEnabled;
mIsFullSearchUrlProvided = isGoogleUrl(searchUrlFull); mIsFullSearchUrlProvided = !doRequireGoogleUrl || isGoogleUrl(searchUrlFull);
mNormalPriorityUri = mIsFullSearchUrlProvided mNormalPriorityUri = mIsFullSearchUrlProvided
? Uri.parse(searchUrlFull) ? Uri.parse(searchUrlFull)
: getUriTemplate(searchTerm, alternateTerm, mid, false); : getUriTemplate(searchTerm, alternateTerm, mid, false);
if (isLowPriorityEnabled) { if (isLowPriorityEnabled) {
if (isGoogleUrl(searchUrlPreload)) { if (!doRequireGoogleUrl || isGoogleUrl(searchUrlPreload)) {
mLowPriorityUri = Uri.parse(searchUrlPreload); mLowPriorityUri = Uri.parse(searchUrlPreload);
} else { } else {
Uri baseLowPriorityUri = getUriTemplate(searchTerm, alternateTerm, mid, true); Uri baseLowPriorityUri = getUriTemplate(searchTerm, alternateTerm, mid, true);
......
...@@ -22,6 +22,7 @@ import org.chromium.chrome.test.ChromeTabbedActivityTestRule; ...@@ -22,6 +22,7 @@ import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
/** /**
* Class responsible for testing the ContextualSearchRequest. * Class responsible for testing the ContextualSearchRequest.
* TODO(donnd): Switch to a pure-java test.
*/ */
@RunWith(ChromeJUnit4ClassRunner.class) @RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE}) @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
...@@ -31,18 +32,19 @@ public class ContextualSearchRequestTest { ...@@ -31,18 +32,19 @@ public class ContextualSearchRequestTest {
ContextualSearchRequest mRequest; ContextualSearchRequest mRequest;
ContextualSearchRequest mNormalPriorityOnlyRequest; ContextualSearchRequest mNormalPriorityOnlyRequest;
ContextualSearchRequest mRequestGoogleUrlRequired;
ContextualSearchRequest mRequestGoogleUrlNotRequired;
ContextualSearchRequest mRequestNonGoogleUrlRequired;
ContextualSearchRequest mRequestNonGoogleUrlNotRequired;
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
mActivityTestRule.startMainActivityOnBlankPage(); mActivityTestRule.startMainActivityOnBlankPage();
InstrumentationRegistry.getInstrumentation().runOnMainSync(new Runnable() { InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
@Override mRequest = new ContextualSearchRequest(
public void run() { "barack obama", "barack", "", true, null, null, true);
mRequest = mNormalPriorityOnlyRequest = new ContextualSearchRequest(
new ContextualSearchRequest("barack obama", "barack", "", true, null, null); "woody allen", "allen", "", false, null, null, true);
mNormalPriorityOnlyRequest =
new ContextualSearchRequest("woody allen", "allen", "", false, null, null);
}
}); });
} }
...@@ -83,14 +85,11 @@ public class ContextualSearchRequestTest { ...@@ -83,14 +85,11 @@ public class ContextualSearchRequestTest {
public void testServerProvidedUrls() { public void testServerProvidedUrls() {
String serverUrlFull = "https://www.google.com/search?obama&ctxs=2"; String serverUrlFull = "https://www.google.com/search?obama&ctxs=2";
String serverUrlPreload = "https://www.google.com/s?obama&ctxs=2&pf=c&sns=1"; String serverUrlPreload = "https://www.google.com/s?obama&ctxs=2&pf=c&sns=1";
InstrumentationRegistry.getInstrumentation().runOnMainSync(new Runnable() { InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
@Override mRequest = new ContextualSearchRequest(
public void run() { "", "", "", true, serverUrlFull, serverUrlPreload, true);
mRequest = new ContextualSearchRequest( mNormalPriorityOnlyRequest =
"", "", "", true, serverUrlFull, serverUrlPreload); new ContextualSearchRequest("", "", "", false, serverUrlFull, null, true);
mNormalPriorityOnlyRequest =
new ContextualSearchRequest("", "", "", false, serverUrlFull, null);
}
}); });
Assert.assertTrue(mRequest.isUsingLowPriority()); Assert.assertTrue(mRequest.isUsingLowPriority());
Assert.assertEquals(serverUrlPreload, mRequest.getSearchUrl()); Assert.assertEquals(serverUrlPreload, mRequest.getSearchUrl());
...@@ -100,4 +99,26 @@ public class ContextualSearchRequestTest { ...@@ -100,4 +99,26 @@ public class ContextualSearchRequestTest {
Assert.assertEquals(serverUrlFull, mRequest.getSearchUrl()); Assert.assertEquals(serverUrlFull, mRequest.getSearchUrl());
Assert.assertEquals(serverUrlFull, mNormalPriorityOnlyRequest.getSearchUrl()); Assert.assertEquals(serverUrlFull, mNormalPriorityOnlyRequest.getSearchUrl());
} }
@Test
@SmallTest
@Feature({"ContextualSearch"})
public void testDoRequireGoogleUrl() {
String someGoogleUrl = "https://www.google.com/search?obama&ctxs=2";
String someNonGoogleUrl = "https://www.wikipedia.org";
InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
mRequestGoogleUrlRequired = new ContextualSearchRequest(
"Searchterm", "", "", true, someGoogleUrl, someGoogleUrl, true);
mRequestGoogleUrlNotRequired = new ContextualSearchRequest(
"Searchterm", "", "", true, someGoogleUrl, someGoogleUrl, false);
mRequestNonGoogleUrlRequired = new ContextualSearchRequest(
"Searchterm", "", "", true, someNonGoogleUrl, someNonGoogleUrl, true);
mRequestNonGoogleUrlNotRequired = new ContextualSearchRequest(
"Searchterm", "", "", true, someNonGoogleUrl, someNonGoogleUrl, false);
});
Assert.assertEquals(someGoogleUrl, mRequestGoogleUrlRequired.getSearchUrl());
Assert.assertEquals(someGoogleUrl, mRequestGoogleUrlNotRequired.getSearchUrl());
Assert.assertNotEquals(someNonGoogleUrl, mRequestNonGoogleUrlRequired.getSearchUrl());
Assert.assertEquals(someNonGoogleUrl, mRequestNonGoogleUrlNotRequired.getSearchUrl());
}
} }
...@@ -63,12 +63,12 @@ public class ContextualSearchContextTest { ...@@ -63,12 +63,12 @@ public class ContextualSearchContextTest {
private void setupLongpressOfBarack() { private void setupLongpressOfBarack() {
int barackStartOffset = "Now ".length(); int barackStartOffset = "Now ".length();
int barackEndOffset = "Now Barack".length() - barackStartOffset; int barackEndOffset = "Now Barack".length() - barackStartOffset;
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, barackStartOffset, barackEndOffset); mContext.setSurroundingText(SAMPLE_TEXT, barackStartOffset, barackEndOffset);
} }
private void setupTapInBarack() { private void setupTapInBarack() {
int barackBeforeROffset = "Now Ba".length(); int barackBeforeROffset = "Now Ba".length();
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, barackBeforeROffset, barackBeforeROffset); mContext.setSurroundingText(SAMPLE_TEXT, barackBeforeROffset, barackBeforeROffset);
} }
private void simulateSelectWordAroundCaret(int startAdjust, int endAdjust) { private void simulateSelectWordAroundCaret(int startAdjust, int endAdjust) {
...@@ -86,7 +86,7 @@ public class ContextualSearchContextTest { ...@@ -86,7 +86,7 @@ public class ContextualSearchContextTest {
private void setupResolvingTapInObama() { private void setupResolvingTapInObama() {
int obamaBeforeMOffset = "Now Barack Oba".length(); int obamaBeforeMOffset = "Now Barack Oba".length();
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, obamaBeforeMOffset, obamaBeforeMOffset); mContext.setSurroundingText(SAMPLE_TEXT, obamaBeforeMOffset, obamaBeforeMOffset);
mContext.setResolveProperties(HOME_COUNTRY, true, 0, 0, ""); mContext.setResolveProperties(HOME_COUNTRY, true, 0, 0, "");
} }
...@@ -187,7 +187,7 @@ public class ContextualSearchContextTest { ...@@ -187,7 +187,7 @@ public class ContextualSearchContextTest {
@Feature({"ContextualSearch", "Context"}) @Feature({"ContextualSearch", "Context"})
public void testAnalysisAtStartOfText() { public void testAnalysisAtStartOfText() {
int startOffset = 0; int startOffset = 0;
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, startOffset, startOffset); mContext.setSurroundingText(SAMPLE_TEXT, startOffset, startOffset);
assertNull(mContext.getWordPreviousToTap()); assertNull(mContext.getWordPreviousToTap());
// We can't recognize the first word because we need a space before it to do so. // We can't recognize the first word because we need a space before it to do so.
assertNull(mContext.getWordTapped()); assertNull(mContext.getWordTapped());
...@@ -198,7 +198,7 @@ public class ContextualSearchContextTest { ...@@ -198,7 +198,7 @@ public class ContextualSearchContextTest {
@Feature({"ContextualSearch", "Context"}) @Feature({"ContextualSearch", "Context"})
public void testAnalysisAtSecondWordOfText() { public void testAnalysisAtSecondWordOfText() {
int secondWordOffset = "Now ".length(); int secondWordOffset = "Now ".length();
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, secondWordOffset, secondWordOffset); mContext.setSurroundingText(SAMPLE_TEXT, secondWordOffset, secondWordOffset);
assertNull(mContext.getWordPreviousToTap()); assertNull(mContext.getWordPreviousToTap());
assertEquals("Barack", mContext.getWordTapped()); assertEquals("Barack", mContext.getWordTapped());
assertEquals("Obama", mContext.getWordFollowingTap()); assertEquals("Obama", mContext.getWordFollowingTap());
...@@ -208,7 +208,7 @@ public class ContextualSearchContextTest { ...@@ -208,7 +208,7 @@ public class ContextualSearchContextTest {
@Feature({"ContextualSearch", "Context"}) @Feature({"ContextualSearch", "Context"})
public void testAnalysisAtEndOfText() { public void testAnalysisAtEndOfText() {
int endOffset = SAMPLE_TEXT.length(); int endOffset = SAMPLE_TEXT.length();
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, endOffset, endOffset); mContext.setSurroundingText(SAMPLE_TEXT, endOffset, endOffset);
assertNull(mContext.getWordPreviousToTap()); assertNull(mContext.getWordPreviousToTap());
assertNull(mContext.getWordTapped()); assertNull(mContext.getWordTapped());
assertNull(mContext.getWordFollowingTap()); assertNull(mContext.getWordFollowingTap());
...@@ -218,9 +218,17 @@ public class ContextualSearchContextTest { ...@@ -218,9 +218,17 @@ public class ContextualSearchContextTest {
@Feature({"ContextualSearch", "Context"}) @Feature({"ContextualSearch", "Context"})
public void testAnalysisAtWordBeforeEndOfText() { public void testAnalysisAtWordBeforeEndOfText() {
int wordBeforeEndOffset = SAMPLE_TEXT.length() - "s ambiguous.".length(); int wordBeforeEndOffset = SAMPLE_TEXT.length() - "s ambiguous.".length();
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, wordBeforeEndOffset, wordBeforeEndOffset); mContext.setSurroundingText(SAMPLE_TEXT, wordBeforeEndOffset, wordBeforeEndOffset);
assertEquals("Clinton", mContext.getWordPreviousToTap()); assertEquals("Clinton", mContext.getWordPreviousToTap());
assertEquals("is", mContext.getWordTapped()); assertEquals("is", mContext.getWordTapped());
assertEquals("ambiguous", mContext.getWordFollowingTap()); assertEquals("ambiguous", mContext.getWordFollowingTap());
} }
@Test
@Feature({"ContextualSearch", "Context"})
public void testAllowInsertionPointSelection() {
String sample = "sample";
mContext.setSurroundingText(UTF_8, sample, sample.length(), sample.length(), true, false);
assertTrue(mContext.hasValidSelection());
}
} }
...@@ -49,7 +49,7 @@ public class ContextualSearchEntityHeuristicTest { ...@@ -49,7 +49,7 @@ public class ContextualSearchEntityHeuristicTest {
private void setupInstanceToTest(Locale locale, int tapOffset) { private void setupInstanceToTest(Locale locale, int tapOffset) {
mContext = new ContextualSearchContext.ChangeIgnoringContext(); mContext = new ContextualSearchContext.ChangeIgnoringContext();
mContext.setSurroundingText(UTF_8, SAMPLE_TEXT, tapOffset, tapOffset); mContext.setSurroundingText(SAMPLE_TEXT, tapOffset, tapOffset);
when(mContextJniMock.detectLanguage(anyLong(), eq(mContext))) when(mContextJniMock.detectLanguage(anyLong(), eq(mContext)))
.thenReturn(locale.getLanguage()); .thenReturn(locale.getLanguage());
mEntityHeuristic = ContextualSearchEntityHeuristic.testInstance(mContext, true); mEntityHeuristic = ContextualSearchEntityHeuristic.testInstance(mContext, true);
...@@ -113,7 +113,7 @@ public class ContextualSearchEntityHeuristicTest { ...@@ -113,7 +113,7 @@ public class ContextualSearchEntityHeuristicTest {
when(mContextJniMock.detectLanguage(anyLong(), eq(context))).thenReturn(language); when(mContextJniMock.detectLanguage(anyLong(), eq(context))).thenReturn(language);
assert text.startsWith(start); assert text.startsWith(start);
int tapOffset = start.length(); int tapOffset = start.length();
context.setSurroundingText(UTF_8, text, tapOffset, tapOffset); context.setSurroundingText(text, tapOffset, tapOffset);
return ContextualSearchEntityHeuristic.testInstance(context, true); return ContextualSearchEntityHeuristic.testInstance(context, true);
} }
......
...@@ -299,7 +299,8 @@ std::string ContextualSearchDelegate::BuildRequestUrl( ...@@ -299,7 +299,8 @@ std::string ContextualSearchDelegate::BuildRequestUrl(
} }
int mainFunctionVersion = kContextualSearchRequestVersion; int mainFunctionVersion = kContextualSearchRequestVersion;
if (base::FeatureList::IsEnabled(chrome::android::kRelatedSearches)) { if (base::FeatureList::IsEnabled(chrome::android::kRelatedSearches) &&
context_->GetStartOffset() == context_->GetEndOffset()) {
mainFunctionVersion = kRelatedSearchesVersion; mainFunctionVersion = kRelatedSearchesVersion;
} }
...@@ -406,6 +407,12 @@ bool ContextualSearchDelegate::CanSendPageURL( ...@@ -406,6 +407,12 @@ bool ContextualSearchDelegate::CanSendPageURL(
if (field_trial_->IsSendBasePageURLDisabled()) if (field_trial_->IsSendBasePageURLDisabled())
return false; return false;
// TODO(donnd): privacy review needed before launch.
// See https://crbug.com/1064141.
if (base::FeatureList::IsEnabled(chrome::android::kRelatedSearches)) {
return true;
}
// Ensure that the default search provider is Google. // Ensure that the default search provider is Google.
const TemplateURL* default_search_provider = const TemplateURL* default_search_provider =
template_url_service->GetDefaultSearchProvider(); template_url_service->GetDefaultSearchProvider();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment