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

10 files changed, 63 insertions, 32 deletions

diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index 5941dcb..4f8d317 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -769,6 +769,7 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
       target_lang = TranslateManager::GetLanguageCode(target_lang);
       return !!(params_.edit_flags & WebContextMenuData::CanTranslate) &&
              source_tab_contents_->language_state().page_translatable() &&
+             !original_lang.empty() &&  // Did we receive the page language yet?
              original_lang != target_lang &&
              !source_tab_contents_->language_state().IsPageTranslated() &&
              !source_tab_contents_->interstitial_page() &&
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);
 }
+
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index 47f0d52..927f7d3 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -134,6 +134,8 @@ const int kMaxSessionHistoryEntries = 50;
 
 const wchar_t kChromiumRendererIdProperty[] = L"ChromiumRendererId";
 
+const char* const kUnknownLanguageCode = "und";
+
 }  // namespace chrome
 
 #undef FPL
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index a1efdbf..84d6052 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -90,6 +90,10 @@ extern const int kMaxSessionHistoryEntries;
 
 extern const wchar_t kChromiumRendererIdProperty[];
 
+// The language code used when the language of a page could not be detected.
+// (Matches what the CLD -Compact Language Detection- library reports.)
+extern const char* const kUnknownLanguageCode;
+
 }  // namespace chrome
 
 #endif  // CHROME_COMMON_CHROME_CONSTANTS_H_
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index f693325..e6eccfa 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -264,9 +264,6 @@ static const char* const kUnreachableWebDataURL =
 
 static const char* const kBackForwardNavigationScheme = "history";
 
-// static
-const char* const RenderView::kUnknownLanguageCode = "und";
-
 static void GetRedirectChain(WebDataSource* ds, std::vector<GURL>* result) {
   WebVector<WebURL> urls;
   ds->redirectChain(urls);
@@ -372,9 +369,9 @@ static std::string DetermineTextLanguage(const string16& text) {
   // Text with less than 100 bytes will probably not provide good results.
   // Report it as unknown language.
   if (text.length() < 100)
-    return RenderView::kUnknownLanguageCode;
+    return chrome::kUnknownLanguageCode;
 
-  std::string language = RenderView::kUnknownLanguageCode;
+  std::string language = chrome::kUnknownLanguageCode;
   int num_languages = 0;
   bool is_reliable = false;
   Language cld_language =
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index f60669d..72bdad5 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -567,9 +567,6 @@ class RenderView : public RenderWidget,
       std::string* json_retval);
   virtual WebKit::WebCookieJar* GetCookieJar();
 
-  // The language code used when the page language is unknown.
-  static const char* const kUnknownLanguageCode;
-
   // Please do not add your stuff randomly to the end here. If there is an
   // appropriate section, add it there. If not, there are some random functions
   // nearer to the top you can add it to.
diff --git a/chrome/renderer/translate_helper.cc b/chrome/renderer/translate_helper.cc
index 5075917..4f5c89b 100644
--- a/chrome/renderer/translate_helper.cc
+++ b/chrome/renderer/translate_helper.cc
@@ -5,6 +5,7 @@
 #include "chrome/renderer/translate_helper.h"
 
 #include "base/compiler_specific.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/renderer/render_view.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebScriptSource.h"
@@ -58,7 +59,7 @@ void TranslateHelper::TranslatePage(int page_id,
   page_id_ = page_id;
   // If the source language is undetermined, we'll let the translate element
   // detect it.
-  source_lang_ = (source_lang != RenderView::kUnknownLanguageCode) ?
+  source_lang_ = (source_lang != chrome::kUnknownLanguageCode) ?
                   source_lang : kAutoDetectionLanguage;
   target_lang_ = target_lang;
 
diff --git a/chrome/renderer/translate_helper_unittest.cc b/chrome/renderer/translate_helper_unittest.cc
index eef25fb..0221006 100644
--- a/chrome/renderer/translate_helper_unittest.cc
+++ b/chrome/renderer/translate_helper_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/common/chrome_constants.h"
 #include "chrome/renderer/translate_helper.h"
 #include "chrome/test/render_view_test.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -189,7 +190,7 @@ TEST_F(TranslateHelperTest, UndefinedSourceLang) {
       .WillRepeatedly(Return(true));
 
   translate_helper_->TranslatePage(view_->page_id(),
-                                   RenderView::kUnknownLanguageCode, "fr",
+                                   chrome::kUnknownLanguageCode, "fr",
                                    std::string());
   MessageLoop::current()->RunAllPending();