diff options
| author | avi <avi@chromium.org> | 2015-07-06 21:42:36 -0700 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2015-07-07 04:43:15 +0000 |
| commit | 259dc79058886a5b109ff65431bc1a19e854469e (patch) | |
| tree | 0c987d5176db58533b88537e12cad62ec548dfdd /content/browser | |
| parent | f38b0349ac26baf2d936183df96c3c1a4e6d60f1 (diff) | |
| download | chromium_src-259dc79058886a5b109ff65431bc1a19e854469e.zip chromium_src-259dc79058886a5b109ff65431bc1a19e854469e.tar.gz chromium_src-259dc79058886a5b109ff65431bc1a19e854469e.tar.bz2 | |
Remove NAVIGATION_TYPE_IN_PAGE.
BUG=251330
TEST=all tests should stay green
Review URL: https://codereview.chromium.org/1214073005
Cr-Commit-Position: refs/heads/master@{#337562}
Diffstat (limited to 'content/browser')
6 files changed, 60 insertions, 122 deletions
diff --git a/content/browser/android/web_contents_observer_proxy.cc b/content/browser/android/web_contents_observer_proxy.cc index c4cb1b7..9d82f17 100644 --- a/content/browser/android/web_contents_observer_proxy.cc +++ b/content/browser/android/web_contents_observer_proxy.cc @@ -133,8 +133,7 @@ void WebContentsObserverProxy::DidNavigateMainFrame( // is actually a fragment navigation, or a history API navigation to a URL // that would also be valid for a fragment navigation. bool is_fragment_navigation = - urls_same_ignoring_fragment && - (details.type == NAVIGATION_TYPE_IN_PAGE || details.is_in_page); + urls_same_ignoring_fragment && details.is_in_page; Java_WebContentsObserverProxy_didNavigateMainFrame( env, obj.obj(), jstring_url.obj(), jstring_base_url.obj(), details.is_navigation_to_different_page(), is_fragment_navigation, diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc index 321aaea..2229ff0 100644 --- a/content/browser/frame_host/navigation_controller_impl.cc +++ b/content/browser/frame_host/navigation_controller_impl.cc @@ -860,14 +860,12 @@ bool NavigationControllerImpl::RendererDidNavigate( RendererDidNavigateToNewPage(rfh, params, details->did_replace_entry); break; case NAVIGATION_TYPE_EXISTING_PAGE: + details->did_replace_entry = details->is_in_page; RendererDidNavigateToExistingPage(rfh, params); break; case NAVIGATION_TYPE_SAME_PAGE: RendererDidNavigateToSamePage(rfh, params); break; - case NAVIGATION_TYPE_IN_PAGE: - RendererDidNavigateInPage(rfh, params, &details->did_replace_entry); - break; case NAVIGATION_TYPE_NEW_SUBFRAME: RendererDidNavigateNewSubframe(rfh, params); break; @@ -1000,14 +998,9 @@ NavigationType NavigationControllerImpl::ClassifyNavigation( if (!last_committed) return NAVIGATION_TYPE_NAV_IGNORE; - if (IsURLInPageNavigation(params.url, params.was_within_same_page, rfh)) { - // This is history.replaceState(), which is renderer-initiated yet within - // the same page. - return NAVIGATION_TYPE_IN_PAGE; - } else { - // This is history.reload() or a client-side redirect. - return NAVIGATION_TYPE_EXISTING_PAGE; - } + // This is history.replaceState(), history.reload(), or a client-side + // redirect. + return NAVIGATION_TYPE_EXISTING_PAGE; } if (pending_entry_ && pending_entry_index_ == -1 && @@ -1046,14 +1039,6 @@ NavigationType NavigationControllerImpl::ClassifyNavigation( return NAVIGATION_TYPE_NEW_PAGE; } - // Any top-level navigations with the same base (minus the reference fragment) - // are in-page navigations. (We weeded out subframe navigations above.) Most - // of the time this doesn't matter since Blink doesn't tell us about subframe - // navigations that don't actually navigate, but it can happen when there is - // an encoding override (it always sends a navigation request). - if (IsURLInPageNavigation(params.url, params.was_within_same_page, rfh)) - return NAVIGATION_TYPE_IN_PAGE; - // Since we weeded out "new" navigations above, we know this is an existing // (back/forward) navigation. return NAVIGATION_TYPE_EXISTING_PAGE; @@ -1231,49 +1216,6 @@ void NavigationControllerImpl::RendererDidNavigateToSamePage( DiscardNonCommittedEntries(); } -void NavigationControllerImpl::RendererDidNavigateInPage( - RenderFrameHostImpl* rfh, - const FrameHostMsg_DidCommitProvisionalLoad_Params& params, - bool* did_replace_entry) { - DCHECK(!rfh->GetParent()) << - "Blink should only tell us about in-page navs for the main frame."; - - NavigationEntryImpl* existing_entry; - if (params.nav_entry_id) { - // This is a browser-initiated history navigation across an existing - // fragment navigation or pushState-created entry. - existing_entry = GetEntryWithUniqueID(params.nav_entry_id); - } else { - // This is renderer-initiated. The only kinds of renderer-initated - // navigations that are IN_PAGE are history.replaceState, which lands us at - // the last committed entry. - existing_entry = GetLastCommittedEntry(); - } - DCHECK(existing_entry); - - // Reference fragment navigation. We're guaranteed to have the last_committed - // entry and it will be the same page as the new navigation (minus the - // reference fragments, of course). We'll update the URL of the existing - // entry without pruning the forward history. - existing_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR - : PAGE_TYPE_NORMAL); - existing_entry->SetURL(params.url); - if (existing_entry->update_virtual_url_with_url()) - UpdateVirtualURLToURL(existing_entry, params.url); - - existing_entry->SetHasPostData(params.is_post); - existing_entry->SetPostID(params.post_id); - - // This replaces the existing entry since the page ID didn't change. - *did_replace_entry = true; - - DiscardNonCommittedEntriesInternal(); - - // If a transient entry was removed, the indices might have changed, so we - // have to query the entry index again. - last_committed_entry_index_ = GetIndexOfEntry(existing_entry); -} - void NavigationControllerImpl::RendererDidNavigateNewSubframe( RenderFrameHostImpl* rfh, const FrameHostMsg_DidCommitProvisionalLoad_Params& params) { diff --git a/content/browser/frame_host/navigation_controller_impl.h b/content/browser/frame_host/navigation_controller_impl.h index ba0bb55..ee14045 100644 --- a/content/browser/frame_host/navigation_controller_impl.h +++ b/content/browser/frame_host/navigation_controller_impl.h @@ -286,10 +286,6 @@ class CONTENT_EXPORT NavigationControllerImpl void RendererDidNavigateToSamePage( RenderFrameHostImpl* rfh, const FrameHostMsg_DidCommitProvisionalLoad_Params& params); - void RendererDidNavigateInPage( - RenderFrameHostImpl* rfh, - const FrameHostMsg_DidCommitProvisionalLoad_Params& params, - bool* did_replace_entry); void RendererDidNavigateNewSubframe( RenderFrameHostImpl* rfh, const FrameHostMsg_DidCommitProvisionalLoad_Params& params); diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc index d676d45..ba4a49d 100644 --- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc +++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc @@ -552,6 +552,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, // transition? Lots of these transitions should be cleaned up. EXPECT_EQ(ui::PAGE_TRANSITION_LINK, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -562,6 +563,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, capturer.Wait(); EXPECT_EQ(ui::PAGE_TRANSITION_LINK, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type); + EXPECT_TRUE(capturer.details().is_in_page); } { @@ -572,6 +574,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, capturer.Wait(); EXPECT_EQ(ui::PAGE_TRANSITION_LINK, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -585,6 +588,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -597,6 +601,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type); + EXPECT_TRUE(capturer.details().is_in_page); } } @@ -625,6 +630,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -637,6 +643,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -650,6 +657,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -663,6 +671,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -676,6 +685,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -689,6 +699,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -698,6 +709,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, capturer.Wait(); EXPECT_EQ(ui::PAGE_TRANSITION_RELOAD, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -709,6 +721,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } { @@ -722,62 +735,29 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT, capturer.params().transition); EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_FALSE(capturer.details().is_in_page); } -} -// Verify that navigations for NAVIGATION_TYPE_SAME_PAGE are correctly -// classified. -IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, - NavigationTypeClassification_SamePage) { - GURL url1(embedded_test_server()->GetURL( - "/navigation_controller/simple_page_1.html")); - NavigateToURL(shell(), url1); - - FrameTreeNode* root = - static_cast<WebContentsImpl*>(shell()->web_contents())-> - GetFrameTree()->root(); - - { - // Simple load. - FrameNavigateParamsCapturer capturer(root); - GURL frame_url(embedded_test_server()->GetURL( - "/navigation_controller/simple_page_1.html")); - NavigateFrameToURL(root, frame_url); - capturer.Wait(); - EXPECT_EQ(ui::PAGE_TRANSITION_LINK, capturer.params().transition); - EXPECT_EQ(NAVIGATION_TYPE_SAME_PAGE, capturer.details().type); - } -} - -// Verify that navigations for NAVIGATION_TYPE_IN_PAGE are correctly -// classified. -IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, - NavigationTypeClassification_InPage) { - GURL url1(embedded_test_server()->GetURL( - "/navigation_controller/simple_page_1.html")); - NavigateToURL(shell(), url1); - - FrameTreeNode* root = - static_cast<WebContentsImpl*>(shell()->web_contents())-> - GetFrameTree()->root(); + // Now, various in-page navigations. { // history.replaceState(). FrameNavigateParamsCapturer capturer(root); std::string script = - "history.replaceState({}, 'page 1', 'simple_page_2.html')"; + "history.replaceState({}, 'page 2', 'simple_page_2.html')"; EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); capturer.Wait(); - EXPECT_EQ(ui::PAGE_TRANSITION_LINK, capturer.params().transition); - EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, capturer.details().type); + EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT, + capturer.params().transition); + EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); EXPECT_TRUE(capturer.details().is_in_page); } // Back and forward across a fragment navigation. - GURL url2(embedded_test_server()->GetURL( + GURL url_links(embedded_test_server()->GetURL( "/navigation_controller/page_with_links.html")); - NavigateToURL(shell(), url2); + NavigateToURL(shell(), url_links); std::string script = "document.getElementById('fraglink').click()"; EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); EXPECT_TRUE(WaitForLoadStop(shell()->web_contents())); @@ -791,7 +771,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FORWARD_BACK | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); - EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, capturer.details().type); + EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); EXPECT_TRUE(capturer.details().is_in_page); } @@ -802,7 +782,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, capturer.Wait(); EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_FORWARD_BACK, capturer.params().transition); - EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, capturer.details().type); + EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); EXPECT_TRUE(capturer.details().is_in_page); } @@ -822,7 +802,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, | ui::PAGE_TRANSITION_FORWARD_BACK | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR, capturer.params().transition); - EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, capturer.details().type); + EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); EXPECT_TRUE(capturer.details().is_in_page); } @@ -833,11 +813,35 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, capturer.Wait(); EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_FORWARD_BACK, capturer.params().transition); - EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, capturer.details().type); + EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); EXPECT_TRUE(capturer.details().is_in_page); } } +// Verify that navigations for NAVIGATION_TYPE_SAME_PAGE are correctly +// classified. +IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, + NavigationTypeClassification_SamePage) { + GURL url1(embedded_test_server()->GetURL( + "/navigation_controller/simple_page_1.html")); + NavigateToURL(shell(), url1); + + FrameTreeNode* root = + static_cast<WebContentsImpl*>(shell()->web_contents())-> + GetFrameTree()->root(); + + { + // Simple load. + FrameNavigateParamsCapturer capturer(root); + GURL frame_url(embedded_test_server()->GetURL( + "/navigation_controller/simple_page_1.html")); + NavigateFrameToURL(root, frame_url); + capturer.Wait(); + EXPECT_EQ(ui::PAGE_TRANSITION_LINK, capturer.params().transition); + EXPECT_EQ(NAVIGATION_TYPE_SAME_PAGE, capturer.details().type); + } +} + // Verify that navigations for NAVIGATION_TYPE_NEW_SUBFRAME and // NAVIGATION_TYPE_AUTO_SUBFRAME are properly classified. IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, @@ -1836,7 +1840,8 @@ void DoReplaceStateWhilePending(Shell* shell, // The fact that there was a pending entry shouldn't interfere with the // classification. - EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, capturer.details().type); + EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type); + EXPECT_TRUE(capturer.details().is_in_page); } ResourceDispatcherHost::Get()->SetDelegate(nullptr); diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc index 1520c03..2d7fc1c 100644 --- a/content/browser/frame_host/navigation_controller_impl_unittest.cc +++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc @@ -4943,7 +4943,7 @@ TEST_F(NavigationControllerTest, UnreachableURLGivesErrorPage) { controller_impl().RendererDidNavigate(main_test_rfh(), params, &details); EXPECT_EQ(PAGE_TYPE_ERROR, controller_impl().GetLastCommittedEntry()->GetPageType()); - EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, details.type); + EXPECT_TRUE(details.is_in_page); } } diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc index 6546c60..5381a68 100644 --- a/content/browser/presentation/presentation_service_impl.cc +++ b/content/browser/presentation/presentation_service_impl.cc @@ -490,14 +490,10 @@ void PresentationServiceImpl::DidNavigateAnyFrame( // If a frame navigation is in-page (e.g. navigating to a fragment in // same page) then we do not unregister listeners. - bool in_page_navigation = details.is_in_page || - details.type == content::NAVIGATION_TYPE_IN_PAGE; - DVLOG(2) << "DidNavigateAnyFrame: " - << "prev host: " << prev_url_host << ", curr host: " << curr_url_host - << ", in_page_navigation: " << in_page_navigation; - - if (in_page_navigation) + << "prev host: " << prev_url_host << ", curr host: " << curr_url_host + << ", details.is_in_page: " << details.is_in_page; + if (details.is_in_page) return; // Reset if the frame actually navigated. |