Skip to content

Commit

Permalink
Translate: remove --disable-translate flag
Browse files Browse the repository at this point in the history
Today, the flag is not maintained well, and is not consistently checked
to invoke the translation feature.

For testing, the translate feature is automatically disabled when
the API Key is not used to compile chromium. This check is also rough,
but should work similarly.
See, https://codereview.chromium.org/1185703007

To disable the feature completely, PolicyList can overwrite the translate
setting. This should work as users disable the feature in chrome://settings.
See, https://www.chromium.org/administrators/policy-list-3#TranslateEnabled

BUG=n/a

Review-Url: https://codereview.chromium.org/2819813002
Cr-Commit-Position: refs/heads/master@{#466605}
  • Loading branch information
toyoshim authored and Commit bot committed Apr 24, 2017
1 parent d5521d9 commit 4baa420
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ enum MetricsNameIndex {
// below.
enum InitiationStatusType {
INITIATION_STATUS_DISABLED_BY_PREFS,
INITIATION_STATUS_DISABLED_BY_SWITCH,
DEPRECATE_INITIATION_STATUS_DISABLED_BY_SWITCH,
INITIATION_STATUS_DISABLED_BY_CONFIG,
INITIATION_STATUS_LANGUAGE_IS_NOT_SUPPORTED,
INITIATION_STATUS_URL_IS_NOT_SUPPORTED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class MetricsRecorder {
}

void CheckInitiationStatus(int expected_disabled_by_prefs,
int expected_disabled_by_switch,
int expected_disabled_by_config,
int expected_disabled_by_build,
int expected_language_is_not_supported,
Expand All @@ -46,10 +45,6 @@ class MetricsRecorder {
EXPECT_EQ(expected_disabled_by_prefs,
GetCountWithoutSnapshot(translate::TranslateBrowserMetrics::
INITIATION_STATUS_DISABLED_BY_PREFS));
EXPECT_EQ(
expected_disabled_by_switch,
GetCountWithoutSnapshot(translate::TranslateBrowserMetrics::
INITIATION_STATUS_DISABLED_BY_SWITCH));
EXPECT_EQ(
expected_disabled_by_config,
GetCountWithoutSnapshot(translate::TranslateBrowserMetrics::
Expand Down Expand Up @@ -132,46 +127,43 @@ TEST(TranslateBrowserMetricsTest, ReportInitiationStatus) {
MetricsRecorder recorder(translate::TranslateBrowserMetrics::GetMetricsName(
translate::TranslateBrowserMetrics::UMA_INITIATION_STATUS));

recorder.CheckInitiationStatus(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
recorder.CheckInitiationStatus(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_PREFS);
recorder.CheckInitiationStatus(1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_SWITCH);
recorder.CheckInitiationStatus(1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
recorder.CheckInitiationStatus(1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_CONFIG);
recorder.CheckInitiationStatus(1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0);
recorder.CheckInitiationStatus(1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_KEY);
recorder.CheckInitiationStatus(1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
recorder.CheckInitiationStatus(1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::
INITIATION_STATUS_LANGUAGE_IS_NOT_SUPPORTED);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0);
recorder.CheckInitiationStatus(1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::
INITIATION_STATUS_MIME_TYPE_IS_NOT_SUPPORTED);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::
INITIATION_STATUS_URL_IS_NOT_SUPPORTED);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_SIMILAR_LANGUAGES);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_ACCEPT_LANGUAGES);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_AUTO_BY_CONFIG);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_AUTO_BY_LINK);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0);
translate::TranslateBrowserMetrics::ReportInitiationStatus(
translate::TranslateBrowserMetrics::INITIATION_STATUS_SHOW_INFOBAR);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1);
recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1);
}

TEST(TranslateBrowserMetricsTest, ReportLanguageDetectionError) {
Expand Down
22 changes: 6 additions & 16 deletions components/translate/core/browser/translate_download_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "components/translate/core/browser/translate_download_manager.h"

#include "base/command_line.h"
#include "base/logging.h"
#include "base/memory/singleton.h"
#include "components/prefs/pref_service.h"
Expand All @@ -31,29 +30,20 @@ void TranslateDownloadManager::Shutdown() {
}

// static
void TranslateDownloadManager::RequestLanguageList() {
void TranslateDownloadManager::RequestLanguageList(PrefService* prefs) {
// We don't want to do this when translate is disabled.
DCHECK(prefs != NULL);
if (!prefs->GetBoolean(prefs::kEnableTranslate))
return;

TranslateLanguageList* language_list = GetInstance()->language_list();
if (!language_list) {
NOTREACHED();
return;
}

language_list->RequestLanguageList();
}

// static
void TranslateDownloadManager::RequestLanguageList(PrefService* prefs) {
// We don't want to do this when translate is disabled.
DCHECK(prefs != NULL);
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
translate::switches::kDisableTranslate) ||
!prefs->GetBoolean(prefs::kEnableTranslate)) {
return;
}

RequestLanguageList();
}

// static
void TranslateDownloadManager::GetSupportedLanguages(
std::vector<std::string>* languages) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,10 @@ class TranslateDownloadManager {
TranslateScript* script() { return script_.get(); }

// Let the caller decide if and when we should fetch the language list from
// the translate server. This is a NOOP if switches::kDisableTranslate is set
// or if prefs::kEnableTranslate is set to false.
// the translate server. This is a NOOP if prefs::kEnableTranslate is set to
// false.
static void RequestLanguageList(PrefService* prefs);

// Fetches the language list from the translate server.
static void RequestLanguageList();

// Fills |languages| with the list of languages that the translate server can
// translate to and from.
static void GetSupportedLanguages(std::vector<std::string>* languages);
Expand Down
9 changes: 0 additions & 9 deletions components/translate/core/browser/translate_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,6 @@ void TranslateManager::InitiateTranslation(const std::string& page_lang) {
return;
}

// Allow disabling of translate from the command line to assist with
// automated browser testing.
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
translate::switches::kDisableTranslate)) {
TranslateBrowserMetrics::ReportInitiationStatus(
TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_SWITCH);
return;
}

// MHTML pages currently cannot be translated.
// See bug: 217945.
if (translate_driver_->GetContentsMimeType() == "multipart/related") {
Expand Down
5 changes: 0 additions & 5 deletions components/translate/core/common/translate_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@
namespace translate {
namespace switches {

// Allows disabling of translate from the command line to assist with automated
// browser testing (e.g. Selenium/WebDriver). Normal browser users should
// disable translate with the preference.
const char kDisableTranslate[] = "disable-translate";

// Overrides the default server used for Google Translate.
const char kTranslateScriptURL[] = "translate-script-url";

Expand Down
1 change: 0 additions & 1 deletion components/translate/core/common/translate_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
namespace translate {
namespace switches {

extern const char kDisableTranslate[];
extern const char kTranslateScriptURL[];
extern const char kTranslateSecurityOrigin[];
extern const char kTranslateRankerModelURL[];
Expand Down

0 comments on commit 4baa420

Please sign in to comment.