Trigger translate immediately when the 'Translate...' button is pressed in the overflow menu.

This also sets mUserInteracted = true in a couple places where it was not being set previously. I believe the intention of the bool and the INFOBAR_DECLINE metric is to track the number of immediate closes of the infobar, but please let me know if this isn't the case.

Bug: 1123180
Change-Id: Ia8bd867575b7d242554cec8336a31aa5ead714e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388777
Commit-Queue: Josh Simmons <jds@google.com>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Megan Jablonski <megjablon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806410}

chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java

@@ -229,7 +229,9 @@ public class TranslateCompactInfoBar extends InfoBar
             // Set translating status in the beginning for pages translated automatically.
             mTabLayout.getTabAt(TARGET_TAB_INDEX).select();
             mTabLayout.showProgressBarOnTab(TARGET_TAB_INDEX);
-            mUserInteracted = true;
+            if (mOptions.triggeredFromMenu()) {
+                mUserInteracted = true;
+            }
         } else if (mInitialStep == AFTER_TRANSLATING_INFOBAR) {
             // Focus on target tab since we are after translation.
             mTabLayout.getTabAt(TARGET_TAB_INDEX).select();
@@ -298,14 +300,34 @@ public class TranslateCompactInfoBar extends InfoBar
         }
     }
 
-    private void startTranslating(int tabPosition) {
-        if (TARGET_TAB_INDEX == tabPosition) {
-            // Already on the target tab.
+    /**
+     * Selects the tab at tabIndex without triggering onTabSelected. This avoids treating the
+     * selection as a user interaction and prevents a potential loop where a translate state change
+     * updates the UI, which then updates the translate state.
+     */
+    private void silentlySelectTabAt(int tabIndex) {
+        if (mTabLayout == null) {
+            return;
+        }
+
+        mTabLayout.removeOnTabSelectedListener(this);
+        mTabLayout.getTabAt(tabIndex).select();
+        mTabLayout.addOnTabSelectedListener(this);
+    }
+
+    /**
+     * Begins the translation process and marks it as initiated by the user.
+     */
+    private void startUserInitiatedTranslation() {
+        mUserInteracted = true;
+        onButtonClicked(ActionType.TRANSLATE);
+    }
+
+    @CalledByNative
+    private void onTranslating() {
+        if (mTabLayout != null) {
+            silentlySelectTabAt(TARGET_TAB_INDEX);
             mTabLayout.showProgressBarOnTab(TARGET_TAB_INDEX);
-            onButtonClicked(ActionType.TRANSLATE);
-            mUserInteracted = true;
-        } else {
-            mTabLayout.getTabAt(TARGET_TAB_INDEX).select();
         }
     }
 
@@ -319,11 +341,7 @@ public class TranslateCompactInfoBar extends InfoBar
                 Toast.makeText(getContext(), R.string.translate_infobar_error, Toast.LENGTH_SHORT)
                         .show();
                 errorUIShown = true;
-                // Disable OnTabSelectedListener then revert selection.
-                mTabLayout.removeOnTabSelectedListener(this);
-                mTabLayout.getTabAt(SOURCE_TAB_INDEX).select();
-                // Add OnTabSelectedListener back.
-                mTabLayout.addOnTabSelectedListener(this);
+                silentlySelectTabAt(SOURCE_TAB_INDEX);
             }
         }
         return errorUIShown;
@@ -382,13 +400,14 @@ public class TranslateCompactInfoBar extends InfoBar
             case SOURCE_TAB_INDEX:
                 incrementAndRecordTranslationsPerPageCount();
                 recordInfobarAction(INFOBAR_REVERT);
+                mUserInteracted = true;
                 onButtonClicked(ActionType.TRANSLATE_SHOW_ORIGINAL);
                 return;
             case TARGET_TAB_INDEX:
                 recordInfobarAction(INFOBAR_TARGET_TAB_TRANSLATE);
                 recordInfobarLanguageData(
                         INFOBAR_HISTOGRAM_TRANSLATE_LANGUAGE, mOptions.targetLanguageCode());
