Commit 85e49634 authored by Anthony Cui's avatar Anthony Cui Committed by Commit Bot

Consolidate URL scheme translate logic into TranslateService::IsTranslatableURL

TranslateUtils.canTranslateCurrentTab performs URL checks that overlap
with TranslateService::IsTranslatableURL. Consolidate the logic in these
two places.

Some of these checks are Android-specific. However, it is still
preferable to move them to a single, shared location. This is also
important for supporting logging of manual translate availability from
menus, which is added in this change
https://chromium-review.googlesource.com/c/chromium/src/+/2468796

Bug: 1127094
Change-Id: Ifd626f2f9c7d6c7c929e36d07c420f87314d8bd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2444549
Commit-Queue: Anthony Cui <cuianthony@chromium.org>
Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Reviewed-by: default avatarScott Little <sclittle@chromium.org>
Reviewed-by: default avatarMegan Jablonski <megjablon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819015}
parent e08c049c
...@@ -4,10 +4,7 @@ ...@@ -4,10 +4,7 @@
package org.chromium.chrome.browser.translate; package org.chromium.chrome.browser.translate;
import android.text.TextUtils;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.components.embedder_support.util.UrlConstants;
/** /**
* Utility classes related to the translate feature. * Utility classes related to the translate feature.
...@@ -15,15 +12,10 @@ import org.chromium.components.embedder_support.util.UrlConstants; ...@@ -15,15 +12,10 @@ import org.chromium.components.embedder_support.util.UrlConstants;
public class TranslateUtils { public class TranslateUtils {
/** /**
* Returns true iff the content displayed in the current tab can be translated. * Returns true iff the content displayed in the current tab can be translated.
*
* @param tab The tab in question. * @param tab The tab in question.
*/ */
public static boolean canTranslateCurrentTab(Tab tab) { public static boolean canTranslateCurrentTab(Tab tab) {
String url = tab.getUrlString(); return tab.getWebContents() != null && TranslateBridge.canManuallyTranslate(tab);
boolean isChromeScheme = url.startsWith(UrlConstants.CHROME_URL_PREFIX)
|| url.startsWith(UrlConstants.CHROME_NATIVE_URL_PREFIX);
boolean isFileScheme = url.startsWith(UrlConstants.FILE_URL_PREFIX);
boolean isContentScheme = url.startsWith(UrlConstants.CONTENT_URL_PREFIX);
return !isChromeScheme && !isFileScheme && !isContentScheme && !TextUtils.isEmpty(url)
&& tab.getWebContents() != null && TranslateBridge.canManuallyTranslate(tab);
} }
} }
...@@ -221,6 +221,9 @@ public class AppMenuPropertiesDelegateUnitTest { ...@@ -221,6 +221,9 @@ public class AppMenuPropertiesDelegateUnitTest {
setUpMocksForPageMenu(); setUpMocksForPageMenu();
when(mTab.getUrlString()).thenReturn(UrlConstants.NTP_URL); when(mTab.getUrlString()).thenReturn(UrlConstants.NTP_URL);
when(mTab.isNativePage()).thenReturn(true); when(mTab.isNativePage()).thenReturn(true);
doReturn(false)
.when(mAppMenuPropertiesDelegate)
.shouldShowTranslateMenuItem(any(Tab.class));
Assert.assertEquals(MenuGroup.PAGE_MENU, mAppMenuPropertiesDelegate.getMenuGroup()); Assert.assertEquals(MenuGroup.PAGE_MENU, mAppMenuPropertiesDelegate.getMenuGroup());
Menu menu = createTestMenu(); Menu menu = createTestMenu();
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "chrome/browser/translate/chrome_translate_client.h" #include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "components/language/core/browser/language_model.h" #include "components/language/core/browser/language_model.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/translate/core/browser/translate_download_manager.h" #include "components/translate/core/browser/translate_download_manager.h"
...@@ -23,6 +24,7 @@ ...@@ -23,6 +24,7 @@
#include "content/public/browser/network_service_instance.h" #include "content/public/browser/network_service_instance.h"
#include "content/public/common/url_constants.h" #include "content/public/common/url_constants.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/url_constants.h"
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
#include "chrome/browser/chromeos/file_manager/app_id.h" #include "chrome/browser/chromeos/file_manager/app_id.h"
...@@ -32,7 +34,7 @@ ...@@ -32,7 +34,7 @@
namespace { namespace {
// The singleton instance of TranslateService. // The singleton instance of TranslateService.
TranslateService* g_translate_service = nullptr; TranslateService* g_translate_service = nullptr;
} } // namespace
TranslateService::TranslateService() TranslateService::TranslateService()
: resource_request_allowed_notifier_( : resource_request_allowed_notifier_(
...@@ -129,15 +131,21 @@ std::string TranslateService::GetTargetLanguage( ...@@ -129,15 +131,21 @@ std::string TranslateService::GetTargetLanguage(
// static // static
bool TranslateService::IsTranslatableURL(const GURL& url) { bool TranslateService::IsTranslatableURL(const GURL& url) {
// A URLs is translatable unless it is one of the following: // A URL is translatable unless it is one of the following:
// - empty (can happen for popups created with window.open("")) // - empty (can happen for popups created with window.open(""))
// - an internal URL (chrome:// and others) // - an internal URL:
// - chrome:// and chrome-native:// for all platforms
// - file:// and content:// are Android-specific, and are thus impossible
// on other platforms
// - the devtools (which is considered UI) // - the devtools (which is considered UI)
// - about:blank // - about:blank
// - Chrome OS file manager extension // - Chrome OS file manager extension
// - an FTP page (as FTP pages tend to have long lists of filenames that may // - an FTP page (as FTP pages tend to have long lists of filenames that may
// confuse the CLD) // confuse the CLD)
return !url.is_empty() && !url.SchemeIs(content::kChromeUIScheme) && return !url.is_empty() && !url.SchemeIs(content::kChromeUIScheme) &&
!url.SchemeIs(chrome::kChromeNativeScheme) &&
!url.SchemeIs(url::kFileScheme) &&
!url.SchemeIs(url::kContentScheme) &&
!url.SchemeIs(content::kChromeDevToolsScheme) && !url.IsAboutBlank() && !url.SchemeIs(content::kChromeDevToolsScheme) && !url.IsAboutBlank() &&
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
!(url.SchemeIs(extensions::kExtensionScheme) && !(url.SchemeIs(extensions::kExtensionScheme) &&
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "content/public/test/browser_task_environment.h" #include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/url_constants.h"
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
#include "chrome/browser/chromeos/file_manager/app_id.h" #include "chrome/browser/chromeos/file_manager/app_id.h"
...@@ -35,6 +36,19 @@ TEST(TranslateServiceTest, CheckTranslatableURL) { ...@@ -35,6 +36,19 @@ TEST(TranslateServiceTest, CheckTranslatableURL) {
GURL devtools_url = GURL(devtools); GURL devtools_url = GURL(devtools);
EXPECT_FALSE(TranslateService::IsTranslatableURL(devtools_url)); EXPECT_FALSE(TranslateService::IsTranslatableURL(devtools_url));
std::string chrome_native = std::string(chrome::kChromeNativeScheme) + "://";
GURL chrome_native_url = GURL(chrome_native);
EXPECT_FALSE(TranslateService::IsTranslatableURL(chrome_native_url));
// kContentScheme and kFileScheme are Android-specific.
std::string content = std::string(url::kContentScheme) + "://";
GURL content_url = GURL(content);
EXPECT_FALSE(TranslateService::IsTranslatableURL(content_url));
std::string file = std::string(url::kFileScheme) + "://";
GURL file_url = GURL(file);
EXPECT_FALSE(TranslateService::IsTranslatableURL(file_url));
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
std::string filemanager = std::string(extensions::kExtensionScheme) + std::string filemanager = std::string(extensions::kExtensionScheme) +
std::string("://") + std::string("://") +
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment