diff options
author | inferno@chromium.org <inferno@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-09-29 18:09:47 +0000 |
---|---|---|
committer | inferno@chromium.org <inferno@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-09-29 18:09:47 +0000 |
commit | 34f284aac688f8d077eafb7767b255c2fcc58dba (patch) | |
tree | 11a6005a46fc0f5d6359798d69a8971f8f7b4e06 | |
parent | 1832f314f6c6dbeb78426be9edfd34de17f79a9e (diff) | |
download | chromium_src-34f284aac688f8d077eafb7767b255c2fcc58dba.zip chromium_src-34f284aac688f8d077eafb7767b255c2fcc58dba.tar.gz chromium_src-34f284aac688f8d077eafb7767b255c2fcc58dba.tar.bz2 |
Revert 60963 - Merge 58701 - Relanding this:
Don't create pending entries when a navigation is initiated by the page.
If the page reloads while such a navigation happens, we could end up with the wrong pending entry. Also make sure TestTabContents::NavigateAndCommit() does commit on the right RVH.
BUG=51680
TEST=See bug for steps.
TBR=creis
Review URL: http://codereview.chromium.org/3257002
Review URL: http://codereview.chromium.org/3346005
Review URL: http://codereview.chromium.org/3537005
TBR=inferno@chromium.org
Review URL: http://codereview.chromium.org/3585002
git-svn-id: svn://svn.chromium.org/chrome/branches/517/src@60965 0039d316-1c4b-4281-b951-d872f2087c98
10 files changed, 93 insertions, 230 deletions
diff --git a/chrome/browser/back_forward_menu_model_unittest.cc b/chrome/browser/back_forward_menu_model_unittest.cc index f6d2ea7..b4be325 100644 --- a/chrome/browser/back_forward_menu_model_unittest.cc +++ b/chrome/browser/back_forward_menu_model_unittest.cc @@ -40,23 +40,27 @@ class BackFwdMenuModelTest : public RenderViewHostTestHarness { // will be pending after we ask to navigate there). void NavigateToOffset(int offset) { controller().GoToOffset(offset); - contents()->CommitPendingNavigation(); + const NavigationEntry* entry = controller().pending_entry(); + rvh()->SendNavigate(entry->page_id(), entry->url()); } // Same as NavigateToOffset but goes to an absolute index. void NavigateToIndex(int index) { controller().GoToIndex(index); - contents()->CommitPendingNavigation(); + const NavigationEntry* entry = controller().pending_entry(); + rvh()->SendNavigate(entry->page_id(), entry->url()); } // Goes back/forward and commits the load. void GoBack() { controller().GoBack(); - contents()->CommitPendingNavigation(); + const NavigationEntry* entry = controller().pending_entry(); + rvh()->SendNavigate(entry->page_id(), entry->url()); } void GoForward() { controller().GoForward(); - contents()->CommitPendingNavigation(); + const NavigationEntry* entry = controller().pending_entry(); + rvh()->SendNavigate(entry->page_id(), entry->url()); } }; diff --git a/chrome/browser/renderer_host/test/test_render_view_host.cc b/chrome/browser/renderer_host/test/test_render_view_host.cc index 70d6a9c..2621c83 100644 --- a/chrome/browser/renderer_host/test/test_render_view_host.cc +++ b/chrome/browser/renderer_host/test/test_render_view_host.cc @@ -236,8 +236,6 @@ TestingProfile* RenderViewHostTestHarness::profile() { } MockRenderProcessHost* RenderViewHostTestHarness::process() { - if (pending_rvh()) - return static_cast<MockRenderProcessHost*>(pending_rvh()->process()); return static_cast<MockRenderProcessHost*>(rvh()->process()); } diff --git a/chrome/browser/sessions/tab_restore_service_browsertest.cc b/chrome/browser/sessions/tab_restore_service_browsertest.cc index 46f91e8..dcbd688 100644 --- a/chrome/browser/sessions/tab_restore_service_browsertest.cc +++ b/chrome/browser/sessions/tab_restore_service_browsertest.cc @@ -9,7 +9,6 @@ #include "chrome/browser/sessions/tab_restore_service.h" #include "chrome/browser/tab_contents/navigation_controller.h" #include "chrome/browser/tab_contents/navigation_entry.h" -#include "chrome/browser/tab_contents/test_tab_contents.h" #include "chrome/test/render_view_test.h" #include "chrome/test/testing_profile.h" #include "testing/gtest/include/gtest/gtest.h" @@ -69,7 +68,8 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness { // Navigate back. We have to do this song and dance as NavigationController // isn't happy if you navigate immediately while going back. controller().GoToIndex(index); - contents()->CommitPendingNavigation(); + rvh()->SendNavigate(controller().pending_entry()->page_id(), + controller().pending_entry()->url()); } void RecreateService() { @@ -157,9 +157,9 @@ TEST_F(TabRestoreServiceTest, Basic) { tab = static_cast<TabRestoreService::Tab*>(entry); EXPECT_FALSE(tab->pinned); ASSERT_EQ(3U, tab->navigations.size()); - EXPECT_EQ(url1_, tab->navigations[0].virtual_url()); - EXPECT_EQ(url2_, tab->navigations[1].virtual_url()); - EXPECT_EQ(url3_, tab->navigations[2].virtual_url()); + EXPECT_TRUE(url1_ == tab->navigations[0].virtual_url()); + EXPECT_TRUE(url2_ == tab->navigations[1].virtual_url()); + EXPECT_TRUE(url3_ == tab->navigations[2].virtual_url()); EXPECT_EQ(1, tab->current_navigation_index); EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(), tab->timestamp.ToInternalValue()); diff --git a/chrome/browser/tab_contents/navigation_controller_unittest.cc b/chrome/browser/tab_contents/navigation_controller_unittest.cc index da94be0..ba3bf71 100644 --- a/chrome/browser/tab_contents/navigation_controller_unittest.cc +++ b/chrome/browser/tab_contents/navigation_controller_unittest.cc @@ -1805,59 +1805,6 @@ TEST_F(NavigationControllerTest, CopyStateFromAndPrune3) { EXPECT_EQ(controller().session_id().id(), other_controller.session_id().id()); } -// Tests that navigations initiated from the page (with the history object) -// work as expected without navigation entries. -TEST_F(NavigationControllerTest, HistoryNavigate) { - const GURL url1("http://foo1"); - const GURL url2("http://foo2"); - const GURL url3("http://foo3"); - - NavigateAndCommit(url1); - NavigateAndCommit(url2); - NavigateAndCommit(url3); - controller().GoBack(); - contents()->CommitPendingNavigation(); - - // Casts the TabContents to a RenderViewHostDelegate::BrowserIntegration so we - // can call GoToEntryAtOffset which is private. - RenderViewHostDelegate::BrowserIntegration* rvh_delegate = - static_cast<RenderViewHostDelegate::BrowserIntegration*>(contents()); - - // Simulate the page calling history.back(), it should not create a pending - // entry. - rvh_delegate->GoToEntryAtOffset(-1); - EXPECT_EQ(-1, controller().pending_entry_index()); - // The actual cross-navigation is suspended until the current RVH tells us - // it unloaded, simulate that. - contents()->ProceedWithCrossSiteNavigation(); - // Also make sure we told the page to navigate. - const IPC::Message* message = - process()->sink().GetFirstMessageMatching(ViewMsg_Navigate::ID); - ASSERT_TRUE(message != NULL); - Tuple1<ViewMsg_Navigate_Params> nav_params; - ViewMsg_Navigate::Read(message, &nav_params); - EXPECT_EQ(url1, nav_params.a.url); - process()->sink().ClearMessages(); - - // Now test history.forward() - rvh_delegate->GoToEntryAtOffset(1); - EXPECT_EQ(-1, controller().pending_entry_index()); - // The actual cross-navigation is suspended until the current RVH tells us - // it unloaded, simulate that. - contents()->ProceedWithCrossSiteNavigation(); - message = process()->sink().GetFirstMessageMatching(ViewMsg_Navigate::ID); - ASSERT_TRUE(message != NULL); - ViewMsg_Navigate::Read(message, &nav_params); - EXPECT_EQ(url3, nav_params.a.url); - process()->sink().ClearMessages(); - - // Make sure an extravagant history.go() doesn't break. - rvh_delegate->GoToEntryAtOffset(120); // Out of bounds. - EXPECT_EQ(-1, controller().pending_entry_index()); - message = process()->sink().GetFirstMessageMatching(ViewMsg_Navigate::ID); - EXPECT_TRUE(message == NULL); -} - /* TODO(brettw) These test pass on my local machine but fail on the XP buildbot (but not Vista) cleaning up the directory after they run. This should be fixed. diff --git a/chrome/browser/tab_contents/render_view_host_manager_unittest.cc b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc index 74d73e6..72dfaca 100644 --- a/chrome/browser/tab_contents/render_view_host_manager_unittest.cc +++ b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc @@ -11,7 +11,6 @@ #include "chrome/browser/tab_contents/render_view_host_manager.h" #include "chrome/browser/tab_contents/test_tab_contents.h" #include "chrome/common/render_messages.h" -#include "chrome/common/render_messages_params.h" #include "chrome/common/url_constants.h" #include "chrome/test/test_notification_tracker.h" #include "chrome/test/testing_profile.h" @@ -296,49 +295,3 @@ TEST_F(RenderViewHostManagerTest, NonDOMUIChromeURLs) { EXPECT_TRUE(ShouldSwapProcesses(&manager, &ntp_entry, &about_entry)); } - -// Tests that we don't end up in an inconsistent state if a page does a back and -// then reload. http://crbug.com/51680 -TEST_F(RenderViewHostManagerTest, PageDoesBackAndReload) { - GURL url1("http://www.google.com/"); - GURL url2("http://www.evil-site.com/"); - - // Navigate to a safe site, then an evil site. - contents()->NavigateAndCommit(url1); - RenderViewHost* host1 = contents()->render_view_host(); - contents()->NavigateAndCommit(url2); - RenderViewHost* host2 = contents()->render_view_host(); - // We should have got a new RVH for the evil site. - EXPECT_NE(host1, host2); - - // Casts the TabContents to a RenderViewHostDelegate::BrowserIntegration so we - // can call GoToEntryAtOffset which is private. - RenderViewHostDelegate::BrowserIntegration* rvh_delegate = - static_cast<RenderViewHostDelegate::BrowserIntegration*>(contents()); - - // Now let's simulate the evil page calling history.back(). - rvh_delegate->GoToEntryAtOffset(-1); - // The pending RVH should be the one for the Google. - EXPECT_EQ(host1, contents()->render_manager()->pending_render_view_host()); - - // Before that RVH has committed, the evil page reloads itself. - ViewHostMsg_FrameNavigate_Params params; - params.page_id = 1; - params.url = url2; - params.transition = PageTransition::CLIENT_REDIRECT; - params.should_update_history = false; - params.gesture = NavigationGestureAuto; - params.was_within_same_page = false; - params.is_post = false; - contents()->TestDidNavigate(host2, params); - - // That should have cancelled the pending RVH, and the evil RVH should be the - // current one. - EXPECT_TRUE(contents()->render_manager()->pending_render_view_host() == NULL); - EXPECT_EQ(host2, contents()->render_manager()->current_host()); - - // Also we should not have a pending navigation entry. - NavigationEntry* entry = contents()->controller().GetActiveEntry(); - ASSERT_TRUE(entry != NULL); - EXPECT_EQ(url2, entry->url()); -} diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index ac97b73..1829cfc 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -227,12 +227,12 @@ ViewMsg_Navigate_Params::NavigationType GetNavigationType( return ViewMsg_Navigate_Params::NORMAL; } -void MakeNavigateParams(const NavigationEntry& entry, - const NavigationController& controller, +void MakeNavigateParams(const NavigationController& controller, NavigationController::ReloadType reload_type, ViewMsg_Navigate_Params* params) { + const NavigationEntry& entry = *controller.pending_entry(); params->page_id = entry.page_id(); - params->pending_history_list_offset = controller.GetIndexOfEntry(&entry); + params->pending_history_list_offset = controller.pending_entry_index(); params->current_history_list_offset = controller.last_committed_entry_index(); params->current_history_list_length = controller.entry_count(); params->url = entry.url(); @@ -849,12 +849,8 @@ void TabContents::OpenURL(const GURL& url, const GURL& referrer, bool TabContents::NavigateToPendingEntry( NavigationController::ReloadType reload_type) { - return NavigateToEntry(*controller_.pending_entry(), reload_type); -} + const NavigationEntry& entry = *controller_.pending_entry(); -bool TabContents::NavigateToEntry( - const NavigationEntry& entry, - NavigationController::ReloadType reload_type) { RenderViewHost* dest_render_view_host = render_manager_.Navigate(entry); if (!dest_render_view_host) return false; // Unable to create the desired render view host. @@ -886,7 +882,7 @@ bool TabContents::NavigateToEntry( // Navigate in the desired RenderViewHost. ViewMsg_Navigate_Params navigate_params; - MakeNavigateParams(entry, controller_, reload_type, &navigate_params); + MakeNavigateParams(controller_, reload_type, &navigate_params); dest_render_view_host->Navigate(navigate_params); if (entry.page_id() == -1) { @@ -1998,15 +1994,8 @@ void TabContents::OnFindReply(int request_id, } void TabContents::GoToEntryAtOffset(int offset) { - if (!delegate_ || delegate_->OnGoToEntryOffset(offset)) { - NavigationEntry* entry = controller_.GetEntryAtOffset(offset); - if (!entry) - return; - // Note that we don't call NavigationController::GotToOffset() as we don't - // want to create a pending navigation entry (it might end up lingering - // http://crbug.com/51680). - NavigateToEntry(*entry, NavigationController::NO_RELOAD); - } + if (!delegate_ || delegate_->OnGoToEntryOffset(offset)) + controller_.GoToOffset(offset); } void TabContents::OnMissingPluginStatus(int status) { diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 49867ee..11a46b3 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -823,12 +823,6 @@ class TabContents : public PageNavigator, // different and was therefore updated. bool UpdateTitleForEntry(NavigationEntry* entry, const std::wstring& title); - // Causes the TabContents to navigate in the right renderer to |entry|, which - // must be already part of the entries in the navigation controller. - // This does not change the NavigationController state. - bool NavigateToEntry(const NavigationEntry& entry, - NavigationController::ReloadType reload_type); - // Misc non-view stuff ------------------------------------------------------- // Helper functions for sending notifications. diff --git a/chrome/browser/tab_contents/test_tab_contents.cc b/chrome/browser/tab_contents/test_tab_contents.cc index ab6a325..1650851 100644 --- a/chrome/browser/tab_contents/test_tab_contents.cc +++ b/chrome/browser/tab_contents/test_tab_contents.cc @@ -45,7 +45,7 @@ void TestTabContents::Observe(NotificationType type, } } -TestRenderViewHost* TestTabContents::pending_rvh() const { +TestRenderViewHost* TestTabContents::pending_rvh() { return static_cast<TestRenderViewHost*>( render_manager_.pending_render_view_host_); } @@ -70,34 +70,7 @@ void TestTabContents::NavigateAndCommit(const GURL& url) { bool reverse_on_redirect = false; BrowserURLHandler::RewriteURLIfNecessary( &loaded_url, profile(), &reverse_on_redirect); - - // LoadURL created a navigation entry, now simulate the RenderView sending - // a notification that it actually navigated. - CommitPendingNavigation(); -} - -void TestTabContents::CommitPendingNavigation() { - // If we are doing a cross-site navigation, this simulates the current RVH - // notifying that it has unloaded so the pending RVH is resumed and can - // navigate. - ProceedWithCrossSiteNavigation(); - TestRenderViewHost* rvh = pending_rvh(); - if (!rvh) - rvh = static_cast<TestRenderViewHost*>(render_manager_.current_host()); - - const NavigationEntry* entry = controller().pending_entry(); - DCHECK(entry); - int page_id = entry->page_id(); - if (page_id == -1) { - // It's a new navigation, assign a never-seen page id to it. - page_id = - static_cast<MockRenderProcessHost*>(rvh->process())->max_page_id() + 1; - } - rvh->SendNavigate(page_id, entry->url()); -} - -void TestTabContents::ProceedWithCrossSiteNavigation() { - if (!pending_rvh()) - return; - render_manager_.ShouldClosePage(true, true); + static_cast<TestRenderViewHost*>(render_view_host())->SendNavigate( + static_cast<MockRenderProcessHost*>(render_view_host()->process())-> + max_page_id() + 1, loaded_url); } diff --git a/chrome/browser/tab_contents/test_tab_contents.h b/chrome/browser/tab_contents/test_tab_contents.h index 21bc096..f0750d7 100644 --- a/chrome/browser/tab_contents/test_tab_contents.h +++ b/chrome/browser/tab_contents/test_tab_contents.h @@ -20,7 +20,7 @@ class TestTabContents : public TabContents { // The render view host factory will be passed on to the TestTabContents(Profile* profile, SiteInstance* instance); - TestRenderViewHost* pending_rvh() const; + TestRenderViewHost* pending_rvh(); // State accessor. bool cross_navigation_pending() { @@ -62,15 +62,6 @@ class TestTabContents : public TabContents { // emulates what happens on a new navigation. void NavigateAndCommit(const GURL& url); - // Simulates the appropriate RenderView (pending if any, current otherwise) - // sending a navigate notification for the NavigationController pending entry. - void CommitPendingNavigation(); - - // Simulates the current RVH notifying that it has unloaded so that the - // pending RVH navigation can proceed. - // Does nothing if no cross-navigation is pending. - void ProceedWithCrossSiteNavigation(); - // Set by individual tests. bool transition_cross_site; diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc index 8f2b801..dc469a9 100644 --- a/chrome/browser/translate/translate_manager_unittest.cc +++ b/chrome/browser/translate/translate_manager_unittest.cc @@ -8,7 +8,6 @@ #include "chrome/app/chrome_dll_resource.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/renderer_host/mock_render_process_host.h" -#include "chrome/browser/tab_contents/navigation_controller.h" #include "chrome/browser/tab_contents/render_view_context_menu.h" #include "chrome/browser/tab_contents/test_tab_contents.h" #include "chrome/browser/translate/translate_infobar_delegate.h" @@ -42,13 +41,11 @@ class TranslateManagerTest : public RenderViewHostTestHarness, // Simluates navigating to a page and getting the page contents and language // for that navigation. - void SimulateNavigation(const GURL& url, + void SimulateNavigation(const GURL& url, int page_id, const std::string& contents, const std::string& lang, bool page_translatable) { NavigateAndCommit(url); - int page_id = RenderViewHostTestHarness::contents()->controller(). - GetLastCommittedEntry()->page_id(); SimulateOnPageContents(url, page_id, contents, lang, page_translatable); } @@ -282,7 +279,7 @@ class TestRenderViewContextMenu : public RenderViewContextMenu { TEST_F(TranslateManagerTest, NormalTranslate) { // Simulate navigating to a page. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // We should have an infobar. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); @@ -306,6 +303,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { int page_id = 0; std::string original_lang, target_lang; EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + EXPECT_EQ(0, page_id); EXPECT_EQ("fr", original_lang); EXPECT_EQ("en", target_lang); @@ -323,6 +321,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { std::string new_original_lang = infobar->GetLanguageCodeAt(0); infobar->SetOriginalLanguage(0); EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + EXPECT_EQ(0, page_id); EXPECT_EQ(new_original_lang, original_lang); EXPECT_EQ("en", target_lang); // Simulate the render notifying the translation has been done. @@ -338,6 +337,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { std::string new_target_lang = infobar->GetLanguageCodeAt(1); infobar->SetTargetLanguage(1); EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + EXPECT_EQ(0, page_id); EXPECT_EQ(new_original_lang, original_lang); EXPECT_EQ(new_target_lang, target_lang); // Simulate the render notifying the translation has been done. @@ -350,7 +350,7 @@ TEST_F(TranslateManagerTest, NormalTranslate) { TEST_F(TranslateManagerTest, TranslateScriptNotAvailable) { // Simulate navigating to a page. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // We should have an infobar. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); @@ -377,7 +377,7 @@ TEST_F(TranslateManagerTest, TranslateScriptNotAvailable) { TEST_F(TranslateManagerTest, TranslateUnknownLanguage) { // Simulate navigating to a page ("und" is the string returned by the CLD for // languages it does not recognize). - SimulateNavigation(GURL("http://www.google.mys"), "G00g1e", "und", true); + SimulateNavigation(GURL("http://www.google.mys"), 0, "G00g1e", "und", true); // We should not have an infobar as we don't know the language. ASSERT_TRUE(GetTranslateInfoBar() == NULL); @@ -411,7 +411,8 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) { // Let's run the same steps but this time the server detects the page is // already in English. - SimulateNavigation(GURL("http://www.google.com"), "The Google", "und", true); + SimulateNavigation(GURL("http://www.google.com"), 1, "The Google", "und", + true); menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); menu->ExecuteCommand(IDC_CONTENT_CONTEXT_TRANSLATE); @@ -424,7 +425,8 @@ TEST_F(TranslateManagerTest, TranslateUnknownLanguage) { // Let's run the same steps again but this time the server fails to detect the // page's language (it returns an empty string). - SimulateNavigation(GURL("http://www.google.com"), "The Google", "und", true); + SimulateNavigation(GURL("http://www.google.com"), 2, "The Google", "und", + true); menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); menu->ExecuteCommand(IDC_CONTENT_CONTEXT_TRANSLATE); @@ -509,7 +511,7 @@ TEST_F(TranslateManagerTest, TestAllLanguages) { // Tests auto-translate on page. TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Simulate the user translating. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); @@ -522,7 +524,7 @@ TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) { // Now navigate to a new page in the same language. process()->sink().ClearMessages(); - SimulateNavigation(GURL("http://news.google.fr"), "Les news", "fr", true); + SimulateNavigation(GURL("http://news.google.fr"), 1, "Les news", "fr", true); // This should have automatically triggered a translation. int page_id = 0; @@ -534,7 +536,7 @@ TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) { // Now navigate to a page in a different language. process()->sink().ClearMessages(); - SimulateNavigation(GURL("http://news.google.es"), "Las news", "es", true); + SimulateNavigation(GURL("http://news.google.es"), 1, "Las news", "es", true); // This should not have triggered a translate. EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); @@ -543,7 +545,7 @@ TEST_F(TranslateManagerTest, AutoTranslateOnNavigate) { // Tests that multiple OnPageContents do not cause multiple infobars. TEST_F(TranslateManagerTest, MultipleOnPageContents) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Simulate clicking 'Nope' (don't translate). EXPECT_TRUE(DenyTranslation()); @@ -555,7 +557,7 @@ TEST_F(TranslateManagerTest, MultipleOnPageContents) { EXPECT_EQ(0, contents()->infobar_delegate_count()); // Do the same steps but simulate closing the infobar this time. - SimulateNavigation(GURL("http://www.youtube.fr"), "Le YouTube", "fr", + SimulateNavigation(GURL("http://www.youtube.fr"), 1, "Le YouTube", "fr", true); EXPECT_TRUE(CloseTranslateInfoBar()); EXPECT_EQ(0, contents()->infobar_delegate_count()); @@ -567,7 +569,7 @@ TEST_F(TranslateManagerTest, MultipleOnPageContents) { // Test that reloading the page brings back the infobar. TEST_F(TranslateManagerTest, Reload) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Close the infobar. EXPECT_TRUE(CloseTranslateInfoBar()); @@ -594,7 +596,7 @@ TEST_F(TranslateManagerTest, ReloadFromLocationBar) { GURL url("http://www.google.fr"); // Simulate navigating to a page and getting its language. - SimulateNavigation(url, "Le Google", "fr", true); + SimulateNavigation(url, 0, "Le Google", "fr", true); // Close the infobar. EXPECT_TRUE(CloseTranslateInfoBar()); @@ -622,18 +624,18 @@ TEST_F(TranslateManagerTest, ReloadFromLocationBar) { // in-page. TEST_F(TranslateManagerTest, CloseInfoBarInPageNavigation) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Close the infobar. EXPECT_TRUE(CloseTranslateInfoBar()); // Navigate in page, no infobar should be shown. - SimulateNavigation(GURL("http://www.google.fr/#ref1"), "Le Google", "fr", + SimulateNavigation(GURL("http://www.google.fr/#ref1"), 0, "Le Google", "fr", true); EXPECT_TRUE(GetTranslateInfoBar() == NULL); // Navigate out of page, a new infobar should show. - SimulateNavigation(GURL("http://www.google.fr/foot"), "Le Google", "fr", + SimulateNavigation(GURL("http://www.google.fr/foot"), 0, "Le Google", "fr", true); EXPECT_TRUE(GetTranslateInfoBar() != NULL); } @@ -642,7 +644,7 @@ TEST_F(TranslateManagerTest, CloseInfoBarInPageNavigation) { // in a subframe. (http://crbug.com/48215) TEST_F(TranslateManagerTest, CloseInfoBarInSubframeNavigation) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Close the infobar. EXPECT_TRUE(CloseTranslateInfoBar()); @@ -658,7 +660,7 @@ TEST_F(TranslateManagerTest, CloseInfoBarInSubframeNavigation) { EXPECT_TRUE(GetTranslateInfoBar() == NULL); // Navigate out of page, a new infobar should show. - SimulateNavigation(GURL("http://www.google.fr/foot"), "Le Google", "fr", + SimulateNavigation(GURL("http://www.google.fr/foot"), 3, "Le Google", "fr", true); EXPECT_TRUE(GetTranslateInfoBar() != NULL); } @@ -668,18 +670,18 @@ TEST_F(TranslateManagerTest, CloseInfoBarInSubframeNavigation) { // Tests that denying translation is sticky when navigating in page. TEST_F(TranslateManagerTest, DenyTranslateInPageNavigation) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Simulate clicking 'Nope' (don't translate). EXPECT_TRUE(DenyTranslation()); // Navigate in page, no infobar should be shown. - SimulateNavigation(GURL("http://www.google.fr/#ref1"), "Le Google", "fr", + SimulateNavigation(GURL("http://www.google.fr/#ref1"), 0, "Le Google", "fr", true); EXPECT_TRUE(GetTranslateInfoBar() == NULL); // Navigate out of page, a new infobar should show. - SimulateNavigation(GURL("http://www.google.fr/foot"), "Le Google", "fr", + SimulateNavigation(GURL("http://www.google.fr/foot"), 0, "Le Google", "fr", true); EXPECT_TRUE(GetTranslateInfoBar() != NULL); } @@ -688,7 +690,7 @@ TEST_F(TranslateManagerTest, DenyTranslateInPageNavigation) { // return when navigating in page. TEST_F(TranslateManagerTest, TranslateCloseInfoBarInPageNavigation) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Simulate the user translating. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); @@ -702,7 +704,7 @@ TEST_F(TranslateManagerTest, TranslateCloseInfoBarInPageNavigation) { EXPECT_TRUE(CloseTranslateInfoBar()); // Navigate in page, no infobar should be shown. - SimulateNavigation(GURL("http://www.google.fr/#ref1"), "Le Google", "fr", + SimulateNavigation(GURL("http://www.google.fr/#ref1"), 0, "Le Google", "fr", true); EXPECT_TRUE(GetTranslateInfoBar() == NULL); @@ -710,7 +712,7 @@ TEST_F(TranslateManagerTest, TranslateCloseInfoBarInPageNavigation) { // Note that we navigate to a page in a different language so we don't trigger // the auto-translate feature (it would translate the page automatically and // the before translate inforbar would not be shown). - SimulateNavigation(GURL("http://www.google.de"), "Das Google", "de", true); + SimulateNavigation(GURL("http://www.google.de"), 0, "Das Google", "de", true); EXPECT_TRUE(GetTranslateInfoBar() != NULL); } @@ -718,7 +720,7 @@ TEST_F(TranslateManagerTest, TranslateCloseInfoBarInPageNavigation) { // in-page. TEST_F(TranslateManagerTest, TranslateInPageNavigation) { // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // Simulate the user translating. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); @@ -733,7 +735,7 @@ TEST_F(TranslateManagerTest, TranslateInPageNavigation) { // Navigate in page, the same infobar should still be shown. ClearRemovedInfoBars(); - SimulateNavigation(GURL("http://www.google.fr/#ref1"), "Le Google", "fr", + SimulateNavigation(GURL("http://www.google.fr/#ref1"), 0, "Le Google", "fr", true); EXPECT_FALSE(InfoBarRemoved()); EXPECT_EQ(infobar, GetTranslateInfoBar()); @@ -741,7 +743,7 @@ TEST_F(TranslateManagerTest, TranslateInPageNavigation) { // Navigate out of page, a new infobar should show. // See note in TranslateCloseInfoBarInPageNavigation test on why it is // important to navigate to a page in a different language for this test. - SimulateNavigation(GURL("http://www.google.de"), "Das Google", "de", true); + SimulateNavigation(GURL("http://www.google.de"), 0, "Das Google", "de", true); // The old infobar is gone. EXPECT_TRUE(CheckInfoBarRemovedAndReset(infobar)); // And there is a new one. @@ -752,7 +754,7 @@ TEST_F(TranslateManagerTest, TranslateInPageNavigation) { // unsupported language. TEST_F(TranslateManagerTest, CLDReportsUnsupportedPageLanguage) { // Simulate navigating to a page and getting an unsupported language. - SimulateNavigation(GURL("http://www.google.com"), "Google", "qbz", true); + SimulateNavigation(GURL("http://www.google.com"), 0, "Google", "qbz", true); // No info-bar should be shown. EXPECT_TRUE(GetTranslateInfoBar() == NULL); @@ -763,7 +765,8 @@ TEST_F(TranslateManagerTest, CLDReportsUnsupportedPageLanguage) { // The translation server might return a language we don't support. TEST_F(TranslateManagerTest, ServerReportsUnsupportedLanguage) { // Simulate navigating to a page and translating it. - SimulateNavigation(GURL("http://mail.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://mail.google.fr"), 0, "Le Google", "fr", + true); TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); process()->sink().ClearMessages(); @@ -803,7 +806,7 @@ TEST_F(TranslateManagerTest, UnsupportedUILanguage) { // Simulate navigating to a page in a language supported by the translate // server. - SimulateNavigation(GURL("http://www.google.com"), "Google", "en", true); + SimulateNavigation(GURL("http://www.google.com"), 0, "Google", "en", true); // No info-bar should be shown. EXPECT_TRUE(GetTranslateInfoBar() == NULL); @@ -818,7 +821,7 @@ TEST_F(TranslateManagerTest, TranslateEnabledPref) { prefs->SetBoolean(prefs::kEnableTranslate, true); // Simulate navigating to a page and getting its language. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // An infobar should be shown. TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); @@ -844,7 +847,7 @@ TEST_F(TranslateManagerTest, TranslateEnabledPref) { TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) { // Simulate navigating to a page and getting its language. GURL url("http://www.google.fr"); - SimulateNavigation(url, "Le Google", "fr", true); + SimulateNavigation(url, 0, "Le Google", "fr", true); // An infobar should be shown. EXPECT_TRUE(GetTranslateInfoBar() != NULL); @@ -865,7 +868,8 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) { EXPECT_TRUE(CloseTranslateInfoBar()); // Navigate to a new page also in French. - SimulateNavigation(GURL("http://wwww.youtube.fr"), "Le YouTube", "fr", true); + SimulateNavigation(GURL("http://wwww.youtube.fr"), 1, "Le YouTube", "fr", + true); // There should not be a translate infobar. EXPECT_TRUE(GetTranslateInfoBar() == NULL); @@ -877,7 +881,7 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) { EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url)); // Navigate to a page in French. - SimulateNavigation(url, "Le Google", "fr", true); + SimulateNavigation(url, 2, "Le Google", "fr", true); // There should be a translate infobar. EXPECT_TRUE(GetTranslateInfoBar() != NULL); @@ -890,7 +894,7 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) { // Simulate navigating to a page and getting its language. GURL url("http://www.google.fr"); std::string host(url.host()); - SimulateNavigation(url, "Le Google", "fr", true); + SimulateNavigation(url, 0, "Le Google", "fr", true); // An infobar should be shown. EXPECT_TRUE(GetTranslateInfoBar() != NULL); @@ -911,7 +915,8 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) { EXPECT_TRUE(CloseTranslateInfoBar()); // Navigate to a new page also on the same site. - SimulateNavigation(GURL("http://www.google.fr/hello"), "Bonjour", "fr", true); + SimulateNavigation(GURL("http://www.google.fr/hello"), 1, "Bonjour", "fr", + true); // There should not be a translate infobar. EXPECT_TRUE(GetTranslateInfoBar() == NULL); @@ -923,7 +928,7 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) { EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url)); // Navigate to a page in French. - SimulateNavigation(url, "Le Google", "fr", true); + SimulateNavigation(url, 0, "Le Google", "fr", true); // There should be a translate infobar. EXPECT_TRUE(GetTranslateInfoBar() != NULL); @@ -942,7 +947,7 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) { translate_prefs.WhitelistLanguagePair("fr", "en"); // Load a page in French. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); // It should have triggered an automatic translation to English. @@ -955,12 +960,13 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) { int page_id = 0; std::string original_lang, target_lang; EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + EXPECT_EQ(0, page_id); EXPECT_EQ("fr", original_lang); EXPECT_EQ("en", target_lang); process()->sink().ClearMessages(); // Try another language, it should not be autotranslated. - SimulateNavigation(GURL("http://www.google.es"), "El Google", "es", true); + SimulateNavigation(GURL("http://www.google.es"), 1, "El Google", "es", true); EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); EXPECT_TRUE(GetTranslateInfoBar() != NULL); EXPECT_TRUE(CloseTranslateInfoBar()); @@ -970,7 +976,8 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) { TestingProfile* test_profile = static_cast<TestingProfile*>(contents()->profile()); test_profile->set_off_the_record(true); - SimulateNavigation(GURL("http://www.youtube.fr"), "Le YouTube", "fr", true); + SimulateNavigation(GURL("http://www.youtube.fr"), 2, "Le YouTube", "fr", + true); EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); EXPECT_TRUE(GetTranslateInfoBar() != NULL); EXPECT_TRUE(CloseTranslateInfoBar()); @@ -980,7 +987,7 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) { // behavior, which is show a "before translate" infobar. SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateWhitelists); translate_prefs.RemoveLanguagePairFromWhitelist("fr", "en"); - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 3, "Le Google", "fr", true); EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); @@ -1027,6 +1034,7 @@ TEST_F(TranslateManagerTest, ContextMenu) { int page_id = 0; std::string original_lang, target_lang; EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + EXPECT_EQ(0, page_id); EXPECT_EQ("fr", original_lang); EXPECT_EQ("en", target_lang); process()->sink().ClearMessages(); @@ -1048,11 +1056,12 @@ TEST_F(TranslateManagerTest, ContextMenu) { // Test that selecting translate in the context menu WHILE the page is being // translated does nothing (this could happen if autotranslate kicks-in and // the user selects the menu while the translation is being performed). - SimulateNavigation(GURL("http://www.google.es"), "El Google", "es", true); + SimulateNavigation(GURL("http://www.google.es"), 1, "El Google", "es", true); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); infobar->Translate(); EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + EXPECT_EQ(1, page_id); process()->sink().ClearMessages(); menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); @@ -1063,11 +1072,12 @@ TEST_F(TranslateManagerTest, ContextMenu) { // Now test that selecting translate in the context menu AFTER the page has // been translated does nothing. - SimulateNavigation(GURL("http://www.google.de"), "Das Google", "de", true); + SimulateNavigation(GURL("http://www.google.de"), 2, "Das Google", "de", true); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); infobar->Translate(); EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + EXPECT_EQ(2, page_id); process()->sink().ClearMessages(); menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); @@ -1080,7 +1090,7 @@ TEST_F(TranslateManagerTest, ContextMenu) { // Test that the translate context menu is disabled when the page is in the // same language as the UI. - SimulateNavigation(url, "Google", "en", true); + SimulateNavigation(url, 0, "Google", "en", true); menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_TRANSLATE)); @@ -1088,7 +1098,7 @@ TEST_F(TranslateManagerTest, ContextMenu) { // Test that the translate context menu is enabled when the page is in an // unknown language. - SimulateNavigation(url, "G00g1e", "und", true); + SimulateNavigation(url, 0, "G00g1e", "und", true); menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_TRANSLATE)); @@ -1096,7 +1106,7 @@ TEST_F(TranslateManagerTest, ContextMenu) { // Test that the translate context menu is disabled when the page is in an // unsupported language. - SimulateNavigation(url, "G00g1e", "qbz", true); + SimulateNavigation(url, 0, "G00g1e", "qbz", true); menu.reset(TestRenderViewContextMenu::CreateContextMenu(contents())); menu->Init(); EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_TRANSLATE)); @@ -1122,7 +1132,8 @@ TEST_F(TranslateManagerTest, BeforeTranslateExtraButtons) { for (int i = 0; i < 8; ++i) { SCOPED_TRACE(::testing::Message::Message() << "Iteration " << i << " incognito mode=" << test_profile->IsOffTheRecord()); - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 1, "Le Google", "fr", + true); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type()); @@ -1154,7 +1165,8 @@ TEST_F(TranslateManagerTest, BeforeTranslateExtraButtons) { for (int i = 0; i < 8; ++i) { SCOPED_TRACE(::testing::Message::Message() << "Iteration " << i << " incognito mode=" << test_profile->IsOffTheRecord()); - SimulateNavigation(GURL("http://www.google.de"), "Das Google", "de", true); + SimulateNavigation(GURL("http://www.google.de"), 1, "Das Google", "de", + true); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE, infobar->type()); @@ -1180,7 +1192,8 @@ TEST_F(TranslateManagerTest, BeforeTranslateExtraButtons) { // should not be translated. TEST_F(TranslateManagerTest, NonTranslatablePage) { // Simulate navigating to a page. - SimulateNavigation(GURL("http://mail.google.fr"), "Le Google", "fr", false); + SimulateNavigation(GURL("http://mail.google.fr"), 0, "Le Google", "fr", + false); // We should not have an infobar. EXPECT_TRUE(GetTranslateInfoBar() == NULL); @@ -1198,7 +1211,7 @@ TEST_F(TranslateManagerTest, ScriptExpires) { ExpireTranslateScriptImmediately(); // Simulate navigating to a page and translating it. - SimulateNavigation(GURL("http://www.google.fr"), "Le Google", "fr", true); + SimulateNavigation(GURL("http://www.google.fr"), 0, "Le Google", "fr", true); TranslateInfoBarDelegate* infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); process()->sink().ClearMessages(); @@ -1211,7 +1224,7 @@ TEST_F(TranslateManagerTest, ScriptExpires) { MessageLoop::current()->RunAllPending(); // Do another navigation and translation. - SimulateNavigation(GURL("http://www.google.es"), "El Google", "es", true); + SimulateNavigation(GURL("http://www.google.es"), 1, "El Google", "es", true); infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); process()->sink().ClearMessages(); @@ -1227,6 +1240,7 @@ TEST_F(TranslateManagerTest, ScriptExpires) { 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); } |