Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(26)

Unified Diff: components/translate/core/browser/translate_manager.cc

Issue 2697703004: Allow TranslateRanker to override decisions taken by heuristics. (Closed)
Patch Set: Make ShouldSuppressBubbleUI easier to read. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/translate/core/browser/translate_manager.cc
diff --git a/components/translate/core/browser/translate_manager.cc b/components/translate/core/browser/translate_manager.cc
index ed9d73548dd99f7cfb8149ba7201446aa4eb22ea..f09b9d7f8caa2b9f94933238af989b88c1b473be 100644
--- a/components/translate/core/browser/translate_manager.cc
+++ b/components/translate/core/browser/translate_manager.cc
@@ -165,8 +165,7 @@ void TranslateManager::InitiateTranslation(const std::string& page_lang) {
if (net::NetworkChangeNotifier::IsOffline())
return;
- if (!ignore_missing_key_for_testing_ &&
- !::google_apis::HasKeysConfigured()) {
+ if (!ignore_missing_key_for_testing_ && !::google_apis::HasKeysConfigured()) {
// Without an API key, translate won't work, so don't offer to translate
// in the first place. Leave prefs::kEnableTranslate on, though, because
// that settings syncs and we don't want to turn off translate everywhere
@@ -228,22 +227,13 @@ void TranslateManager::InitiateTranslation(const std::string& page_lang) {
return;
}
- bool ranker_should_offer_translation = true;
- if (translate_ranker_->IsQueryEnabled() ||
- translate_ranker_->IsEnforcementEnabled()) {
- ranker_should_offer_translation = translate_ranker_->ShouldOfferTranslation(
- *translate_prefs, language_code, target_lang);
- translate_event_->set_ranker_request_timestamp_sec(
- (base::TimeTicks::Now() - base::TimeTicks()).InSeconds());
- translate_event_->set_ranker_version(translate_ranker_->GetModelVersion());
- translate_event_->set_ranker_response(
- ranker_should_offer_translation
- ? metrics::TranslateEventProto::SHOW
- : metrics::TranslateEventProto::DONT_SHOW);
- }
+ // Querying the ranker now, but not exiting immediately so that we may log
+ // other potential suppression reasons.
+ bool should_offer_translation = translate_ranker_->ShouldOfferTranslation(
+ *translate_prefs, language_code, target_lang, translate_event_.get());
- // Nothing to do if either the language Chrome is in or the language of the
- // page is not supported by the translation server.
+ // Nothing to do if either the language Chrome is in or the language of
+ // the page is not supported by the translation server.
if (target_lang.empty() ||
!TranslateDownloadManager::IsSupportedLanguage(language_code)) {
TranslateBrowserMetrics::ReportInitiationStatus(
@@ -257,8 +247,7 @@ void TranslateManager::InitiateTranslation(const std::string& page_lang) {
TranslateAcceptLanguages* accept_languages =
translate_client_->GetTranslateAcceptLanguages();
// Don't translate any user black-listed languages.
- if (!translate_prefs->CanTranslateLanguage(accept_languages,
- language_code)) {
+ if (!translate_prefs->CanTranslateLanguage(accept_languages, language_code)) {
TranslateBrowserMetrics::ReportInitiationStatus(
TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_CONFIG);
RecordTranslateEvent(
@@ -312,8 +301,7 @@ void TranslateManager::InitiateTranslation(const std::string& page_lang) {
return;
}
- if (!ranker_should_offer_translation &&
- translate_ranker_->IsEnforcementEnabled()) {
+ if (!should_offer_translation) {
TranslateBrowserMetrics::ReportInitiationStatus(
TranslateBrowserMetrics::INITIATION_STATUS_ABORTED_BY_RANKER);
RecordTranslateEvent(metrics::TranslateEventProto::DISABLED_BY_RANKER);
@@ -325,10 +313,8 @@ void TranslateManager::InitiateTranslation(const std::string& page_lang) {
// Prompts the user if they want the page translated.
translate_client_->ShowTranslateUI(translate::TRANSLATE_STEP_BEFORE_TRANSLATE,
- language_code,
- target_lang,
- TranslateErrors::NONE,
- false);
+ language_code, target_lang,
+ TranslateErrors::NONE, false);
}
bool TranslateManager::LanguageInULP(const std::string& language) const {
@@ -375,11 +361,9 @@ void TranslateManager::TranslatePage(const std::string& original_source_lang,
}
// Trigger the "translating now" UI.
- translate_client_->ShowTranslateUI(translate::TRANSLATE_STEP_TRANSLATING,
- source_lang,
- target_lang,
- TranslateErrors::NONE,
- triggered_from_menu);
+ translate_client_->ShowTranslateUI(
+ translate::TRANSLATE_STEP_TRANSLATING, source_lang, target_lang,
+ TranslateErrors::NONE, triggered_from_menu);
TranslateScript* script = TranslateDownloadManager::GetInstance()->script();
DCHECK(script != NULL);
@@ -392,9 +376,9 @@ void TranslateManager::TranslatePage(const std::string& original_source_lang,
// The script is not available yet. Queue that request and query for the
// script. Once it is downloaded we'll do the translate.
- TranslateScript::RequestCallback callback = base::Bind(
- &TranslateManager::OnTranslateScriptFetchComplete, GetWeakPtr(),
- source_lang, target_lang);
+ TranslateScript::RequestCallback callback =
+ base::Bind(&TranslateManager::OnTranslateScriptFetchComplete,
+ GetWeakPtr(), source_lang, target_lang);
script->Request(callback);
}
@@ -418,8 +402,7 @@ void TranslateManager::ReportLanguageDetectionError() {
translate_driver_->GetLastCommittedURL().spec());
report_error_url =
- net::AppendQueryParameter(report_error_url,
- kSourceLanguageQueryName,
+ net::AppendQueryParameter(report_error_url, kSourceLanguageQueryName,
language_state_.original_language());
report_error_url = translate::AddHostLocaleToUrl(report_error_url);
@@ -432,8 +415,8 @@ void TranslateManager::DoTranslatePage(const std::string& translate_script,
const std::string& source_lang,
const std::string& target_lang) {
language_state_.set_translation_pending(true);
- translate_driver_->TranslatePage(
- page_seq_no_, translate_script, source_lang, target_lang);
+ translate_driver_->TranslatePage(page_seq_no_, translate_script, source_lang,
+ target_lang);
}
// Notifies |g_callback_list_| of translate errors.
@@ -463,9 +446,7 @@ void TranslateManager::PageTranslated(const std::string& source_lang,
}
translate_client_->ShowTranslateUI(translate::TRANSLATE_STEP_AFTER_TRANSLATE,
- source_lang,
- target_lang,
- error_type,
+ source_lang, target_lang, error_type,
false);
NotifyTranslateError(error_type);
}
@@ -486,11 +467,8 @@ void TranslateManager::OnTranslateScriptFetchComplete(
DoTranslatePage(translate_script->data(), source_lang, target_lang);
} else {
translate_client_->ShowTranslateUI(
- translate::TRANSLATE_STEP_TRANSLATE_ERROR,
- source_lang,
- target_lang,
- TranslateErrors::NETWORK,
- false);
+ translate::TRANSLATE_STEP_TRANSLATE_ERROR, source_lang, target_lang,
+ TranslateErrors::NETWORK, false);
NotifyTranslateError(TranslateErrors::NETWORK);
}
}
@@ -601,13 +579,41 @@ void TranslateManager::InitTranslateEvent(const std::string& src_lang,
}
void TranslateManager::RecordTranslateEvent(int event_type) {
- DCHECK(metrics::TranslateEventProto::EventType_IsValid(event_type));
- translate_event_->set_event_type(
- static_cast<metrics::TranslateEventProto::EventType>(event_type));
- translate_event_->set_event_timestamp_sec(
- (base::TimeTicks::Now() - base::TimeTicks()).InSeconds());
- translate_ranker_->AddTranslateEvent(*translate_event_,
- translate_driver_->GetVisibleURL());
+ translate_ranker_->RecordTranslateEvent(
+ event_type, translate_driver_->GetVisibleURL(), translate_event_.get());
+}
+
+bool TranslateManager::ShouldOverrideDecision(int event_type) {
+ return translate_ranker_->ShouldOverrideDecision(
+ event_type, translate_driver_->GetVisibleURL(), translate_event_.get());
+}
+
+bool TranslateManager::ShouldSuppressBubbleUI(
+ bool triggered_from_menu,
+ const std::string& source_language) {
+ // Suppress the UI if the user navigates to a page with
+ // the same language as the previous page. In the new UI,
+ // continue offering translation after the user navigates
+ // to another page.
+ language_state_.SetTranslateEnabled(true);
+ if (!base::FeatureList::IsEnabled(translate::kTranslateUI2016Q2) &&
+ !language_state_.HasLanguageChanged() &&
+ !ShouldOverrideDecision(
+ metrics::TranslateEventProto::MATCHES_PREVIOUS_LANGUAGE)) {
+ return true;
+ }
+
+ // Suppress the UI if the user denied translation for this language
+ // too often.
+ if (!triggered_from_menu &&
+ translate_client_->GetTranslatePrefs()->IsTooOftenDenied(
+ source_language) &&
+ !ShouldOverrideDecision(
+ metrics::TranslateEventProto::LANGUAGE_DISABLED_BY_AUTO_BLACKLIST)) {
+ return true;
+ }
+
+ return false;
}
} // namespace translate

Powered by Google App Engine
This is Rietveld 408576698