Translate: URL check should be done at once

The URL for Chrome OS file manager extension is checked
separately for now. But this must be done in IsTranslatableURL()
at once.

BUG=none
TEST=unit_tests

Review URL: https://codereview.chromium.org/13770003

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

chrome/browser/translate/translate_manager.cc

@@ -56,6 +56,7 @@
 
 #ifdef FILE_MANAGER_EXTENSION
 #include "chrome/browser/chromeos/extensions/file_manager_util.h"
+#include "extensions/common/constants.h"
 #endif
 
 using content::NavigationController;
@@ -184,12 +185,19 @@ TranslateManager* TranslateManager::GetInstance() {
 // static
 bool TranslateManager::IsTranslatableURL(const GURL& url) {
   // A URLs is translatable unless it is one of the following:
+  // - empty (can happen for popups created with window.open(""))
   // - an internal URL (chrome:// and others)
   // - the devtools (which is considered UI)
+  // - Chrome OS file manager extension
   // - an FTP page (as FTP pages tend to have long lists of filenames that may
   //   confuse the CLD)
-  return !url.SchemeIs(chrome::kChromeUIScheme) &&
+  return !url.is_empty() &&
+         !url.SchemeIs(chrome::kChromeUIScheme) &&
          !url.SchemeIs(chrome::kChromeDevToolsScheme) &&
+#ifdef FILE_MANAGER_EXTENSION
+         !(url.SchemeIs(extensions::kExtensionScheme) &&
+           url.DomainIs(kFileBrowserDomain)) &&
+#endif
          !url.SchemeIs(chrome::kFtpScheme);
 }
 
@@ -487,13 +495,6 @@ TranslateManager::TranslateManager()
 
 void TranslateManager::InitiateTranslation(WebContents* web_contents,
                                            const std::string& page_lang) {
-#ifdef FILE_MANAGER_EXTENSION
-  const GURL& page_url = web_contents->GetURL();
-  if (page_url.SchemeIs("chrome-extension") &&
-      page_url.DomainIs(kFileBrowserDomain))
-    return;
-#endif
-
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   PrefService* prefs = profile->GetOriginalProfile()->GetPrefs();
@@ -505,12 +506,6 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableTranslate))
     return;
 
-  NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
-  if (!entry) {
-    // This can happen for popups created with window.open("").
-    return;
-  }
-
   std::string target_lang = GetTargetLanguage(prefs);
   std::string language_code = GetLanguageCode(page_lang);
   // Nothing to do if either the language Chrome is in or the language of the
@@ -524,8 +519,10 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
   // - similar languages (ex: en-US to en).
   // - any user black-listed URLs or user selected language combination.
   // - any language the user configured as accepted languages.
