[Contextual Search] Trigger the translation one-box.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
index 6ee72c4d7ddf173911a4f9a53bb5cd7cdf9612ab..26ee663ac8795d4286ba9de125395c86bed980d9 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.contextualsearch;
 
 import android.app.Activity;
+import android.content.Context;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewTreeObserver;
@@ -49,10 +50,16 @@ import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.NavigationEntry;
 import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.content_public.common.TopControlsState;
+import org.chromium.ui.UiUtils;
 import org.chromium.ui.base.WindowAndroid;
 
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
 
 import javax.annotation.Nullable;
 
@@ -492,6 +499,7 @@ public class ContextualSearchManager extends ContextualSearchObservable
             boolean shouldPrefetch = mPolicy.shouldPrefetchSearchResult(isTap);
             mSearchRequest = new ContextualSearchRequest(mSelectionController.getSelectedText(),
                     null, shouldPrefetch);
+            // TODO(donnd): figure out a way to do translation on long-press selections.
             mDidStartLoadingResolvedSearchRequest = false;
             mSearchPanelDelegate.displaySearchTerm(mSelectionController.getSelectedText());
             if (shouldPrefetch) loadSearchUrl();
@@ -689,20 +697,22 @@ public class ContextualSearchManager extends ContextualSearchObservable
      *        selection should be expanded by.
      * @param selectionEndAdjust A positive number of characters that the end of the existing
      *        selection should be expanded by.
