The browser would crash when pressing the translate button in the infobar when the locale is Norvegian,

Filipino or any Chrome languages that the translation server does not support.
The crash would happen in the translate infobar, the source index would be
-1 and would cause us to crash accessing that index in a vector.
This CL fixes that by cleaning up the list of supported languages, so they
match Chrome's locales.
Also it makes sure we don't create an infobar if for any reason one of the
language (source or target) is not supported.

BUG=37218
TEST=For each language Chrome can be run into, select that language and open
     www.google.com. Check that the info-bar is shown for languages supported
     by the Google translate server (see the list at http://translate.google.com/)
     and for the supported language translating works.

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

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

7 files changed, 163 insertions, 61 deletions

diff --git a/chrome/browser/renderer_host/translation_service.cc b/chrome/browser/renderer_host/translation_service.cc
index 3413d21..817fdf9 100644
--- a/chrome/browser/renderer_host/translation_service.cc
+++ b/chrome/browser/renderer_host/translation_service.cc
@@ -48,6 +48,17 @@ LocaleToCLDLanguage kLocaleToCLDLanguages[] = {
 };
 
 // The list of languages the Google translation server supports.
+// For information, here is the list of languages that Chrome can be run into
+// but that the translation server does not support:
+// am Amharic
+// bn Bengali
+// gu Gujarati
+// kn Kannada
+// ml Malayalam
+// mr Marathi
+// or Oriya
+// ta Tamil
+// te Telugu
 const char* kSupportedLanguages[] = {
     "af",     // Afrikaans
     "sq",     // Albanian
@@ -63,8 +74,8 @@ const char* kSupportedLanguages[] = {
     "nl",     // Dutch
     "en",     // English
     "et",     // Estonian
-    "tl",     // Tagalog (Filipino)
     "fi",     // Finnish
+    "fil",    // Filipino
     "fr",     // French
     "gl",     // Galician
     "de",     // German
@@ -83,7 +94,7 @@ const char* kSupportedLanguages[] = {
     "mk",     // Macedonian
     "ms",     // Malay
     "mt",     // Maltese
-    "no",     // Norwegian
+    "nb",     // Norwegian
     "fa",     // Persian
     "pl",     // Polish
     "pt",     // Portuguese
diff --git a/chrome/browser/translate/translate_infobars_delegates.cc b/chrome/browser/translate/translate_infobars_delegates.cc
index 8107f4f..db03c5c 100644
--- a/chrome/browser/translate/translate_infobars_delegates.cc
+++ b/chrome/browser/translate/translate_infobars_delegates.cc
@@ -41,36 +41,6 @@ void TranslateInfoBarDelegate::InfoBarClosed() {
 
 // TranslateInfoBarDelegate: public: -------------------------------------------
 
-TranslateInfoBarDelegate::TranslateInfoBarDelegate(TabContents* tab_contents,
-    PrefService* user_prefs, TranslateState state, const GURL& url,
-    const std::string& original_lang_code, const std::string& target_lang_code)
-    : InfoBarDelegate(tab_contents),
-      tab_contents_(tab_contents),
-      prefs_(user_prefs),
-      state_(state),
-      site_(url.HostNoBrackets()),
-      original_lang_index_(-1),
-      target_lang_index_(-1),
-      never_translate_language_(false),
-      never_translate_site_(false),
-      always_translate_(false) {
-  TranslationService::GetSupportedLanguages(&supported_languages_);
-  for (size_t i = 0; i < supported_languages_.size(); ++i) {
-    if (original_lang_code == supported_languages_[i]) {
-      original_lang_index_ = i;
-      break;
-    }
-  }
-  DCHECK(original_lang_index_ > -1);
-  for (size_t i = 0; i < supported_languages_.size(); ++i) {
-    if (target_lang_code == supported_languages_[i]) {
-      target_lang_index_ = i;
-      break;
-    }
-  }
-  DCHECK(target_lang_index_ > -1);
-}
-
 void TranslateInfoBarDelegate::UpdateState(TranslateState new_state) {
   if (state_ != new_state)
     state_ = new_state;
@@ -207,6 +177,38 @@ void TranslateInfoBarDelegate::GetMessageText(string16 *message_text,
 
 // TranslateInfoBarDelegate: static: -------------------------------------------
 
+TranslateInfoBarDelegate* TranslateInfoBarDelegate::Create(
+    TabContents* tab_contents, PrefService* user_prefs, TranslateState state,
+    const GURL& url,
+    const std::string& original_lang_code,
+    const std::string& target_lang_code) {
+  std::vector<std::string> supported_languages;
+  TranslationService::GetSupportedLanguages(&supported_languages);
+
+  int original_lang_index = -1;
+  for (size_t i = 0; i < supported_languages.size(); ++i) {
+    if (original_lang_code == supported_languages[i]) {
+      original_lang_index = i;
+      break;
+    }
+  }
+  if (original_lang_index == -1)
+    return NULL;
+
+  int target_lang_index = -1;
+  for (size_t i = 0; i < supported_languages.size(); ++i) {
+    if (target_lang_code == supported_languages[i]) {
+      target_lang_index = i;
+      break;
+    }
+  }
+  if (target_lang_index == -1)
+    return NULL;
+
+  return new TranslateInfoBarDelegate(tab_contents, user_prefs, state, url,
+                                      original_lang_index, target_lang_index);
+}
+
 string16 TranslateInfoBarDelegate::GetDisplayNameForLocale(
     const std::string& language_code) {
   return l10n_util::GetDisplayNameForLocale(
@@ -221,3 +223,24 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
   return NULL;
 }
 #endif  // !TOOLKIT_VIEWS
+
+// TranslateInfoBarDelegate: private: ------------------------------------------
+
+TranslateInfoBarDelegate::TranslateInfoBarDelegate(TabContents* tab_contents,
+    PrefService* user_prefs, TranslateState state, const GURL& url,
+    int original_lang_index, int target_lang_index)
+    : InfoBarDelegate(tab_contents),
+      tab_contents_(tab_contents),
+      prefs_(user_prefs),
+      state_(state),
+      site_(url.HostNoBrackets()),
+      original_lang_index_(original_lang_index),
+      target_lang_index_(target_lang_index),
+      never_translate_language_(false),
+      never_translate_site_(false),
+      always_translate_(false) {
+  TranslationService::GetSupportedLanguages(&supported_languages_);
+  DCHECK(original_lang_index_ > -1);
+  DCHECK(target_lang_index_ > -1);
+}
+
diff --git a/chrome/browser/translate/translate_infobars_delegates.h b/chrome/browser/translate/translate_infobars_delegates.h
index d2fc074..ce8cede 100644
--- a/chrome/browser/translate/translate_infobars_delegates.h
+++ b/chrome/browser/translate/translate_infobars_delegates.h
@@ -21,9 +21,14 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
     kTranslationFailed,
   };
 
-  TranslateInfoBarDelegate(TabContents* contents, PrefService* user_prefs,
-      TranslateState state, const GURL& url,
-      const std::string& original_language, const std::string& target_language);
+  // Instantiates a TranslateInfoBarDelegate. Can return NULL if the passed
+  // languages are not supported.
+  static TranslateInfoBarDelegate* Create(TabContents* contents,
+                                          PrefService* user_prefs,
+                                          TranslateState state,
+                                          const GURL& url,
+                                          const std::string& original_language,
+                                          const std::string& target_language);
 
   void UpdateState(TranslateState new_state);
   void GetAvailableOriginalLanguages(std::vector<std::string>* languages);
@@ -95,6 +100,13 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
   virtual InfoBar* CreateInfoBar();
 
  private:
+  TranslateInfoBarDelegate(TabContents* contents,
+                           PrefService* user_prefs,
+                           TranslateState state,
+                           const GURL& url,
+                           int original_language_index,
+                           int target_language_index);
+
   TabContents* tab_contents_;  // Weak.
   TranslatePrefs prefs_;
   TranslateState state_;
diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index 9435786..5470a8b 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -17,7 +17,6 @@
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_util.h"
-#include "chrome/browser/translate/translate_infobars_delegates.h"
 #include "chrome/browser/translate/translate_prefs.h"
 #include "chrome/common/notification_details.h"
 #include "chrome/common/notification_service.h"
@@ -92,10 +91,9 @@ void TranslateManager::Observe(NotificationType type,
         if (entry) {
           std::pair<std::string, std::string>* language_pair =
               (Details<std::pair<std::string, std::string> >(details).ptr());
-          PrefService* prefs = tab->profile()->GetPrefs();
-          tab->AddInfoBar(new TranslateInfoBarDelegate(tab, prefs,
-              TranslateInfoBarDelegate::kAfterTranslate, entry->url(),
-              language_pair->first, language_pair->second));
+          AddTranslateInfoBar(tab, TranslateInfoBarDelegate::kAfterTranslate,
+                              entry->url(),
+                              language_pair->first, language_pair->second);
         }
       }
       break;
@@ -147,12 +145,16 @@ void TranslateManager::InitiateTranslation(TabContents* tab,
     return;
   }
 
-  if (!TranslationService::IsSupportedLanguage(page_lang))
-    return;  // Nothing to do, we don't support that language.
-
   std::string ui_lang = TranslationService::GetLanguageCode(
       g_browser_process->GetApplicationLocale());
 
+  // Nothing to do if the language Chrome is in or the language of the page is
+  // not supported by the translation server.
+  if (!TranslationService::IsSupportedLanguage(ui_lang) ||
+      !TranslationService::IsSupportedLanguage(page_lang)) {
+    return;
+  }
+
   // We don't want to translate:
   // - any Chrome specific page (New Tab Page, Download, History... pages).
   // - similar languages (ex: en-US to en).
@@ -178,9 +180,8 @@ void TranslateManager::InitiateTranslation(TabContents* tab,
   }
 
   // Prompts the user if he/she wants the page translated.
-  tab->AddInfoBar(new TranslateInfoBarDelegate(tab, prefs,
-      TranslateInfoBarDelegate::kBeforeTranslate, entry->url(),
-      page_lang, ui_lang));
+  AddTranslateInfoBar(tab, TranslateInfoBarDelegate::kBeforeTranslate,
+                      entry->url(), page_lang, ui_lang);
 }
 
 void TranslateManager::InitiateTranslationPosted(int process_id,
@@ -244,3 +245,19 @@ void TranslateManager::InitAcceptLanguages(PrefService* prefs) {
   }
   accept_languages_[prefs] = accept_langs_set;
 }
+
+void TranslateManager::AddTranslateInfoBar(
+    TabContents* tab, TranslateInfoBarDelegate::TranslateState state,
+    const GURL& url,
+    const std::string& original_language, const std::string& target_language) {
+  PrefService* prefs = tab->profile()->GetPrefs();
+  TranslateInfoBarDelegate* infobar =
+      TranslateInfoBarDelegate::Create(tab, prefs, state, url,
+                                       original_language, target_language);
+  if (!infobar) {
+    NOTREACHED() << "Failed to create infobar for language " <<
+        original_language << " and " << target_language;
+    return;
+  }
+  tab->AddInfoBar(infobar);
+}
diff --git a/chrome/browser/translate/translate_manager.h b/chrome/browser/translate/translate_manager.h
index f14dfdd..7ac1eb8 100644
--- a/chrome/browser/translate/translate_manager.h
+++ b/chrome/browser/translate/translate_manager.h
@@ -11,6 +11,7 @@
 
 #include "base/singleton.h"
 #include "base/task.h"
+#include "chrome/browser/translate/translate_infobars_delegates.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 
@@ -58,6 +59,13 @@ class TranslateManager : public NotificationObserver {
   // preference in |prefs|.
   void InitAcceptLanguages(PrefService* prefs);
 
+  // Convenience method that adds a translate infobar to |tab|.
+  void AddTranslateInfoBar(TabContents* tab,
+                           TranslateInfoBarDelegate::TranslateState state,
+                           const GURL& url,
+                           const std::string& original_language,
+                           const std::string& target_language);
+
   NotificationRegistrar notification_registrar_;
 
   // A map that associates a profile with its parsed "accept languages".
diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc
index eb63416..476faa7 100644
--- a/chrome/browser/translate/translate_manager_unittest.cc
+++ b/chrome/browser/translate/translate_manager_unittest.cc
@@ -12,6 +12,7 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
+#include "chrome/test/testing_browser_process.h"
 
 class TestTranslateManager : public TranslateManager {
  public:
@@ -23,7 +24,7 @@ class TranslateManagerTest : public RenderViewHostTestHarness,
  public:
   TranslateManagerTest() {}
 
-  // Simluates navigating to a page and getting teh page contents and language
+  // Simluates navigating to a page and getting the page contents and language
   // for that navigation.
   void SimulateNavigation(const GURL& url, int page_id,
                           const std::wstring& contents,
@@ -207,7 +208,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) {
 
 // Tests auto-translate on page.
 TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) {
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // Simulate the user translating.
@@ -238,7 +239,7 @@ TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) {
 
 // Tests that multiple OnPageContents do not cause multiple infobars.
 TEST_F(TranslateManagerTest, MultipleOnPageContents) {
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // Simulate clicking 'Nope' (don't translate).
@@ -259,7 +260,7 @@ TEST_F(TranslateManagerTest, MultipleOnPageContents) {
 
 // Test that reloading the page brings back the infobar.
 TEST_F(TranslateManagerTest, Reload) {
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // Close the infobar.
@@ -276,7 +277,7 @@ TEST_F(TranslateManagerTest, Reload) {
 // Tests that a close translate infobar does not reappear when navigating
 // in-page.
 TEST_F(TranslateManagerTest, CloseInfoBarInPageNavigation) {
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // Close the infobar.
@@ -293,7 +294,7 @@ TEST_F(TranslateManagerTest, CloseInfoBarInPageNavigation) {
 
 // Tests that denying translation is sticky when navigating in page.
 TEST_F(TranslateManagerTest, DenyTranslateInPageNavigation) {
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // Simulate clicking 'Nope' (don't translate).
@@ -311,7 +312,7 @@ TEST_F(TranslateManagerTest, DenyTranslateInPageNavigation) {
 // Tests that after translating and closing the infobar, the infobar does not
 // return when navigating in page.
 TEST_F(TranslateManagerTest, TranslateCloseInfoBarInPageNavigation) {
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // Simulate the user translating.
@@ -338,7 +339,7 @@ TEST_F(TranslateManagerTest, TranslateCloseInfoBarInPageNavigation) {
 // Tests that the after translate the infobar still shows when navigating
 // in-page.
 TEST_F(TranslateManagerTest, TranslateInPageNavigation) {
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // Simulate the user translating.
@@ -362,13 +363,41 @@ TEST_F(TranslateManagerTest, TranslateInPageNavigation) {
   EXPECT_TRUE(GetTranslateInfoBar() != NULL);
 }
 
+// Tests that no translate infobar is shown when navigating to a page in an
+// unsupported language.
+TEST_F(TranslateManagerTest, UnsupportedPageLanguage) {
+  // Simulate navigating to a page and getting an unsupported language.
+  SimulateNavigation(GURL("http://www.google.com"), 0, L"Google", "qbz");
+
+  // No info-bar should be shown.
+  EXPECT_TRUE(GetTranslateInfoBar() == NULL);
+}
+
+// Tests that no translate infobar is shown when Chrome is in a language that
+// the translate server does not support.
+TEST_F(TranslateManagerTest, UnsupportedUILanguage) {
+  TestingBrowserProcess* browser_process =
+      static_cast<TestingBrowserProcess*>(g_browser_process);
+  std::string original_lang = browser_process->GetApplicationLocale();
+  browser_process->set_application_locale("qbz");
+
+  // Simulate navigating to a page in a language supported by the translate
+  // server.
+  SimulateNavigation(GURL("http://www.google.com"), 0, L"Google", "en");
+
+  // No info-bar should be shown.
+  EXPECT_TRUE(GetTranslateInfoBar() == NULL);
+
+  browser_process->set_application_locale(original_lang);
+}
+
 // Tests that the translate preference is honored.
 TEST_F(TranslateManagerTest, TranslatePref) {
   // Make sure the pref allows translate.
   PrefService* prefs = contents()->profile()->GetPrefs();
   prefs->SetBoolean(prefs::kEnableTranslate, true);
 
-  // Simulate navigating to a page and gettings its language.
+  // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr");
 
   // An infobar should be shown.
diff --git a/chrome/test/testing_browser_process.h b/chrome/test/testing_browser_process.h
index 62dc8a9..b32717f 100644
--- a/chrome/test/testing_browser_process.h
+++ b/chrome/test/testing_browser_process.h
@@ -25,7 +25,8 @@ class IOThread;
 class TestingBrowserProcess : public BrowserProcess {
  public:
   TestingBrowserProcess()
-      : shutdown_event_(new base::WaitableEvent(true, false)) {
+      : shutdown_event_(new base::WaitableEvent(true, false)),
+        app_locale_("en") {
   }
 
   virtual ~TestingBrowserProcess() {
@@ -140,12 +141,12 @@ class TestingBrowserProcess : public BrowserProcess {
   }
 
   virtual const std::string& GetApplicationLocale() {
-    static std::string* value = NULL;
-    if (!value)
-      value = new std::string("en");
-    return *value;
+    return app_locale_;
+  }
+
+  virtual void set_application_locale(const std::string& app_locale) {
+    app_locale_ = app_locale;
   }
-  virtual void set_application_locale(const std::string& app_locale) {}
 
   virtual base::WaitableEvent* shutdown_event() {
     return shutdown_event_.get();
@@ -161,6 +162,7 @@ class TestingBrowserProcess : public BrowserProcess {
   NotificationService notification_service_;
   scoped_ptr<base::WaitableEvent> shutdown_event_;
   scoped_ptr<Clipboard> clipboard_;
+  std::string app_locale_;
 
   DISALLOW_COPY_AND_ASSIGN(TestingBrowserProcess);
 };