diff options
7 files changed, 73 insertions, 3 deletions
diff --git a/chrome/browser/tab_contents/language_state.cc b/chrome/browser/tab_contents/language_state.cc index 87fe217..0a62027 100644 --- a/chrome/browser/tab_contents/language_state.cc +++ b/chrome/browser/tab_contents/language_state.cc @@ -9,7 +9,8 @@ LanguageState::LanguageState(NavigationController* nav_controller) : navigation_controller_(nav_controller), - translation_pending_(false) { + translation_pending_(false), + translation_declined_(false) { } LanguageState::~LanguageState() { @@ -25,6 +26,7 @@ void LanguageState::DidNavigate(bool reload) { current_lang_.clear(); translation_pending_ = false; + translation_declined_ = false; } void LanguageState::LanguageDetermined(const std::string& page_language) { diff --git a/chrome/browser/tab_contents/language_state.h b/chrome/browser/tab_contents/language_state.h index f8b2a68..21dea9d 100644 --- a/chrome/browser/tab_contents/language_state.h +++ b/chrome/browser/tab_contents/language_state.h @@ -52,6 +52,10 @@ class LanguageState { bool translation_pending() const { return translation_pending_; } void set_translation_pending(bool value) { translation_pending_ = value; } + // Whether the user has already declined to translate the page. + bool translation_declined() const { return translation_declined_; } + void set_translation_declined(bool value) { translation_declined_ = value; } + private: // The languages this page is in. Note that current_lang_ is different from // original_lang_ when the page has been translated. @@ -77,6 +81,11 @@ class LanguageState { // then we can get rid of that state. bool translation_pending_; + // Whether the user has declined to translate the page (by closing the infobar + // for example). This is necessary as a new infobar could be shown if a new + // load happens in the page after the user closed the infobar. + bool translation_declined_; + DISALLOW_COPY_AND_ASSIGN(LanguageState); }; diff --git a/chrome/browser/translate/translate_infobars_delegates.cc b/chrome/browser/translate/translate_infobars_delegates.cc index 06d7da8..9f76e85 100644 --- a/chrome/browser/translate/translate_infobars_delegates.cc +++ b/chrome/browser/translate/translate_infobars_delegates.cc @@ -26,6 +26,15 @@ bool TranslateInfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const { return (!!translate_delegate); } +void TranslateInfoBarDelegate::InfoBarDismissed() { + LanguageState& language_state = tab_contents_->language_state(); + if (!language_state.IsPageTranslated() && + !language_state.translation_pending()) { + // The user closed the infobar without clicking the translate button. + TranslationDeclined(); + } +} + void TranslateInfoBarDelegate::InfoBarClosed() { delete this; } @@ -92,6 +101,15 @@ void TranslateInfoBarDelegate::Translate() { tab_contents_->TranslatePage(original_lang_code(), target_lang_code()); } +void TranslateInfoBarDelegate::TranslationDeclined() { + // Remember that the user declined the translation so as to prevent showing a + // translate infobar for that page again. (TranslateManager initiates + // translations when getting a LANGUAGE_DETERMINED from the page, which + // happens when a load stops. That could happen multiple times, including + // after the user already declined the translation.) + tab_contents_->language_state().set_translation_declined(true); +} + bool TranslateInfoBarDelegate::IsLanguageBlacklisted() { if (state_ == kBeforeTranslate) { never_translate_language_ = diff --git a/chrome/browser/translate/translate_infobars_delegates.h b/chrome/browser/translate/translate_infobars_delegates.h index ceeda17..d2fc074 100644 --- a/chrome/browser/translate/translate_infobars_delegates.h +++ b/chrome/browser/translate/translate_infobars_delegates.h @@ -31,6 +31,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate { void ModifyOriginalLanguage(int lang_index); void ModifyTargetLanguage(int lang_index); void Translate(); + void TranslationDeclined(); bool IsLanguageBlacklisted(); void ToggleLanguageBlacklist(); bool IsSiteBlacklisted(); @@ -84,6 +85,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate { return this; } virtual bool EqualsDelegate(InfoBarDelegate* delegate) const; + virtual void InfoBarDismissed(); virtual void InfoBarClosed(); // Returns the printable version of the language code |language_code|. diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc index 212447b..a1ea1fd 100644 --- a/chrome/browser/translate/translate_manager.cc +++ b/chrome/browser/translate/translate_manager.cc @@ -65,8 +65,10 @@ void TranslateManager::Observe(NotificationType type, std::string language = *(Details<std::string>(details).ptr()); // We may get this notifications multiple times. Make sure to translate // only once. - if (!tab->language_state().translation_pending()) + if (!tab->language_state().translation_pending() && + !tab->language_state().translation_declined()) { InitiateTranslation(tab, language); + } break; } case NotificationType::PAGE_TRANSLATED: { diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc index e29cc42..34d81f2 100644 --- a/chrome/browser/translate/translate_manager_unittest.cc +++ b/chrome/browser/translate/translate_manager_unittest.cc @@ -23,6 +23,12 @@ class TranslateManagerTest : public RenderViewHostTestHarness { const std::wstring& contents, const std::string& lang) { NavigateAndCommit(url); + SimulateOnPageContents(url, page_id, contents, lang); + } + + void SimulateOnPageContents(const GURL& url, int page_id, + const std::wstring& contents, + const std::string& lang) { rvh()->TestOnMessageReceived(ViewHostMsg_PageContents(0, url, page_id, contents, lang)); } @@ -121,7 +127,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { EXPECT_EQ(new_target_lang, target_lang); } -// Test auto-translate on page. +// Tests auto-translate on page. TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) { // Simulate navigating to a page and gettings its language. SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr"); @@ -153,3 +159,33 @@ TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) { // This should not have triggered a translate. EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); } + +// Tests that multiple OnPageContents do not cause multiple infobars. +TEST_F(TranslateManagerTest, MultipleOnPageContents) { + // Simulate navigating to a page and gettings its language. + SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr"); + + // Simulate clicking 'Nope' (don't translate). + ASSERT_EQ(1, contents()->infobar_delegate_count()); + TranslateInfoBarDelegate* infobar = + contents()->GetInfoBarDelegateAt(0)->AsTranslateInfoBarDelegate(); + ASSERT_TRUE(infobar != NULL); + infobar->TranslationDeclined(); + contents()->RemoveInfoBar(infobar); + EXPECT_EQ(0, contents()->infobar_delegate_count()); + + // Send a new PageContents, we should not show an infobar. + SimulateOnPageContents(GURL("http://www.google.fr"), 0, L"Le Google", "fr"); + EXPECT_EQ(0, contents()->infobar_delegate_count()); + + // Do the same steps but simulate closing the infobar this time. + SimulateNavigation(GURL("http://www.youtube.fr"), 1, L"Le YouTube", "fr"); + ASSERT_EQ(1, contents()->infobar_delegate_count()); + infobar = contents()->GetInfoBarDelegateAt(0)->AsTranslateInfoBarDelegate(); + ASSERT_TRUE(infobar != NULL); + infobar->InfoBarDismissed(); // Simulates closing the infobar. + contents()->RemoveInfoBar(infobar); + EXPECT_EQ(0, contents()->infobar_delegate_count()); + SimulateOnPageContents(GURL("http://www.youtube.fr"), 1, L"Le YouTube", "fr"); + EXPECT_EQ(0, contents()->infobar_delegate_count()); +} diff --git a/chrome/browser/views/infobars/translate_infobars.cc b/chrome/browser/views/infobars/translate_infobars.cc index d9660d2..3d5bb78 100644 --- a/chrome/browser/views/infobars/translate_infobars.cc +++ b/chrome/browser/views/infobars/translate_infobars.cc @@ -600,6 +600,7 @@ void TranslateInfoBar::ButtonPressed( UpdateState(TranslateInfoBarDelegate::kTranslating); GetDelegate()->Translate(); } else if (sender == deny_button_) { + GetDelegate()->TranslationDeclined(); RemoveInfoBar(); } else { // Let base InfoBar handle close button. InfoBar::ButtonPressed(sender, event); |