[Contextual Search] Trigger the translation one-box.

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRequest.java

Index: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRequest.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRequest.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRequest.java
index 0b9bb03e4a97cc05a4fa02d4237311d9f2216a24..dc74a3f27f792d1e3e8d9e80f9d792601f785e48 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRequest.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRequest.java
@@ -5,7 +5,9 @@
 package org.chromium.chrome.browser.contextualsearch;
 
 import android.net.Uri;
+import android.text.TextUtils;
 
+import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService;
 
 import java.net.MalformedURLException;
@@ -27,8 +29,16 @@ class ContextualSearchRequest {
     private boolean mIsLowPriority;
     private boolean mHasFailedLowPriorityLoad;
 
+    private static final String GWS_LOW_PRIORITY_SEARCH_PATH = "s";
+    private static final String GWS_SEARCH_NO_SUGGESTIONS_PARAM = "s";

[pedro (no code reviews), 2015/09/23 18:33:54]

Isn't this supposed to be "sns"?

[Donn Denman, 2015/10/09 22:08:23]

Thanks for the catch!  Done.

+    private static final String GWS_SEARCH_NO_SUGGESTIONS_PARAM_VALUE = "1";
+    private static final String GWS_QUERY_PARAM = "q";
     private static final String CTXS_PARAM_PATTERN = "(ctxs=[^&]+)";
     private static final String CTXR_PARAM = "ctxr";
+    @VisibleForTesting
+    static final String TLITE_SOURCE_LANGUAGE_PARAM = "tlitesl";
+    private static final String TLITE_TARGET_LANGUAGE_PARAM = "tlitetl";
+    private static final String TLITE_QUERY_PARAM = "tlitetxt";
 
     /**
      * Creates a search request for the given search term without any alternate term and
@@ -45,18 +55,20 @@ class ContextualSearchRequest {
      * @param searchTerm The resolved search term.
      * @param alternateTerm The alternate search term.
      * @param isLowPriorityEnabled Whether the request can be made at a low priority.
+     * @param needsTranslation Whether translation is needed between the given languages.
+     * @param sourceLanguage The language of the original search term.
+     * @param targetLanguage The language the that the user prefers.
      */
     ContextualSearchRequest(String searchTerm, @Nullable String alternateTerm,
-            boolean isLowPriorityEnabled) {
+            boolean isLowPriorityEnabled, boolean needsTranslation, @Nullable String sourceLanguage,

[pedro (no code reviews), 2015/09/23 18:33:54]

Nit: Instead of passing the extra parameters here in the constructor, I think it
would be clearer passing them through a new method, like
forceTranslationResult(String sourceLanguage, String targetLanguage), since
parameters in the constructor are usually the ones that are always used/needed
for the purpose of the class. In this case, translation is not something that
will always happen.

[Donn Denman, 2015/10/09 22:08:23]

Done.

+            @Nullable String targetLanguage) {
         mWasPrefetch = isLowPriorityEnabled;
-        mNormalPriorityUri = getUriTemplate(searchTerm, alternateTerm, false);
+        mNormalPriorityUri = getSearchUri(
+                searchTerm, alternateTerm, false, needsTranslation, sourceLanguage, targetLanguage);

[pedro (no code reviews), 2015/09/23 18:33:54]

Nit: Also, I think we should store the parameters in class properties and use
them later in the private methods. This would prevent having to pass them over
and over, but also, it feels right. It's kinda strange that this class doesn't
remember what the search term was, for example.

[Donn Denman, 2015/10/09 22:08:23]

You mean remember the parameters instead of modifying the existing URI, or
building multiple URIs, one for low and one for normal priority?

[pedro (no code reviews), 2015/10/23 08:50:35]

I meant that it feels weird that this class doesn't remember what the search
term was, given this class should be the authority about the search request. But
it's OK, is just something to keep in mind. Maybe we can improve this in our
refactoring.

         if (isLowPriorityEnabled) {
-            // TODO(donnd): Call TemplateURL once we have an API for 3rd-party providers.
-            Uri baseLowPriorityUri = getUriTemplate(searchTerm, alternateTerm, true);
-            mLowPriorityUri = baseLowPriorityUri.buildUpon()
-                .path("s")
-                .appendQueryParameter("sns", "1")
-                .build();
+            Uri baseLowPriorityUri = getSearchUri(searchTerm, alternateTerm, true, needsTranslation,
+                    sourceLanguage, targetLanguage);
+            mLowPriorityUri = makeLowPriorityUri(baseLowPriorityUri);
             mIsLowPriority = true;
         } else {
             mIsLowPriority = false;
@@ -65,6 +77,18 @@ class ContextualSearchRequest {
     }
 
     /**
+     * Creates a search request for the given search term with the given alternate term and
+     * low-priority loading capability.
+     * @param searchTerm The resolved search term.
+     * @param alternateTerm The alternate search term.
+     * @param isLowPriorityEnabled Whether the request can be made at a low priority.
+     */
+    ContextualSearchRequest(
+            String searchTerm, @Nullable String alternateTerm, boolean isLowPriorityEnabled) {
+        this(searchTerm, alternateTerm, isLowPriorityEnabled, false, null, null);
+    }
+
+    /**
      * Sets an indicator that the normal-priority URL should be used for this search request.
      */
     void setNormalPriority() {
@@ -140,18 +164,87 @@ class ContextualSearchRequest {
     }
 
     /**
+     * Gets the Uri to use for a Search request.  Adds additional parameters for the special
+     * features triggered by Contextual Search.
+     * @param query The search term to use as the main query in the returned search url.
+     * @param alternateTerm The alternate search term to use as an alternate suggestion.
+     * @param shouldPrefetch Whether the returned url should include a prefetch parameter.
+     * @param needsTranslation Whether translation is needed between the given languages.
+     * @param sourceLanguage The language of the original search term.
+     * @param targetLanguage The language the that the user prefers.
+     * @return A {@link Uri} that access the default search engine with Contextual
+     *         Search and associated parameters appropriately set.
+     */
+    private Uri getSearchUri(String query, @Nullable String alternateTerm, boolean shouldPrefetch,
+            boolean needsTranslation, String sourceLanguage, String targetLanguage) {
+        Uri resultUri = getUriTemplate(query, alternateTerm, shouldPrefetch);
+        if (needsTranslation) {
+            resultUri = makeTranslateUri(resultUri, sourceLanguage, targetLanguage);
+        }
+        return resultUri;
+    }
+
+    /**
      * Uses TemplateUrlService to generate the url for the given query
      * {@link String} for {@code query} with the contextual search version param set.
      * @param query The search term to use as the main query in the returned search url.
      * @param alternateTerm The alternate search term to use as an alternate suggestion.
      * @param shouldPrefetch Whether the returned url should include a prefetch parameter.
-     * @return      A {@link String} that contains the url of the default search engine with
-     *              {@code query} and {@code alternateTerm} inserted as parameters and contextual
-     *              search and prefetch parameters conditionally set.
+     * @return A {@link Uri} that contains the url of the default search engine with
+     *         {@code query} and {@code alternateTerm} inserted as parameters and contextual
+     *         search and prefetch parameters conditionally set.
      */
     private Uri getUriTemplate(String query, @Nullable String alternateTerm,
             boolean shouldPrefetch) {
         return Uri.parse(TemplateUrlService.getInstance().getUrlForContextualSearchQuery(
                 query, alternateTerm, shouldPrefetch));
     }
+
+    /**
+     * @return a low-priority {@code Uri} from the given base {@code Uri}.
+     */
+    private Uri makeLowPriorityUri(Uri baseUri) {
+        return baseUri.buildUpon()
+                .path(GWS_LOW_PRIORITY_SEARCH_PATH)
+                .appendQueryParameter(
+                        GWS_SEARCH_NO_SUGGESTIONS_PARAM, GWS_SEARCH_NO_SUGGESTIONS_PARAM_VALUE)
+                .build();
+    }
+
+    /**
+     * Makes the given {@code Uri} into a similar Uri that triggers a Translate one-box.
+     * @param baseUri The base Uri to build off of.
+     * @param sourceLanguage The language of the original search term.
+     * @param targetLanguage The language the that the user prefers.
+     * @return A {@link Uri} that has additional parameters for Translate appropriately set.
+     */
+    private Uri makeTranslateUri(Uri baseUri, String sourceLanguage, String targetLanguage) {
+        assert !sourceLanguage.isEmpty() && !targetLanguage.isEmpty();
+        // We must replace the q= param, and there seems to be no good way other than clearing
+        // all the query params and adding them all back in, changing q=.
+        Uri.Builder builder = baseUri.buildUpon().clearQuery();
+        String query = null;
+        for (String param : baseUri.getQueryParameterNames()) {
+            String value = baseUri.getQueryParameter(param);
+            if (TextUtils.equals(param, GWS_QUERY_PARAM)) {
+                query = value;
+                value = getTranslateQuery();
+            }
+            builder.appendQueryParameter(param, value);
+        }
+        Uri result = builder.appendQueryParameter(TLITE_SOURCE_LANGUAGE_PARAM, sourceLanguage)
+                             .appendQueryParameter(TLITE_TARGET_LANGUAGE_PARAM, targetLanguage)
+                             .appendQueryParameter(TLITE_QUERY_PARAM, query)
+                             .build();
+        System.out.println("ctxs Uri: " + result);

[pedro (no code reviews), 2015/09/23 18:33:55]

Remove println() call

[Donn Denman, 2015/10/09 22:08:23]

Done.

+        return result;
+    }
+
+    /**
+     * TODO(donnd): This translate API seems wrong to me, hopefully we'll fix it!
+     * For now, this is just place-holder code that will only work to translate to English.
+     */
+    private String getTranslateQuery() {
+        return "Translate";
+    }
 }