Commit 10932de9 authored by Anthony Vallee-Dubois's avatar Anthony Vallee-Dubois Committed by Commit Bot

Refactor TranslateManager::InitiateTranslation

This CL changes InitiateTranslation in 2 ways:
- It decouples the actual decision making from the triggering, metrics
  recording, and Ranker Event logging
- It avoids returning early, to enable some upcoming behavior that
  would have required inserting the same function call in multiple
  places in the function

Functionally, there should be no change introduced from this CL,
although the TranslateManager performs marginally more work on cases
that would previously return early.

Change-Id: Ib78f7f5d42c651c1089711e0f6e14f362fdddebd
Reviewed-on: https://chromium-review.googlesource.com/c/1431480
Commit-Queue: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: default avataranthonyvd <anthonyvd@chromium.org>
Reviewed-by: default avatarMathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625683}
parent 0bcb1c0a
...@@ -174,6 +174,8 @@ class TranslateManager { ...@@ -174,6 +174,8 @@ class TranslateManager {
private: private:
friend class translate::testing::TranslateManagerTest; friend class translate::testing::TranslateManagerTest;
struct TranslateTriggerDecision;
// Sends a translation request to the TranslateDriver. // Sends a translation request to the TranslateDriver.
void DoTranslatePage(const std::string& translate_script, void DoTranslatePage(const std::string& translate_script,
const std::string& source_lang, const std::string& source_lang,
...@@ -197,6 +199,58 @@ class TranslateManager { ...@@ -197,6 +199,58 @@ class TranslateManager {
void AddTargetLanguageToAcceptLanguages( void AddTargetLanguageToAcceptLanguages(
const std::string& target_language_code); const std::string& target_language_code);
// Creates a TranslateTriggerDecision and filters out possible outcomes based
// on the current state. Returns a decision objects ready to be used to
// trigger behavior and record metrics.
const TranslateTriggerDecision ComputePossibleOutcomes(
TranslatePrefs* translate_prefs,
const std::string& page_language_code,
const std::string& target_lang);
// Determines whether translation is even possible (connected to the internet,
// source and target languages don't match, etc) and mutates |decision| based
// on the result.
void FilterIsTranslatePossible(TranslateTriggerDecision* decision,
TranslatePrefs* translate_prefs,
const std::string& page_language_code,
const std::string& target_lang);
// Determines whether auto-translate is a possible outcome, and mutates
// |decision| accordingly.
void FilterAutoTranslate(TranslateTriggerDecision* decision,
TranslatePrefs* translate_prefs,
const std::string& page_language_code);
// Determines whether user prefs prohibit translations for this specific
// navigation. For example, a user can select "never translate this language".
// Mutates |decision| accordingly.
void FilterForUserPrefs(TranslateTriggerDecision* decision,
TranslatePrefs* translate_prefs,
const std::string& page_language_code);
// Enables or disables the translate omnibox icon depending on |decision|. The
// icon is always shown if translate UI is shown, auto-translation happens, or
// the UI is suppressed by ranker.
void MaybeShowOmniboxIcon(const TranslateTriggerDecision& decision);
// Shows the UI or auto-translates based on the state of |decision|. Returns
// true if UI was shown, false otherwise.
bool MaterializeDecision(const TranslateTriggerDecision& decision,
TranslatePrefs* translate_prefs,
const std::string& page_language_code,
const std::string target_lang);
// Records all UMA metrics related to the current |decision|.
void RecordDecisionMetrics(const TranslateTriggerDecision& decision,
const std::string& page_language_code,
bool ui_shown);
// Records the RankerEvent associated with the current |decision|.
void RecordDecisionRankerEvent(const TranslateTriggerDecision& decision,
TranslatePrefs* translate_prefs,
const std::string& page_language_code,
const std::string& target_lang);
// Sequence number of the current page. // Sequence number of the current page.
int page_seq_no_; int page_seq_no_;
......
...@@ -664,6 +664,11 @@ TEST_F(TranslateManagerTest, ...@@ -664,6 +664,11 @@ TEST_F(TranslateManagerTest,
TEST_F(TranslateManagerTest, DontTranslateOffline) { TEST_F(TranslateManagerTest, DontTranslateOffline) {
TranslateManager::SetIgnoreMissingKeyForTesting(true); TranslateManager::SetIgnoreMissingKeyForTesting(true);
TranslateAcceptLanguages accept_languages(&prefs_, accept_languages_prefs);
ON_CALL(mock_translate_client_, GetTranslateAcceptLanguages())
.WillByDefault(Return(&accept_languages));
translate_manager_.reset(new translate::TranslateManager( translate_manager_.reset(new translate::TranslateManager(
&mock_translate_client_, &mock_translate_ranker_, &mock_language_model_)); &mock_translate_client_, &mock_translate_ranker_, &mock_language_model_));
...@@ -675,19 +680,18 @@ TEST_F(TranslateManagerTest, DontTranslateOffline) { ...@@ -675,19 +680,18 @@ TEST_F(TranslateManagerTest, DontTranslateOffline) {
translate_manager_->GetLanguageState().LanguageDetermined("de", true); translate_manager_->GetLanguageState().LanguageDetermined("de", true);
// In the offline case, Initiate will early-out before even hitting the API // In the offline case, Initiate won't trigger any translate behavior, so no
// key test. // UI showing and no auto-translate.
network_notifier_.SimulateOffline(); network_notifier_.SimulateOffline();
translate_manager_->InitiateTranslation("de"); translate_manager_->InitiateTranslation("de");
histogram_tester.ExpectTotalCount(kInitiationStatusName, 0); EXPECT_THAT(histogram_tester.GetAllSamples(kInitiationStatusName),
Not(Contains(Bucket(INITIATION_STATUS_SHOW_INFOBAR, 1))));
// In the online case, InitiateTranslation will proceed past early out tests. EXPECT_THAT(histogram_tester.GetAllSamples(kInitiationStatusName),
network_notifier_.SimulateOnline(); Not(Contains(Bucket(INITIATION_STATUS_SHOW_ICON, 1))));
translate_manager_->InitiateTranslation("de"); EXPECT_THAT(histogram_tester.GetAllSamples(kInitiationStatusName),
histogram_tester.ExpectUniqueSample( Not(Contains(Bucket(INITIATION_STATUS_AUTO_BY_CONFIG, 1))));
kInitiationStatusName, EXPECT_THAT(histogram_tester.GetAllSamples(kInitiationStatusName),
translate::TranslateBrowserMetrics::INITIATION_STATUS_DISABLED_BY_PREFS, Not(Contains(Bucket(INITIATION_STATUS_AUTO_BY_LINK, 1))));
1);
} }
TEST_F(TranslateManagerTest, TestRecordTranslateEvent) { TEST_F(TranslateManagerTest, TestRecordTranslateEvent) {
......
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