-  if (!IsTranslatableURL(entry->GetURL()) || language_code == target_lang ||
-      !TranslatePrefs::CanTranslate(prefs, language_code, entry->GetURL()) ||
+  GURL page_url = web_contents->GetURL();
+  if (!IsTranslatableURL(page_url) ||
+      language_code == target_lang ||
+      !TranslatePrefs::CanTranslate(prefs, language_code, page_url) ||
       IsAcceptLanguage(web_contents, language_code)) {
     return;
   }

chrome/browser/translate/translate_manager.h

@@ -114,8 +114,6 @@ class TranslateManager : public content::NotificationObserver,
 
  private:
   friend struct DefaultSingletonTraits<TranslateManager>;
-  friend class TranslateManagerTest;
-  FRIEND_TEST_ALL_PREFIXES(TranslateManagerTest, LanguageCodeSynonyms);
 
   // Structure that describes a translate request.
   // Translation may be deferred while the translate script is being retrieved

chrome/browser/translate/translate_manager_browsertest.cc

@@ -91,12 +91,13 @@ class NavEntryCommittedObserver : public content::NotificationObserver {
   DISALLOW_COPY_AND_ASSIGN(NavEntryCommittedObserver);
 };
 
-class TranslateManagerTest : public ChromeRenderViewHostTestHarness,
-                             public content::NotificationObserver {
+class TranslateManagerBrowserTest : public ChromeRenderViewHostTestHarness,
+                                    public content::NotificationObserver {
  public:
-  TranslateManagerTest()
-      : pref_callback_(base::Bind(&TranslateManagerTest::OnPreferenceChanged,
-                                  base::Unretained(this))),
+  TranslateManagerBrowserTest()
+      : pref_callback_(
+            base::Bind(&TranslateManagerBrowserTest::OnPreferenceChanged,
+                       base::Unretained(this))),
         ui_thread_(BrowserThread::UI, &message_loop_) {
   }
 
@@ -314,7 +315,7 @@ class TranslateManagerTest : public ChromeRenderViewHostTestHarness,
   // WARNING: the pointers point to deleted objects, use only for comparison.
   std::set<InfoBarDelegate*> removed_infobars_;
 
-  DISALLOW_COPY_AND_ASSIGN(TranslateManagerTest);
+  DISALLOW_COPY_AND_ASSIGN(TranslateManagerBrowserTest);
 };
 
 namespace {
@@ -362,7 +363,7 @@ class TestRenderViewContextMenu : public RenderViewContextMenu {
 
 }  // namespace
 
-TEST_F(TranslateManagerTest, NormalTranslate) {
+TEST_F(TranslateManagerBrowserTest, NormalTranslate) {
   // Simulate navigating to a page.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -445,7 +446,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) {
   ASSERT_EQ(new_target_lang, infobar->target_language_code());
 }
 
-TEST_F(TranslateManagerTest, TranslateScriptNotAvailable) {
+TEST_F(TranslateManagerBrowserTest, TranslateScriptNotAvailable) {
   // Simulate navigating to a page.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -473,7 +474,7 @@ TEST_F(TranslateManagerTest, TranslateScriptNotAvailable) {
 
 // Ensures we deal correctly with pages for which the browser does not recognize
 // the language (the translate server may or not detect the language).
-TEST_F(TranslateManagerTest, TranslateUnknownLanguage) {
+TEST_F(TranslateManagerBrowserTest, TranslateUnknownLanguage) {
   // Simulate navigating to a page ("und" is the string returned by the CLD for
   // languages it does not recognize).
   SimulateNavigation(GURL("http://www.google.mys"), "und", true);
@@ -547,7 +548,7 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) {
 
 // Tests that we show/don't show an info-bar for all languages the CLD can
 // report.
-TEST_F(TranslateManagerTest, TestAllLanguages) {
+TEST_F(TranslateManagerBrowserTest, TestAllLanguages) {
   // The index in kExpectation are the Language enum (see languages.pb.h).
   // true if we expect a translate infobar for that language.
   // Note the supported languages are in translation_manager.cc, see
@@ -616,7 +617,7 @@ TEST_F(TranslateManagerTest, TestAllLanguages) {
 }
 
 // Test the fetching of languages from the translate server
-TEST_F(TranslateManagerTest, FetchLanguagesFromTranslateServer) {
+TEST_F(TranslateManagerBrowserTest, FetchLanguagesFromTranslateServer) {
   std::vector<std::string> server_languages;
   // A list of languages to fake being returned by the translate server.
   server_languages.push_back("aa");
@@ -690,7 +691,7 @@ std::string GetLanguageListString(
 }
 
 // Tests auto-translate on page.
-TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) {
+TEST_F(TranslateManagerBrowserTest, AutoTranslateOnNavigate) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -727,7 +728,7 @@ TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) {
 }
 
 // Tests that multiple OnPageContents do not cause multiple infobars.
-TEST_F(TranslateManagerTest, MultipleOnPageContents) {
+TEST_F(TranslateManagerBrowserTest, MultipleOnPageContents) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -749,7 +750,7 @@ TEST_F(TranslateManagerTest, MultipleOnPageContents) {
 
 // Test that reloading the page brings back the infobar if the
 // reload succeeded and does not bring it back the reload fails.
-TEST_F(TranslateManagerTest, Reload) {
+TEST_F(TranslateManagerBrowserTest, Reload) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -770,7 +771,7 @@ TEST_F(TranslateManagerTest, Reload) {
 
 // Test that reloading the page by way of typing again the URL in the
 // location bar brings back the infobar.
-TEST_F(TranslateManagerTest, ReloadFromLocationBar) {
+TEST_F(TranslateManagerBrowserTest, ReloadFromLocationBar) {
   GURL url("http://www.google.fr");
 
   // Simulate navigating to a page and getting its language.
@@ -802,7 +803,7 @@ TEST_F(TranslateManagerTest, ReloadFromLocationBar) {
 
 // Tests that a closed translate infobar does not reappear when navigating
 // in-page.
-TEST_F(TranslateManagerTest, CloseInfoBarInPageNavigation) {
+TEST_F(TranslateManagerBrowserTest, CloseInfoBarInPageNavigation) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -821,7 +822,7 @@ TEST_F(TranslateManagerTest, CloseInfoBarInPageNavigation) {
 
 // Tests that a closed translate infobar does not reappear when navigating
 // in a subframe. (http://crbug.com/48215)
-TEST_F(TranslateManagerTest, CloseInfoBarInSubframeNavigation) {
+TEST_F(TranslateManagerBrowserTest, CloseInfoBarInSubframeNavigation) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -844,7 +845,7 @@ TEST_F(TranslateManagerTest, CloseInfoBarInSubframeNavigation) {
 }
 
 // Tests that denying translation is sticky when navigating in page.
-TEST_F(TranslateManagerTest, DenyTranslateInPageNavigation) {
+TEST_F(TranslateManagerBrowserTest, DenyTranslateInPageNavigation) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -862,7 +863,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) {
+TEST_F(TranslateManagerBrowserTest, TranslateCloseInfoBarInPageNavigation) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -894,7 +895,7 @@ TEST_F(TranslateManagerTest, TranslateCloseInfoBarInPageNavigation) {
 
 // Tests that the after translate the infobar still shows when navigating
 // in-page.
-TEST_F(TranslateManagerTest, TranslateInPageNavigation) {
+TEST_F(TranslateManagerBrowserTest, TranslateInPageNavigation) {
   // Simulate navigating to a page and getting its language.
   SimulateNavigation(GURL("http://www.google.fr"), "fr", true);
 
@@ -924,7 +925,7 @@ TEST_F(TranslateManagerTest, TranslateInPageNavigation) {
 
 // Tests that no translate infobar is shown when navigating to a page in an
 // unsupported language.
-TEST_F(TranslateManagerTest, CLDReportsUnsupportedPageLanguage) {
+TEST_F(TranslateManagerBrowserTest, CLDReportsUnsupportedPageLanguage) {
   // Simulate navigating to a page and getting an unsupported language.
   SimulateNavigation(GURL("http://www.google.com"), "qbz", true);
 
@@ -935,7 +936,7 @@ TEST_F(TranslateManagerTest, CLDReportsUnsupportedPageLanguage) {
 // Tests that we deal correctly with unsupported languages returned by the
 // server.
 // The translation server might return a language we don't support.
-TEST_F(TranslateManagerTest, ServerReportsUnsupportedLanguage) {
+TEST_F(TranslateManagerBrowserTest, ServerReportsUnsupportedLanguage) {
   // Simulate navigating to a page and translating it.
   SimulateNavigation(GURL("http://mail.google.fr"), "fr", true);
   TranslateInfoBarDelegate* infobar = GetTranslateInfoBar();
@@ -973,7 +974,7 @@ TEST_F(TranslateManagerTest, ServerReportsUnsupportedLanguage) {
 
 // Tests that no translate infobar is shown and context menu is disabled, when
 // Chrome is in a language that the translate server does not support.
-TEST_F(TranslateManagerTest, UnsupportedUILanguage) {
+TEST_F(TranslateManagerBrowserTest, UnsupportedUILanguage) {
   std::string original_lang = g_browser_process->GetApplicationLocale();
   g_browser_process->SetApplicationLocale("qbz");
 
@@ -1001,7 +1002,7 @@ TEST_F(TranslateManagerTest, UnsupportedUILanguage) {
 }
 
 // Tests that the first supported accept language is selected
-TEST_F(TranslateManagerTest, TranslateAcceptLanguage) {
+TEST_F(TranslateManagerBrowserTest, TranslateAcceptLanguage) {
   // Set locate to non-existant language
   std::string original_lang = g_browser_process->GetApplicationLocale();
   g_browser_process->SetApplicationLocale("qbz");
@@ -1031,7 +1032,7 @@ TEST_F(TranslateManagerTest, TranslateAcceptLanguage) {
 }
 
 // Tests that the translate enabled preference is honored.
-TEST_F(TranslateManagerTest, TranslateEnabledPref) {
+TEST_F(TranslateManagerBrowserTest, TranslateEnabledPref) {
   // Make sure the pref allows translate.
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
@@ -1062,7 +1063,7 @@ TEST_F(TranslateManagerTest, TranslateEnabledPref) {
 }
 
 // Tests the "Never translate <language>" pref.
-TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) {
+TEST_F(TranslateManagerBrowserTest, NeverTranslateLanguagePref) {
   // Simulate navigating to a page and getting its language.
   GURL url("http://www.google.fr");
   SimulateNavigation(url, "fr", true);
@@ -1109,7 +1110,7 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) {
 }
 
 // Tests the "Never translate this site" pref.
-TEST_F(TranslateManagerTest, NeverTranslateSitePref) {
+TEST_F(TranslateManagerBrowserTest, NeverTranslateSitePref) {
   // Simulate navigating to a page and getting its language.
   GURL url("http://www.google.fr");
   std::string host(url.host());
@@ -1156,7 +1157,7 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) {
 }
 
 // Tests the "Always translate this language" pref.
-TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
+TEST_F(TranslateManagerBrowserTest, AlwaysTranslateLanguagePref) {
   // Select always translate French to English.
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
@@ -1216,7 +1217,7 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
 }
 
 // Context menu.
-TEST_F(TranslateManagerTest, ContextMenu) {
+TEST_F(TranslateManagerBrowserTest, ContextMenu) {
   // Blacklist www.google.fr and French for translation.
   GURL url("http://www.google.fr");
   Profile* profile =
@@ -1331,7 +1332,7 @@ TEST_F(TranslateManagerTest, ContextMenu) {
 // Tests that an extra always/never translate button is shown on the "before
 // translate" infobar when the translation is accepted/declined 3 times,
 // only when not in incognito mode.
-TEST_F(TranslateManagerTest, BeforeTranslateExtraButtons) {
+TEST_F(TranslateManagerBrowserTest, BeforeTranslateExtraButtons) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
   TranslatePrefs translate_prefs(profile->GetPrefs());
@@ -1410,7 +1411,7 @@ TEST_F(TranslateManagerTest, BeforeTranslateExtraButtons) {
 
 // Tests that we don't show a translate infobar when a page instructs that it
 // should not be translated.
-TEST_F(TranslateManagerTest, NonTranslatablePage) {
+TEST_F(TranslateManagerBrowserTest, NonTranslatablePage) {
   // Simulate navigating to a page.
   SimulateNavigation(GURL("http://mail.google.fr"), "fr", false);
 
@@ -1426,7 +1427,7 @@ TEST_F(TranslateManagerTest, NonTranslatablePage) {
 }
 
 // Tests that the script is expired and refetched as expected.
-TEST_F(TranslateManagerTest, ScriptExpires) {
+TEST_F(TranslateManagerBrowserTest, ScriptExpires) {
   ExpireTranslateScriptImmediately();
 
   // Simulate navigating to a page and translating it.
@@ -1466,7 +1467,7 @@ TEST_F(TranslateManagerTest, ScriptExpires) {
   EXPECT_EQ("en", target_lang);
 }
 
-TEST_F(TranslateManagerTest, DownloadsAndHistoryNotTranslated) {
+TEST_F(TranslateManagerBrowserTest, DownloadsAndHistoryNotTranslated) {
   ASSERT_FALSE(TranslateManager::IsTranslatableURL(
       GURL(chrome::kChromeUIDownloadsURL)));
   ASSERT_FALSE(TranslateManager::IsTranslatableURL(

chrome/browser/translate/translate_manager_unittest.cc

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/translate/translate_manager.h"
+
+#include "content/public/common/url_constants.h"
+#include "googleurl/src/gurl.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#ifdef FILE_MANAGER_EXTENSION
+#include "chrome/browser/chromeos/extensions/file_manager_util.h"
+#include "extensions/common/constants.h"
+#endif
+
+typedef testing::Test TranslateManagerTest;
+
+TEST_F(TranslateManagerTest, CheckTranslatableURL) {
+  GURL empty_url = GURL("");
+  EXPECT_FALSE(TranslateManager::IsTranslatableURL(empty_url));
+
+  std::string chrome = std::string(chrome::kChromeUIScheme) + "://flags";
+  GURL chrome_url = GURL(chrome);
+  EXPECT_FALSE(TranslateManager::IsTranslatableURL(chrome_url));
+
+  std::string devtools = std::string(chrome::kChromeDevToolsScheme) + "://";
+  GURL devtools_url = GURL(devtools);
+  EXPECT_FALSE(TranslateManager::IsTranslatableURL(devtools_url));
+
+#ifdef FILE_MANAGER_EXTENSION
+  std::string filemanager =
+      std::string(extensions::kExtensionScheme) +
+      std::string("://") +
+      std::string(kFileBrowserDomain);
+  GURL filemanager_url = GURL(filemanager);
+  EXPECT_FALSE(TranslateManager::IsTranslatableURL(filemanager_url));
+#endif
+
+  std::string ftp = std::string(chrome::kFtpScheme) + "://google.com/pub";
+  GURL ftp_url = GURL(ftp);
+  EXPECT_FALSE(TranslateManager::IsTranslatableURL(ftp_url));
+
+  GURL right_url = GURL("http://www.tamurayukari.com/");
+  EXPECT_TRUE(TranslateManager::IsTranslatableURL(right_url));
+}

chrome/chrome_tests_unit.gypi

@@ -1194,6 +1194,7 @@
         'browser/thumbnails/render_widget_snapshot_taker_unittest.cc',
         'browser/thumbnails/simple_thumbnail_crop_unittest.cc',
         'browser/thumbnails/thumbnail_service_unittest.cc',
+        'browser/translate/translate_manager_unittest.cc',
         'browser/ui/android/tab_model/tab_model_unittest.cc',
         'browser/ui/ash/event_rewriter_unittest.cc',
         'browser/ui/ash/ime_controller_chromeos_unittest.cc',