diff options
author | jcivelli@chromium.org <jcivelli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-20 06:48:04 +0000 |
---|---|---|
committer | jcivelli@chromium.org <jcivelli@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-20 06:48:04 +0000 |
commit | 70af94ccad4515ab239e77e34d46613fee9600fa (patch) | |
tree | b8baeb8935e392c02bb51954af32618e303e072d /chrome/browser/translate | |
parent | 1db15727b39ee2d203035a969ce0ec8f212b83d9 (diff) | |
download | chromium_src-70af94ccad4515ab239e77e34d46613fee9600fa.zip chromium_src-70af94ccad4515ab239e77e34d46613fee9600fa.tar.gz chromium_src-70af94ccad4515ab239e77e34d46613fee9600fa.tar.bz2 |
Revert 53016 - Makes the browser fetch the translate script regularly.
New versions of that script may get pushed from time to time.
Fetching the script again ensures people that don't restart their browser for long periods of time still get the new versions.
Also renamed the enum from constant style to macro-style, as it is the Chromium way to go from the Chromium style guide (which is in that regard different than the Google style guide).
BUG=None
TEST=Run the unit-tests.
Original review:
http://codereview.chromium.org/3034013/show
Review URL: http://codereview.chromium.org/3041010
TBR=jcivelli@chromium.org
Review URL: http://codereview.chromium.org/2881024
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@53019 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/translate')
-rw-r--r-- | chrome/browser/translate/translate_infobar_delegate.cc | 36 | ||||
-rw-r--r-- | chrome/browser/translate/translate_infobar_delegate.h | 14 | ||||
-rw-r--r-- | chrome/browser/translate/translate_manager.cc | 15 | ||||
-rw-r--r-- | chrome/browser/translate/translate_manager.h | 11 | ||||
-rw-r--r-- | chrome/browser/translate/translate_manager_unittest.cc | 73 |
5 files changed, 42 insertions, 107 deletions
diff --git a/chrome/browser/translate/translate_infobar_delegate.cc b/chrome/browser/translate/translate_infobar_delegate.cc index e1a801b..2233991 100644 --- a/chrome/browser/translate/translate_infobar_delegate.cc +++ b/chrome/browser/translate/translate_infobar_delegate.cc @@ -23,7 +23,7 @@ TranslateInfoBarDelegate* TranslateInfoBarDelegate::CreateDelegate( TabContents* tab_contents, const std::string& original_language, const std::string& target_language) { - DCHECK(type != TRANSLATION_ERROR); + DCHECK(type != kTranslationError); if (!TranslateManager::IsSupportedLanguage(original_language) || !TranslateManager::IsSupportedLanguage(target_language)) { return NULL; @@ -42,7 +42,7 @@ TranslateInfoBarDelegate* TranslateInfoBarDelegate::CreateErrorDelegate( TabContents* tab_contents, const std::string& original_language, const std::string& target_language) { - return new TranslateInfoBarDelegate(TRANSLATION_ERROR, error, tab_contents, + return new TranslateInfoBarDelegate(kTranslationError, error, tab_contents, original_language, target_language); } @@ -54,7 +54,7 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate( const std::string& target_language) : InfoBarDelegate(tab_contents), type_(type), - background_animation_(NONE), + background_animation_(kNone), tab_contents_(tab_contents), original_language_index_(-1), initial_original_language_index_(-1), @@ -62,8 +62,8 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate( error_(error), infobar_view_(NULL), prefs_(tab_contents_->profile()->GetPrefs()) { - DCHECK((type_ != TRANSLATION_ERROR && error == TranslateErrors::NONE) || - (type_ == TRANSLATION_ERROR && error != TranslateErrors::NONE)); + DCHECK((type_ != kTranslationError && error == TranslateErrors::NONE) || + (type_ == kTranslationError && error != TranslateErrors::NONE)); std::vector<std::string> language_codes; TranslateManager::GetSupportedLanguages(&language_codes); @@ -123,7 +123,7 @@ void TranslateInfoBarDelegate::SetOriginalLanguage(int language_index) { original_language_index_ = language_index; if (infobar_view_) infobar_view_->OriginalLanguageChanged(); - if (type_ == AFTER_TRANSLATE) + if (type_ == kAfterTranslate) Translate(); } @@ -132,12 +132,12 @@ void TranslateInfoBarDelegate::SetTargetLanguage(int language_index) { target_language_index_ = language_index; if (infobar_view_) infobar_view_->TargetLanguageChanged(); - if (type_ == AFTER_TRANSLATE) + if (type_ == kAfterTranslate) Translate(); } bool TranslateInfoBarDelegate::IsError() { - return type_ == TRANSLATION_ERROR; + return type_ == kTranslationError; } void TranslateInfoBarDelegate::Translate() { @@ -175,7 +175,7 @@ void TranslateInfoBarDelegate::TranslationDeclined() { } void TranslateInfoBarDelegate::InfoBarDismissed() { - if (type_ != BEFORE_TRANSLATE) + if (type_ != kBeforeTranslate) return; // The user closed the infobar without clicking the translate button. @@ -259,11 +259,11 @@ void TranslateInfoBarDelegate::NeverTranslatePageLanguage() { string16 TranslateInfoBarDelegate::GetMessageInfoBarText() { switch (type_) { - case TRANSLATING: + case kTranslating: return l10n_util::GetStringFUTF16( IDS_TRANSLATE_INFOBAR_TRANSLATING_TO, GetLanguageDisplayableNameAt(target_language_index_)); - case TRANSLATION_ERROR: + case kTranslationError: switch (error_) { case TranslateErrors::NETWORK: return l10n_util::GetStringUTF16( @@ -295,9 +295,9 @@ string16 TranslateInfoBarDelegate::GetMessageInfoBarText() { string16 TranslateInfoBarDelegate::GetMessageInfoBarButtonText() { switch (type_) { - case TRANSLATING: + case kTranslating: return string16(); - case TRANSLATION_ERROR: + case kTranslationError: if (error_ == TranslateErrors::IDENTICAL_LANGUAGES || error_ == TranslateErrors::UNKNOWN_LANGUAGE) { // No retry button, we would fail again with the same error. @@ -313,7 +313,7 @@ string16 TranslateInfoBarDelegate::GetMessageInfoBarButtonText() { } void TranslateInfoBarDelegate::MessageInfoBarButtonPressed() { - DCHECK(type_ == TRANSLATION_ERROR); + DCHECK(type_ == kTranslationError); if (error_ == TranslateErrors::UNSUPPORTED_LANGUAGE) { RevertTranslation(); return; @@ -324,22 +324,22 @@ void TranslateInfoBarDelegate::MessageInfoBarButtonPressed() { } bool TranslateInfoBarDelegate::ShouldShowNeverTranslateButton() { - DCHECK(type_ == BEFORE_TRANSLATE); + DCHECK(type_ == kBeforeTranslate); return prefs_.GetTranslationDeniedCount(GetOriginalLanguageCode()) >= 3; } bool TranslateInfoBarDelegate::ShouldShowAlwaysTranslateButton() { - DCHECK(type_ == BEFORE_TRANSLATE); + DCHECK(type_ == kBeforeTranslate); return prefs_.GetTranslationAcceptedCount(GetOriginalLanguageCode()) >= 3; } void TranslateInfoBarDelegate::UpdateBackgroundAnimation( TranslateInfoBarDelegate* previous_infobar) { if (!previous_infobar || previous_infobar->IsError() == IsError()) { - background_animation_ = NONE; + background_animation_ = kNone; return; } - background_animation_ = IsError() ? NORMAL_TO_ERROR: ERROR_TO_NORMAL; + background_animation_ = IsError() ? kNormalToError: kErrorToNormal; } std::string TranslateInfoBarDelegate::GetPageHost() { diff --git a/chrome/browser/translate/translate_infobar_delegate.h b/chrome/browser/translate/translate_infobar_delegate.h index 4b1b1e9..c82cde7 100644 --- a/chrome/browser/translate/translate_infobar_delegate.h +++ b/chrome/browser/translate/translate_infobar_delegate.h @@ -19,17 +19,17 @@ class TranslateInfoBarDelegate : public InfoBarDelegate { public: // The different types of infobars that can be shown for translation. enum Type { - BEFORE_TRANSLATE, - TRANSLATING, - AFTER_TRANSLATE, - TRANSLATION_ERROR + kBeforeTranslate, + kTranslating, + kAfterTranslate, + kTranslationError }; // The types of background color animations. enum BackgroundAnimationType { - NONE, - NORMAL_TO_ERROR, - ERROR_TO_NORMAL + kNone, + kNormalToError, + kErrorToNormal }; // Factory method to create a non-error translate infobar. diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc index 48b3079..8a8e645 100644 --- a/chrome/browser/translate/translate_manager.cc +++ b/chrome/browser/translate/translate_manager.cc @@ -129,8 +129,6 @@ const char* const kTranslateScriptHeader = const char* const kReportLanguageDetectionErrorURL = "http://translate.google.com/translate_error"; -const int kTranslateScriptExpirationDelayMS = 24 * 60 * 60 * 1000; // 1 day. - } // namespace // static @@ -270,12 +268,6 @@ void TranslateManager::OnURLFetchComplete(const URLFetcher* source, DCHECK(translate_script_.empty()); str.CopyToString(&translate_script_); translate_script_ += "\n" + data; - // We'll expire the cached script after some time, to make sure long running - // browsers still get fixes that might get pushed with newer scripts. - MessageLoop::current()->PostDelayedTask(FROM_HERE, - method_factory_.NewRunnableMethod( - &TranslateManager::ClearTranslateScript), - translate_script_expiration_delay_); } // Process any pending requests. @@ -315,7 +307,6 @@ bool TranslateManager::IsShowingTranslateInfobar(TabContents* tab) { TranslateManager::TranslateManager() : ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)), - translate_script_expiration_delay_(kTranslateScriptExpirationDelayMS), translate_script_request_pending_(false) { notification_registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED, NotificationService::AllSources()); @@ -380,7 +371,7 @@ void TranslateManager::InitiateTranslation(TabContents* tab, // Prompts the user if he/she wants the page translated. tab->AddInfoBar(TranslateInfoBarDelegate::CreateDelegate( - TranslateInfoBarDelegate::BEFORE_TRANSLATE, tab, + TranslateInfoBarDelegate::kBeforeTranslate, tab, page_lang, target_lang)); } @@ -409,7 +400,7 @@ void TranslateManager::TranslatePage(TabContents* tab_contents, // showing (that is the case when the translation was triggered by the // "always translate" for example). infobar = TranslateInfoBarDelegate::CreateDelegate( - TranslateInfoBarDelegate::TRANSLATING, tab_contents, + TranslateInfoBarDelegate::kTranslating, tab_contents, source_lang, target_lang); ShowInfoBar(tab_contents, infobar); } @@ -498,7 +489,7 @@ void TranslateManager::PageTranslated(TabContents* tab, details->source_language, details->target_language); } else { infobar = TranslateInfoBarDelegate::CreateDelegate( - TranslateInfoBarDelegate::AFTER_TRANSLATE, tab, + TranslateInfoBarDelegate::kAfterTranslate, tab, details->source_language, details->target_language); } ShowInfoBar(tab, infobar); diff --git a/chrome/browser/translate/translate_manager.h b/chrome/browser/translate/translate_manager.h index 54debf4..260f345 100644 --- a/chrome/browser/translate/translate_manager.h +++ b/chrome/browser/translate/translate_manager.h @@ -51,6 +51,7 @@ class TranslateManager : public NotificationObserver, void ReportLanguageDetectionError(TabContents* tab_contents); // Clears the translate script, so it will be fetched next time we translate. + // Currently used by unit-tests. void ClearTranslateScript() { translate_script_.clear(); } // NotificationObserver implementation: @@ -66,12 +67,6 @@ class TranslateManager : public NotificationObserver, const ResponseCookies& cookies, const std::string& data); - // Used by unit-tests to override the default delay after which the translate - // script is fetched again from the translation server. - void set_translate_script_expiration_delay(int delay_ms) { - translate_script_expiration_delay_ = delay_ms; - } - // Convenience method to know if a tab is showing a translate infobar. static bool IsShowingTranslateInfobar(TabContents* tab); @@ -163,10 +158,6 @@ class TranslateManager : public NotificationObserver, // The JS injected in the page to do the translation. std::string translate_script_; - // Delay in milli-seconds after which the translate script is fetched again - // from the translate server. - int translate_script_expiration_delay_; - // Whether the translate JS is currently being retrieved. bool translate_script_request_pending_; diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc index 1b1598d..41d8c3d 100644 --- a/chrome/browser/translate/translate_manager_unittest.cc +++ b/chrome/browser/translate/translate_manager_unittest.cc @@ -113,11 +113,6 @@ class TranslateManagerTest : public RenderViewHostTestHarness, removed_infobars_.clear(); } - void ExpireTranslateScriptImmediately() { - Singleton<TranslateManager>::get()-> - set_translate_script_expiration_delay(0); - } - // If there is 1 infobar and it is a translate infobar, deny translation and // returns true. Returns false otherwise. bool DenyTranslation() { @@ -145,12 +140,8 @@ class TranslateManagerTest : public RenderViewHostTestHarness, // the TranslateManager is created before the TabContents. This matters as // they both register for similar events and we want the notifications to // happen in the same sequence (TranslateManager first, TabContents second). - // Also clears the translate script so it is fetched everytime and sets the - // expiration delay to a large value by default (in case it was zeroed in - // a previous test). + // Also clears the translate script so it is fetched everytime. Singleton<TranslateManager>::get()->ClearTranslateScript(); - Singleton<TranslateManager>::get()-> - set_translate_script_expiration_delay(60 * 60 * 1000); RenderViewHostTestHarness::SetUp(); @@ -281,7 +272,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { // We should have an infobar. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kBeforeTranslate, infobar->type()); // Simulate clicking translate. process()->sink().ClearMessages(); @@ -290,7 +281,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { // The "Translating..." infobar should be showing. infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATING, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kTranslating, infobar->type()); // Simulate the translate script being retrieved (it only needs to be done // once in the test as it is cached). @@ -311,7 +302,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { // The after translate infobar should be showing. infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::AFTER_TRANSLATE, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kAfterTranslate, infobar->type()); // Simulate changing the original language, this should trigger a translation. process()->sink().ClearMessages(); @@ -352,7 +343,7 @@ TEST_F(TranslateManagerTest, TranslateScriptNotAvailable) { // We should have an infobar. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kBeforeTranslate, infobar->type()); // Simulate clicking translate. process()->sink().ClearMessages(); @@ -366,7 +357,7 @@ TEST_F(TranslateManagerTest, TranslateScriptNotAvailable) { // And we should have an error infobar showing. infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATION_ERROR, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kTranslationError, infobar->type()); } // Ensures we deal correctly with pages for which the browser does not recognize @@ -384,7 +375,7 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) { TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); menu->ExecuteCommand(IDC_CONTENT_CONTEXT_TRANSLATE); - SimulateURLFetch(true); // Simulate receiving the translate script. + SimulateURLFetch(true); // Simulare receiving the translate script. // Simulate the render notifying the translation has been done, the server // having detected the page was in a known and supported language. @@ -394,7 +385,7 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) { // The after translate infobar should be showing. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::AFTER_TRANSLATE, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kAfterTranslate, infobar->type()); EXPECT_EQ("fr", infobar->GetOriginalLanguageCode()); EXPECT_EQ("en", infobar->GetTargetLanguageCode()); @@ -409,7 +400,7 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) { TranslateErrors::IDENTICAL_LANGUAGES)); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATION_ERROR, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kTranslationError, infobar->type()); EXPECT_EQ(TranslateErrors::IDENTICAL_LANGUAGES, infobar->error()); // Let's run the same steps again but this time the server fails to detect the @@ -423,7 +414,7 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) { TranslateErrors::UNKNOWN_LANGUAGE)); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATION_ERROR, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kTranslationError, infobar->type()); EXPECT_EQ(TranslateErrors::UNKNOWN_LANGUAGE, infobar->error()); } @@ -743,7 +734,7 @@ TEST_F(TranslateManagerTest, ServerReportsUnsupportedLanguage) { // language. infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::TRANSLATION_ERROR, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kTranslationError, infobar->type()); // This infobar should have a button (so the string should not be empty). ASSERT_FALSE(infobar->GetMessageInfoBarButtonText().empty()); @@ -1065,7 +1056,7 @@ TEST_F(TranslateManagerTest, BeforeTranslateExtraButtons) { true); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kBeforeTranslate, infobar->type()); if (i < 3) { EXPECT_FALSE(infobar->ShouldShowAlwaysTranslateButton()); infobar->Translate(); @@ -1093,7 +1084,7 @@ TEST_F(TranslateManagerTest, BeforeTranslateExtraButtons) { true); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type()); + EXPECT_EQ(TranslateInfoBarDelegate::kBeforeTranslate, infobar->type()); if (i < 3) { EXPECT_FALSE(infobar->ShouldShowNeverTranslateButton()); infobar->TranslationDeclined(); @@ -1128,41 +1119,3 @@ TEST_F(TranslateManagerTest, NonTranslatablePage) { EXPECT_FALSE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_TRANSLATE)); } -// Tests that the script is expired and refetched as expected. -TEST_F(TranslateManagerTest, ScriptExpires) { - ExpireTranslateScriptImmediately(); - - // Simulate navigating to a page and translating it. - SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); - TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); - ASSERT_TRUE(infobar != NULL); - process()->sink().ClearMessages(); - infobar->Translate(); - SimulateURLFetch(true); - rvh()->TestOnMessageReceived(ViewHostMsg_PageTranslated(0, 0, "fr", "en", - TranslateErrors::NONE)); - - // A task should have been posted to clear the script, run it. - MessageLoop::current()->RunAllPending(); - - // Do another navigation and translation. - SimulateNavigation(GURL("http://www.google.es"), 1, "El Google", "es", true); - infobar = GetTranslateInfoBar(); - ASSERT_TRUE(infobar != NULL); - process()->sink().ClearMessages(); - infobar->Translate(); - // If we don't simulate the URL fetch, the TranslateManager should be waiting - // for the script and no message should have been sent to the renderer. - EXPECT_TRUE( - process()->sink().GetFirstMessageMatching(ViewMsg_TranslatePage::ID) == - NULL); - // Now simulate the URL fetch. - SimulateURLFetch(true); - // Now the message should have been sent. - int page_id = 0; - std::string original_lang, target_lang; - EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); - EXPECT_EQ(1, page_id); - EXPECT_EQ("es", original_lang); - EXPECT_EQ("en", target_lang); -} |