Few TranslateManager changes:

- Always show a "translating..." infobar when initiating a translation from the context menu, or when the translation is automatic (always translate option). It does not make sense not to show one, as translation may take several seconds and no having any feedback during that time is confusing (also this is what translate in toolbar does).
- Don't enable the translate context menu until we get the page language. This is an effort to ensure the translate infobar delegate always get an original language.
- Makes the translate manager deals correctly with unknown languages to avoid a crasher (bug 49018)

BUG=49018
TEST=See bug. And also, start a translation from the context menu, while the page is being translated a "translating..." infobar should be shown. Also, tests that when a language was selected for "always translate", navigating to a page in that language triggers a "translating..." infobar.

Review URL: http://codereview.chromium.org/3026002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@53384 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 50 insertions, 22 deletions

diff --git a/chrome/browser/translate/translate_infobar_delegate.cc b/chrome/browser/translate/translate_infobar_delegate.cc
index e1a801b..0a78e8f 100644
--- a/chrome/browser/translate/translate_infobar_delegate.cc
+++ b/chrome/browser/translate/translate_infobar_delegate.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/translate/translate_infobar_view.h"
 #include "chrome/browser/translate/translate_manager.h"
+#include "chrome/common/chrome_constants.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 
@@ -24,7 +25,13 @@ TranslateInfoBarDelegate* TranslateInfoBarDelegate::CreateDelegate(
     const std::string& original_language,
     const std::string& target_language) {
   DCHECK(type != TRANSLATION_ERROR);
-  if (!TranslateManager::IsSupportedLanguage(original_language) ||
+  // The original language can only be "unknown" for the "translating"
+  // infobar, which is the case when the user started a translation from the
+  // context menu.
+  DCHECK(type == TRANSLATING ||
+      original_language != chrome::kUnknownLanguageCode);
+  if ((original_language != chrome::kUnknownLanguageCode &&
+          !TranslateManager::IsSupportedLanguage(original_language)) ||
       !TranslateManager::IsSupportedLanguage(target_language)) {
     return NULL;
   }
@@ -32,7 +39,6 @@ TranslateInfoBarDelegate* TranslateInfoBarDelegate::CreateDelegate(
       new TranslateInfoBarDelegate(type, TranslateErrors::NONE,
                                    tab_contents,
                                    original_language, target_language);
-  DCHECK(delegate->original_language_index() != -1);
   DCHECK(delegate->target_language_index() != -1);
   return delegate;
 }
@@ -111,6 +117,8 @@ string16 TranslateInfoBarDelegate::GetLanguageDisplayableNameAt(
 }
 
 std::string TranslateInfoBarDelegate::GetOriginalLanguageCode() const {
+  if (original_language_index() == -1)
+    return chrome::kUnknownLanguageCode;
   return GetLanguageCodeAt(original_language_index());
 }
 
diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index 48b3079..7ce63f6 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -403,16 +403,10 @@ void TranslateManager::TranslatePage(TabContents* tab_contents,
     return;
   }
 
-  TranslateInfoBarDelegate* infobar = GetTranslateInfoBarDelegate(tab_contents);
-  if (infobar) {
-    // We don't show the translating infobar if no translate infobar is already
-    // showing (that is the case when the translation was triggered by the
-    // "always translate" for example).
-    infobar = TranslateInfoBarDelegate::CreateDelegate(
-        TranslateInfoBarDelegate::TRANSLATING, tab_contents,
-        source_lang, target_lang);
-    ShowInfoBar(tab_contents, infobar);
-  }
+  TranslateInfoBarDelegate* infobar = TranslateInfoBarDelegate::CreateDelegate(
+      TranslateInfoBarDelegate::TRANSLATING, tab_contents,
+      source_lang, target_lang);
+  ShowInfoBar(tab_contents, infobar);
 
   if (!translate_script_.empty()) {
     DoTranslatePage(tab_contents, translate_script_, source_lang, target_lang);
diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc
index 1b1598d..227d837 100644
--- a/chrome/browser/translate/translate_manager_unittest.cc
+++ b/chrome/browser/translate/translate_manager_unittest.cc
@@ -384,7 +384,15 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) {
       TestRenderViewContextMenu::CreateContextMenu(contents()));
   menu->Init();
   menu->ExecuteCommand(IDC_CONTENT_CONTEXT_TRANSLATE);
-  SimulateURLFetch(true);  // Simulate receiving the translate script.
+
+  // To test that bug #49018 if fixed, make sure we deal correctly with errors.
+  SimulateURLFetch(false);  // Simulate a failure to fetch the translate script.
+  TranslateInfoBarDelegate* infobar = GetTranslateInfoBar();
+  ASSERT_TRUE(infobar != NULL);
+  EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATION_ERROR, infobar->type());
+  EXPECT_TRUE(infobar->IsError());
+  infobar->MessageInfoBarButtonPressed();
+  SimulateURLFetch(true);  // This time succeed.
 
   // Simulate the render notifying the translation has been done, the server
   // having detected the page was in a known and supported language.
@@ -392,7 +400,7 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) {
       TranslateErrors::NONE));
 
   // The after translate infobar should be showing.
-  TranslateInfoBarDelegate* infobar = GetTranslateInfoBar();
+  infobar = GetTranslateInfoBar();
   ASSERT_TRUE(infobar != NULL);
   EXPECT_EQ(TranslateInfoBarDelegate::AFTER_TRANSLATE, infobar->type());
   EXPECT_EQ("fr", infobar->GetOriginalLanguageCode());
@@ -912,6 +920,12 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
   SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true);
 
   // It should have triggered an automatic translation to English.
+
+  // The translating infobar should be showing.
+  TranslateInfoBarDelegate* infobar = GetTranslateInfoBar();
+  ASSERT_TRUE(infobar != NULL);
+  EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATING, infobar->type());
+
   SimulateURLFetch(true);  // Simulate the translate script being retrieved.
   int page_id = 0;
   std::string original_lang, target_lang;
@@ -920,9 +934,6 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
   EXPECT_EQ("fr", original_lang);
   EXPECT_EQ("en", target_lang);
   process()->sink().ClearMessages();
-  // And we should have no infobar (since we don't send the page translated
-  // notification, the after translate infobar is not shown).
-  EXPECT_TRUE(GetTranslateInfoBar() == NULL);
 
   // Try another language, it should not be autotranslated.
   SimulateNavigation(GURL("http://www.google.es"), 1, "El Google", "es", true);
@@ -943,12 +954,14 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
   test_profile->set_off_the_record(false);  // Get back to non incognito.
 
   // Now revert the always translate pref and make sure we go back to expected
-  // behavior, which is show an infobar.
+  // behavior, which is show a "before translate" infobar.
   SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateWhitelists);
   translate_prefs.RemoveLanguagePairFromWhitelist("fr", "en");
   SimulateNavigation(GURL("http://www.google.fr"), 3, "Le Google", "fr", true);
   EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang));
-  EXPECT_TRUE(GetTranslateInfoBar() != NULL);
+  infobar = GetTranslateInfoBar();
+  ASSERT_TRUE(infobar != NULL);
+  EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type());
   prefs->RemovePrefObserver(TranslatePrefs::kPrefTranslateWhitelists,
                             &pref_observer_);
 }
@@ -963,18 +976,30 @@ TEST_F(TranslateManagerTest, ContextMenu) {
   EXPECT_TRUE(translate_prefs.IsLanguageBlacklisted("fr"));
   EXPECT_TRUE(translate_prefs.IsSiteBlacklisted(url.host()));
 
-  // Simulate navigating to a page in French. The translate menu should show.
-  SimulateNavigation(url, 0, "Le Google", "fr", true);
+  // Simulate navigating to a page in French. The translate menu should show but
+  // should only be enabled when the page language has been received.
+  NavigateAndCommit(url);
   scoped_ptr<TestRenderViewContextMenu> menu(
       TestRenderViewContextMenu::CreateContextMenu(contents()));
   menu->Init();
   EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_TRANSLATE));
+  EXPECT_FALSE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_TRANSLATE));
+
+  // Simulate receiving the language.
+  SimulateOnPageContents(url, 0, "Le Google", "fr", true);
+  menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents()));
+  menu->Init();
+  EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_TRANSLATE));
   EXPECT_TRUE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_TRANSLATE));
 
   // Use the menu to translate the page.
   menu->ExecuteCommand(IDC_CONTENT_CONTEXT_TRANSLATE);
 
   // That should have triggered a translation.
+  // The "translating..." infobar should be showing.
+  TranslateInfoBarDelegate* infobar = GetTranslateInfoBar();
+  ASSERT_TRUE(infobar != NULL);
+  EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATING, infobar->type());
   SimulateURLFetch(true);  // Simulate the translate script being retrieved.
   int page_id = 0;
   std::string original_lang, target_lang;
@@ -1002,7 +1027,7 @@ TEST_F(TranslateManagerTest, ContextMenu) {
   // translated does nothing (this could happen if autotranslate kicks-in and
   // the user selects the menu while the translation is being performed).
   SimulateNavigation(GURL("http://www.google.es"), 1, "El Google", "es", true);
-  TranslateInfoBarDelegate* infobar = GetTranslateInfoBar();
+  infobar = GetTranslateInfoBar();
   ASSERT_TRUE(infobar != NULL);
   infobar->Translate();
   EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang));
@@ -1166,3 +1191,4 @@ TEST_F(TranslateManagerTest, ScriptExpires) {
   EXPECT_EQ("es", original_lang);
   EXPECT_EQ("en", target_lang);
 }
+