+     * @param contextLanguage The language of the original search term, or an empty string.
      */
     @CalledByNative
     public void onSearchTermResolutionResponse(boolean isNetworkUnavailable, int responseCode,
             final String searchTerm, final String displayText, final String alternateTerm,
-            boolean doPreventPreload, int selectionStartAdjust, int selectionEndAdjust) {
+            boolean doPreventPreload, int selectionStartAdjust, int selectionEndAdjust,
+            final String contextLanguage) {
         mNetworkCommunicator.handleSearchTermResolutionResponse(isNetworkUnavailable, responseCode,
                 searchTerm, displayText, alternateTerm, doPreventPreload, selectionStartAdjust,
-                selectionEndAdjust);
+                selectionEndAdjust, contextLanguage);
     }
 
     @Override
     public void handleSearchTermResolutionResponse(boolean isNetworkUnavailable, int responseCode,
             String searchTerm, String displayText, String alternateTerm, boolean doPreventPreload,
-            int selectionStartAdjust, int selectionEndAdjust) {
+            int selectionStartAdjust, int selectionEndAdjust, String contextLanguage) {
         if (!mSearchPanelDelegate.isShowing()) return;
 
         // Show an appropriate message for what to search for.
@@ -735,6 +745,14 @@ public class ContextualSearchManager extends ContextualSearchObservable
             // appear in the user's history until the user views it).  See crbug.com/406446.
             boolean shouldPreload = !doPreventPreload && mPolicy.shouldPrefetchSearchResult(true);
             mSearchRequest = new ContextualSearchRequest(searchTerm, alternateTerm, shouldPreload);
+            // Trigger translation, if enabled.
+            if (!contextLanguage.isEmpty() && mPolicy.isTranslationEnabled()) {
+                List<String> targetLanguages = getTargetLanguages();

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

We should cache the targetLanguages to prevent calling getTargetLanguages()
every time. I wouldn't cache it here either, since this method might be called
during animations. Maybe cache during the Manager's initialize() method?

We should also check mPolicy.isTranslationEnabled() and only cache it when
necessary.

[Donn Denman, 2015/10/28 22:17:15]

I think we should call getTargetLanguages every time, since the user can respond
to an infobar at any time that says "don't ever translate" some language and
that's reflected in the accept-headers and the target languages.
Already done -- all isTranslationEnabled does is check the fieldtrial, which
caches.

[pedro (no code reviews), 2015/10/30 00:58:42]

My point is that getTargetLanguages() makes a 2 JNI calls and the user changing
the translation preferences should happen only once per language the user
speaks, which pretty rare.

The mSearchPanelDelegate.onSearchTermResolutionResponse(message) call above will
start the resolution animation and will most likely cause a view to be inflated
(unless the bar was already peeking).

Besides that, the icon and/or peeking animations might be happening at the same
time, which makes things even worse (less time for each animation to process its
changes).

We could cache it when right after requesting a resolution, for example. This
way when the resolution is done we would would have it ready.

[Donn Denman, 2015/11/03 01:06:26]

Done, but for the record, I think this is premature optimization.

Having said that, I don't actually know how expensive those JNI calls are, and
caching as you suggest doesn't seem too bad.  I agree that the user doesn't
choose new languages to not translate very often.  However, to nit pick your
comment, it could be more often than the languages they *do* speak (could be
dozens).

+                if (mPolicy.needsTranslation(contextLanguage, targetLanguages)) {
+                    mSearchRequest.forceTranslation(
+                            contextLanguage, mPolicy.bestTargetLanguage(targetLanguages));
+                }
+            }
             mDidStartLoadingResolvedSearchRequest = false;
             if (mSearchPanelDelegate.isContentViewShowing()) {
                 mSearchRequest.setNormalPriority();
@@ -782,6 +800,63 @@ public class ContextualSearchManager extends ContextualSearchObservable
     }
 
     // ============================================================================================
+    // Translation support
+    // ============================================================================================
+
+    /**
+     * Gets the list of target languages for the current user, with the first
+     * item in the list being the user's primary language.
+     * @return The {@link List} of languages the user understands with their primary language first.
+     */
+    private List<String> getTargetLanguages() {

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

This method makes 2 JNI calls so we should cache its return value to avoid
making those calls every time.

[Donn Denman, 2015/10/28 22:17:15]

I think caching the target language is fine since it seldom changes.  So I added
a method here to cache that, and removed the caching from the native code
called.

However, As I said above, I don't think we should cache the accept-languages,
since the user can easily change this by responding to an infobar to never
translate <x>.

[pedro (no code reviews), 2015/10/30 00:58:42]

See comment above.

[Donn Denman, 2015/11/03 01:06:26]

Done.

+        // Using LinkedHashSet keeps the entries both unique and ordered.
+        Set<String> uniqueLanguages = new LinkedHashSet<String>();
+
+        // The primary language always comes first.
+        uniqueLanguages.add(
+                trimLocaleToLanguage(nativeGetTargetLanguage(mNativeContextualSearchManagerPtr)));
+
+        // Next add languages the user knows how to type.
+        Context context = mSearchPanelDelegate.getContext();

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

Can't we use mWindowAndroid.getApplicationContext() instead?

[Donn Denman, 2015/10/28 22:17:15]

Done.  Thanks!

+        List<String> locales = context != null
+                ? new ArrayList<String>(UiUtils.getIMELocales(context))
+                : new ArrayList<String>();
+        for (int i = 0; i < locales.size(); i++) {
+            uniqueLanguages.add(trimLocaleToLanguage(locales.get(i)));
+        }
+
+        // Add the accept languages last, since they are a weaker hint than presence of a keyboard.
+        List<String> acceptLanguages = getAcceptLanguages();
+        for (int i = 0; i < acceptLanguages.size(); i++) {
+            uniqueLanguages.add(trimLocaleToLanguage(acceptLanguages.get(i)));
+        }
+        return new ArrayList<String>(uniqueLanguages);
+    }
+
+    /**
+     * Gets the list of accept languages for this user.
+     * @return The {@link List} of languages the user understands or does not want translated.
+     */
+    private List<String> getAcceptLanguages() {
+        String acceptLanguages = nativeGetAcceptLanguages(mNativeContextualSearchManagerPtr);
+        List<String> result = new ArrayList<String>();
+        for (String language : acceptLanguages.split(",")) {
+            result.add(language);
+        }
+        return result;
+    }
+
+    /**
+     * @return The given locale as a language code.
+     */
+    private String trimLocaleToLanguage(String locale) {
+        String trimmedLocale = locale.substring(0, 2);

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

Do we really need to trim it? I think
Locale.forLanguageTag(locale).getLanguage() should return just the language
part, excluding the country/region part.

Locale.forLanguageTag() expects the code with "-", not "_". If the locale string
uses underscore, you would have to replace them. Still, I think it's better
because it doesn't assume the size of the language part of the code (here
assumed to be 2).

http://docs.oracle.com/javase/7/docs/api/java/util/Locale.html#def_language

[Donn Denman, 2015/11/03 01:06:25]

Unfortunately this seems to use API 21, and we're using a lower API currently.

I will need to land another CL when the translation API changes, so we should
get someone more translation-savvy to review all my TODOs then, e.g. Newt.

+        // TODO(donnd): use getScript instead of getLanguage to trim down to the two character
+        // language code?
+        return new Locale(trimmedLocale).getLanguage();
+    }
+
+    // ============================================================================================
     // OverlayContentDelegate
     // ============================================================================================
 
@@ -1228,4 +1303,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
     private native void nativeGatherSurroundingText(long nativeContextualSearchManager,
             String selection, boolean useResolvedSearchTerm, ContentViewCore baseContentViewCore,
             boolean maySendBasePageUrl);
+    private native String nativeGetTargetLanguage(long nativeContextualSearchManager);
+    private native String nativeGetAcceptLanguages(long nativeContextualSearchManager);
 }