Chromium Code Reviews Chromium Code Reviews
Issue 2381503003:
PlzNav: Fix NavigationControllerBrowserTest.EnsureSamePageNavigationUpdatesFrameNaviga… (Closed)
| Index: content/browser/frame_host/navigation_controller_impl_browsertest.cc |
| diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc |
| index 4ccab789b2bf2054a016a74d9664d1b01b4dfeaa..cd3ae8ae1c49b21ff1bf8d4d64e54398260a224b 100644 |
| --- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc |
| +++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc |
| @@ -6157,6 +6157,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, |
| observer.Wait(); |
| } |
| + EXPECT_EQ(2, web_contents->GetController().GetEntryCount()); |
| + |
| // Prior to fixing the issue, the above omnibox navigation (which is |
| // classified as SAME_PAGE) was leaving the FrameNavigationEntry with the |
| // same document sequence number as the previous entry but updates the URL. |
| @@ -6177,6 +6179,72 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, |
| EXPECT_EQ(start_url.GetOrigin().spec(), origin + "/"); |
| } |
| +IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, |
| + SameURLNavigationWithDifferentVirtualURL) { |
|
Charlie Reis
2016/10/15 00:00:49
nit: Can we indicate this test is about LoadDataWi
|
| + WebContentsImpl* web_contents = |
| + static_cast<WebContentsImpl*>(shell()->web_contents()); |
| + const NavigationControllerImpl& controller = |
| + static_cast<const NavigationControllerImpl&>( |
| + web_contents->GetController()); |
| + |
| + const GURL base_url("http://baseurl"); |
| + const GURL virtual_url1("http://virtual1"); |
| + const GURL virtual_url2("http://virtual2"); |
| + const std::string data = "<html><body>foo</body></html>"; |
| + const GURL data_url = GURL("data:text/html;charset=utf-8," + data); |
|
nasko
2016/10/13 00:06:16
nit: Wouldn't it work the same if you don't use th
|
| + |
| + // Navigate to a data URL with a virtual URL. |
| + { |
| + TestNavigationObserver observer(web_contents); |
| + shell()->LoadDataWithBaseURL(virtual_url1, data, base_url); |
| + observer.Wait(); |
| + |
| + NavigationEntryImpl* entry = controller.GetLastCommittedEntry(); |
| + EXPECT_EQ(base_url, entry->GetBaseURLForDataURL()); |
| + EXPECT_EQ(virtual_url1, entry->GetVirtualURL()); |
| + EXPECT_EQ(virtual_url1, entry->GetHistoryURLForDataURL()); |
| + EXPECT_EQ(data_url, entry->GetURL()); |
| + } |
| + |
| + // Navigate to the same URL with different virtual URL. |
| + { |
| + TestNavigationObserver observer(web_contents); |
| + shell()->LoadDataWithBaseURL(virtual_url2, data, base_url); |
| + observer.Wait(); |
| + |
| + NavigationEntryImpl* entry = controller.GetLastCommittedEntry(); |
| + EXPECT_EQ(base_url, entry->GetBaseURLForDataURL()); |
| + EXPECT_EQ(virtual_url2, entry->GetVirtualURL()); |
| + EXPECT_EQ(virtual_url2, entry->GetHistoryURLForDataURL()); |
| + EXPECT_EQ(data_url, entry->GetURL()); |
| + } |
| + |
| + // Navigate to the same URL with *same* virtual URL. |
|
Charlie Reis
2016/10/15 00:00:49
We should be checking whether these are creating n
|
| + { |
| + TestNavigationObserver observer(web_contents); |
| + shell()->LoadDataWithBaseURL(virtual_url2, data, base_url); |
| + observer.Wait(); |
| + |
| + NavigationEntryImpl* entry = controller.GetLastCommittedEntry(); |
| + EXPECT_EQ(base_url, entry->GetBaseURLForDataURL()); |
| + EXPECT_EQ(virtual_url2, entry->GetVirtualURL()); |
| + EXPECT_EQ(virtual_url2, entry->GetHistoryURLForDataURL()); |
| + EXPECT_EQ(data_url, entry->GetURL()); |
| + } |
| + |
| + // Simulate the user hitting Enter in the omnibox without changing the URL. |
| + { |
| + TestNavigationObserver observer(web_contents); |
| + web_contents->GetController().LoadURL(web_contents->GetLastCommittedURL(), |
| + Referrer(), ui::PAGE_TRANSITION_LINK, |
| + std::string()); |
| + observer.Wait(); |
| + } |
| + |
| + // All of these cause history entries. |
|
nasko
2016/10/13 00:06:16
Hmm, the whole idea of the hitting Enter with the
Charlie Reis
2016/10/15 00:00:49
Agreed, seems this like would be 2 rather than 4 i
boliu
2016/10/17 17:42:43
Hmm, I don't think spec-ed out much, and webview a
|
| + EXPECT_EQ(4, web_contents->GetController().GetEntryCount()); |
| +} |
| + |
| // A BrowserMessageFilter that delays FrameHostMsg_DidCommitProvisionalLoad IPC |
| // message for a specified URL, navigates the WebContents back and then |
| // processes the commit message. |