-                startTranslating(TARGET_TAB_INDEX);
+                startUserInitiatedTranslation();
                 return;
             default:
                 assert false : "Unexpected Tab Index";
@@ -460,7 +479,7 @@ public class TranslateCompactInfoBar extends InfoBar
                     TranslateCompactInfoBar.this, TranslateOption.TARGET_CODE, code);
             // Adjust UI.
             mTabLayout.replaceTabTitle(TARGET_TAB_INDEX, mOptions.getRepresentationFromCode(code));
-            startTranslating(mTabLayout.getSelectedTabPosition());
+            startUserInitiatedTranslation();
         }
     }
 
@@ -474,7 +493,7 @@ public class TranslateCompactInfoBar extends InfoBar
                     TranslateCompactInfoBar.this, TranslateOption.SOURCE_CODE, code);
             // Adjust UI.
             mTabLayout.replaceTabTitle(SOURCE_TAB_INDEX, mOptions.getRepresentationFromCode(code));
-            startTranslating(mTabLayout.getSelectedTabPosition());
+            startUserInitiatedTranslation();
         }
     }
 
@@ -585,20 +604,22 @@ public class TranslateCompactInfoBar extends InfoBar
 
         switch (actionId) {
             case ACTION_OVERFLOW_ALWAYS_TRANSLATE:
+                mUserInteracted = true;
                 toggleAlwaysTranslate();
                 // Start translating if always translate is selected and if page is not already
                 // translated to the target language.
                 if (mOptions.getTranslateState(TranslateOptions.Type.ALWAYS_LANGUAGE)
                         && mTabLayout.getSelectedTabPosition() == SOURCE_TAB_INDEX) {
-                    startTranslating(mTabLayout.getSelectedTabPosition());
+                    startUserInitiatedTranslation();
                 }
                 return;
             case ACTION_AUTO_ALWAYS_TRANSLATE:
                 toggleAlwaysTranslate();
                 return;
             case ACTION_OVERFLOW_NEVER_LANGUAGE:
-            case ACTION_AUTO_NEVER_LANGUAGE:
                 mUserInteracted = true;
+                // Fallthrough intentional.
+            case ACTION_AUTO_NEVER_LANGUAGE:
                 // After applying this option, the infobar will dismiss.
                 TranslateCompactInfoBarJni.get().applyBoolTranslateOption(
                         mNativeTranslateInfoBarPtr, TranslateCompactInfoBar.this,

chrome/android/javatests/src/org/chromium/chrome/browser/translate/TranslateCompactInfoBarTest.java

@@ -48,6 +48,7 @@ public class TranslateCompactInfoBarTest {
             new ChromeActivityTestRule<>(ChromeActivity.class);
 
     private static final String TRANSLATE_PAGE = "/chrome/test/data/translate/fr_test.html";
+    private static final String NON_TRANSLATE_PAGE = "/chrome/test/data/android/simple.html";
 
     private InfoBarContainer mInfoBarContainer;
     private InfoBarTestAnimationListener mListener;
@@ -166,4 +167,64 @@ public class TranslateCompactInfoBarTest {
         Assert.assertFalse(infoBar.isSourceTabSelectedForTesting());
         Assert.assertTrue(infoBar.isTargetTabSelectedForTesting());
     }
+
+    /**
+     * Test that translation starts automatically when "Translate..." is pressed in the menu.
+     */
+    @Test
+    @MediumTest
+    @Feature({"Browser", "Main"})
+    @Restriction(ChromeRestriction.RESTRICTION_TYPE_GOOGLE_PLAY_SERVICES)
+    public void testStartTranslateOnManualInitiation() throws TimeoutException {
+        // Load a page that won't trigger the translate recommendation.
+        mActivityTestRule.loadUrl(mTestServer.getURL(NON_TRANSLATE_PAGE));
+
+        Assert.assertTrue(mInfoBarContainer.getInfoBarsForTesting().isEmpty());
+
+        // Invoke bar by clicking the manual translate button.
+        MenuUtils.invokeCustomMenuActionSync(InstrumentationRegistry.getInstrumentation(),
+                mActivityTestRule.getActivity(), R.id.translate_id);
+        mListener.addInfoBarAnimationFinished("InfoBar not opened.");
+
+        TranslateCompactInfoBar infoBar =
+                (TranslateCompactInfoBar) mInfoBarContainer.getInfoBarsForTesting().get(0);
+
+        // Only the target tab is selected.
+        Assert.assertFalse(infoBar.isSourceTabSelectedForTesting());
+        Assert.assertTrue(infoBar.isTargetTabSelectedForTesting());
+    }
+
+    /**
+     * Test that pressing "Translate..." will start a translation even if the infobar is visible.
+     */
+    @Test
+    @MediumTest
+    @Feature({"Browser", "Main"})
+    @Restriction(ChromeRestriction.RESTRICTION_TYPE_GOOGLE_PLAY_SERVICES)
+    public void testManualInitiationWithBarOpen() throws TimeoutException {
+        mActivityTestRule.loadUrl(mTestServer.getURL(TRANSLATE_PAGE));
+        mListener.addInfoBarAnimationFinished("InfoBar not opened.");
+
+        TranslateCompactInfoBar infoBar =
+                (TranslateCompactInfoBar) mInfoBarContainer.getInfoBarsForTesting().get(0);
+
+        // Only the source tab is selected.
+        Assert.assertTrue(infoBar.isSourceTabSelectedForTesting());
+        Assert.assertFalse(infoBar.isTargetTabSelectedForTesting());
+
+        MenuUtils.invokeCustomMenuActionSync(InstrumentationRegistry.getInstrumentation(),
+                mActivityTestRule.getActivity(), R.id.translate_id);
+
+        // Only the target tab is selected.
+        Assert.assertFalse(infoBar.isSourceTabSelectedForTesting());
+        Assert.assertTrue(infoBar.isTargetTabSelectedForTesting());
+
+        // Verify that hitting "Translate..." again doesn't revert the translation.
+        MenuUtils.invokeCustomMenuActionSync(InstrumentationRegistry.getInstrumentation(),
+                mActivityTestRule.getActivity(), R.id.translate_id);
+
+        // Only the target tab is selected.
+        Assert.assertFalse(infoBar.isSourceTabSelectedForTesting());
+        Assert.assertTrue(infoBar.isTargetTabSelectedForTesting());
+    }
 }

chrome/browser/translate/chrome_translate_client.cc

@@ -245,7 +245,6 @@ bool ChromeTranslateClient::ShowTranslateUI(
       InfoBarService::FromWebContents(web_contents()),
       web_contents()->GetBrowserContext()->IsOffTheRecord(), step,
       source_language, target_language, error_type, triggered_from_menu);
-  return true;
 #else
   DCHECK(TranslateService::IsTranslateBubbleEnabled());
   // Bubble UI.
@@ -299,7 +298,7 @@ void ChromeTranslateClient::ManualTranslateWhenReady() {
     manual_translate_on_ready_ = true;
   } else {
     translate::TranslateManager* manager = GetTranslateManager();
-    manager->InitiateManualTranslation();
+    manager->InitiateManualTranslation(true);
   }
 }
 #endif
@@ -356,7 +355,7 @@ void ChromeTranslateClient::OnLanguageDetermined(
 #if defined(OS_ANDROID)
   // See ChromeTranslateClient::ManualTranslateOnReady
   if (manual_translate_on_ready_) {
-    GetTranslateManager()->InitiateManualTranslation();
+    GetTranslateManager()->InitiateManualTranslation(true);
     manual_translate_on_ready_ = false;
   }
 #endif

chrome/browser/ui/android/infobars/translate_compact_infobar.cc

@@ -229,6 +229,9 @@ void TranslateCompactInfoBar::OnTranslateStepChanged(
     if (error_ui_shown) {
       GetDelegate()->OnErrorShown(error_type);
     }
+  } else if (step == translate::TRANSLATE_STEP_TRANSLATING) {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_TranslateCompactInfoBar_onTranslating(env, GetJavaInfoBar());
   }
 }