Remove page id from FrameNavigateParams.

content/browser/web_contents/web_contents_impl_unittest.cc

Index: content/browser/web_contents/web_contents_impl_unittest.cc
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index e2ef0e578ae3c1c8ea288b508e7229fdf1343233..df5f3f5ac7d9c44ed5f766d1ce9d6671e71237cf 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -154,9 +154,9 @@ class TestInterstitialPage : public InterstitialPageImpl {
       CommandReceived();
   }
 
-  void TestDidNavigate(int page_id, const GURL& url) {
+  void TestDidNavigate(const GURL& url) {
     FrameHostMsg_DidCommitProvisionalLoad_Params params;
-    InitNavigateParams(&params, page_id, url, PAGE_TRANSITION_TYPED);
+    InitNavigateParams(&params, url, PAGE_TRANSITION_TYPED);
     DidNavigate(GetRenderViewHostForTesting(), params);
   }
 
@@ -342,11 +342,10 @@ TEST_F(WebContentsImplTest, UpdateTitle) {
   NavigationControllerImpl& cont =
       static_cast<NavigationControllerImpl&>(controller());
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
-  InitNavigateParams(
-      &params, 0, GURL(url::kAboutBlankURL), PAGE_TRANSITION_TYPED);
+  InitNavigateParams(&params, GURL(url::kAboutBlankURL), PAGE_TRANSITION_TYPED);
 
   LoadCommittedDetails details;
-  cont.RendererDidNavigate(contents()->GetMainFrame(), params, &details);
+  cont.RendererDidNavigate(contents()->GetMainFrame(), 0, params, &details);
 
   contents()->UpdateTitle(contents()->GetMainFrame(), 0,
                           base::ASCIIToUTF16("    Lots O' Whitespace\n"),
@@ -391,9 +390,9 @@ TEST_F(WebContentsImplTest, NTPViewSource) {
       ViewMsg_EnableViewSourceMode::ID));
 
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
-  InitNavigateParams(&params, 0, kGURL, PAGE_TRANSITION_TYPED);
+  InitNavigateParams(&params, kGURL, PAGE_TRANSITION_TYPED);
   LoadCommittedDetails details;
-  cont.RendererDidNavigate(contents()->GetMainFrame(), params, &details);
+  cont.RendererDidNavigate(contents()->GetMainFrame(), 0, params, &details);
   // Also check title and url.
   EXPECT_EQ(base::ASCIIToUTF16(kUrl), contents()->GetTitle());
 }
@@ -1462,7 +1461,7 @@ TEST_F(WebContentsImplTest,
   EXPECT_FALSE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == NULL);
   // Let's commit the interstitial navigation.
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);

[Charlie Reis, 2014/09/11 22:55:39]

All of these tests used to pass a page ID to TestDidNavigate, and now it's gone
from the test entirely (unlike the other cases where it now gets passed in
elsewhere).  Is the page ID entirely unnecessary in this case?

[Avi (use Gerrit), 2014/09/11 23:10:10]

In a previous patchset, I commented TestDidNavigate with:

// *** page_id?

I was shocked, but the page id doesn't seem to matter. The interstitial page, if
you look at those files in this patch, ignored the page id parameter that was
passed into them, so I guess it makes sense that we can drop all of these.

The bots are green. Surely that means that this patch is correct? :)

[Charlie Reis, 2014/09/11 23:15:28]

Ah, that makes sense.  Interstitials don't maintain a session history of their
own; they just want to know if their one page has committed or not.

   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_TRUE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == interstitial);
@@ -1509,7 +1508,7 @@ TEST_F(WebContentsImplTest,
   EXPECT_FALSE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == NULL);
   // Let's commit the interstitial navigation.
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_TRUE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == interstitial);
@@ -1554,7 +1553,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialNoNewNavigationDontProceed) {
   EXPECT_FALSE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == NULL);
   // Let's commit the interstitial navigation.
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_TRUE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == interstitial);
@@ -1605,7 +1604,7 @@ TEST_F(WebContentsImplTest,
   EXPECT_FALSE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == NULL);
   // Let's commit the interstitial navigation.
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_TRUE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == interstitial);
@@ -1663,7 +1662,7 @@ TEST_F(WebContentsImplTest,
   EXPECT_FALSE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == NULL);
   // Let's commit the interstitial navigation.
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_TRUE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == interstitial);
@@ -1720,7 +1719,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialNoNewNavigationProceed) {
   EXPECT_FALSE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == NULL);
   // Let's commit the interstitial navigation.
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_TRUE(contents()->ShowingInterstitialPage());
   EXPECT_TRUE(contents()->GetInterstitialPage() == interstitial);
@@ -1757,7 +1756,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialThenNavigate) {
       new TestInterstitialPage(contents(), true, url, &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, url);
+  interstitial->TestDidNavigate(url);
 
   // While interstitial showing, navigate to a new URL.
   const GURL url2("http://www.yahoo.com");
@@ -1786,7 +1785,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialThenGoBack) {
                                &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(2, interstitial_url);
+  interstitial->TestDidNavigate(interstitial_url);
 
   // While the interstitial is showing, go back.
   controller().GoBack();
@@ -1821,7 +1820,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialCrashRendererThenGoBack) {
                                &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(2, interstitial_url);
+  interstitial->TestDidNavigate(interstitial_url);
 
   // Crash the renderer
   test_rvh()->OnMessageReceived(
@@ -1867,7 +1866,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialCrashRendererThenNavigate) {
       ViewHostMsg_RenderProcessGone(
           0, base::TERMINATION_STATUS_PROCESS_CRASHED, -1));
 
-  interstitial->TestDidNavigate(2, interstitial_url);
+  interstitial->TestDidNavigate(interstitial_url);
 }
 
 // Test navigating to a page that shows an interstitial, then close the
@@ -1882,7 +1881,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialThenCloseTab) {
       new TestInterstitialPage(contents(), true, url, &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, url);
+  interstitial->TestDidNavigate(url);
 
   // Now close the contents.
   DeleteContents();
@@ -1904,7 +1903,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialThenCloseAndShutdown) {
       new TestInterstitialPage(contents(), true, url, &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, url);
+  interstitial->TestDidNavigate(url);
   RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
       interstitial->GetRenderViewHostForTesting());
 
@@ -1939,7 +1938,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialProceedMultipleCommands) {
       new TestInterstitialPage(contents(), true, url2, &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);
 
   // Run a command.
   EXPECT_EQ(0, interstitial->command_received_count());
@@ -1974,7 +1973,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialOnInterstitial) {
       new TestInterstitialPage(contents(), true, url1, &state1, &deleted1);
   TestInterstitialPageStateGuard state_guard1(interstitial1);
   interstitial1->Show();
-  interstitial1->TestDidNavigate(1, url1);
+  interstitial1->TestDidNavigate(url1);
 
   // Now show another interstitial.
   TestInterstitialPage::InterstitialState state2 =
@@ -1985,7 +1984,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialOnInterstitial) {
       new TestInterstitialPage(contents(), true, url2, &state2, &deleted2);
   TestInterstitialPageStateGuard state_guard2(interstitial2);
   interstitial2->Show();
-  interstitial2->TestDidNavigate(1, url2);
+  interstitial2->TestDidNavigate(url2);
 
   // Showing interstitial2 should have caused interstitial1 to go away.
   EXPECT_EQ(TestInterstitialPage::CANCELED, state1);
@@ -2027,7 +2026,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialProceedShowInterstitial) {
       new TestInterstitialPage(contents(), true, url1, &state1, &deleted1);
   TestInterstitialPageStateGuard state_guard1(interstitial1);
   interstitial1->Show();
-  interstitial1->TestDidNavigate(1, url1);
+  interstitial1->TestDidNavigate(url1);
 
   // Take action.  The interstitial won't be hidden until the navigation is
   // committed.
@@ -2044,7 +2043,7 @@ TEST_F(WebContentsImplTest, ShowInterstitialProceedShowInterstitial) {
       new TestInterstitialPage(contents(), true, url2, &state2, &deleted2);
   TestInterstitialPageStateGuard state_guard2(interstitial2);
   interstitial2->Show();
-  interstitial2->TestDidNavigate(1, url2);
+  interstitial2->TestDidNavigate(url2);
 
   // Showing interstitial2 should have caused interstitial1 to go away.
   EXPECT_EQ(TestInterstitialPage::UNDECIDED, state2);
@@ -2090,7 +2089,7 @@ TEST_F(WebContentsImplTest, NavigateBeforeInterstitialShows) {
   ASSERT_FALSE(deleted);
 
   // Now let's make the interstitial navigation commit.
-  interstitial->TestDidNavigate(1, interstitial_url);
+  interstitial->TestDidNavigate(interstitial_url);
 
   // After it loaded the interstitial should be gone.
   EXPECT_EQ(TestInterstitialPage::CANCELED, state);
@@ -2135,7 +2134,7 @@ TEST_F(WebContentsImplTest, TwoQuickInterstitials) {
   ASSERT_FALSE(deleted2);
 
   // Make the interstitial navigation commit it should be showing.
-  interstitial2->TestDidNavigate(1, interstitial_url);
+  interstitial2->TestDidNavigate(interstitial_url);
   EXPECT_EQ(interstitial2, contents()->GetInterstitialPage());
 }
 
@@ -2162,7 +2161,7 @@ TEST_F(WebContentsImplTest, InterstitialCrasher) {
   interstitial =
       new TestInterstitialPage(contents(), true, url, &state, &deleted);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, url);
+  interstitial->TestDidNavigate(url);
   // Simulate a renderer crash.
   interstitial->TestRenderViewTerminated(
       base::TERMINATION_STATUS_PROCESS_CRASHED, -1);
@@ -2191,7 +2190,7 @@ TEST_F(WebContentsImplTest, NewInterstitialDoesNotCancelPendingEntry) {
       new TestInterstitialPage(contents(), true, kGURL, &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, kGURL);
+  interstitial->TestDidNavigate(kGURL);
 
   // Initiate a new navigation from the browser that also triggers an
   // interstitial.
@@ -2204,7 +2203,7 @@ TEST_F(WebContentsImplTest, NewInterstitialDoesNotCancelPendingEntry) {
       new TestInterstitialPage(contents(), true, kGURL, &state2, &deleted2);
   TestInterstitialPageStateGuard state_guard2(interstitial2);
   interstitial2->Show();
-  interstitial2->TestDidNavigate(1, kGURL);
+  interstitial2->TestDidNavigate(kGURL);
 
   // Make sure we still have an entry.
   NavigationEntry* entry = contents()->GetController().GetPendingEntry();
@@ -2240,7 +2239,7 @@ TEST_F(WebContentsImplTest, NoJSMessageOnInterstitials) {
       new TestInterstitialPage(contents(), true, kGURL, &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, kGURL);
+  interstitial->TestDidNavigate(kGURL);
 
   // While the interstitial is showing, let's simulate the hidden page
   // attempting to show a JS message.
@@ -2273,7 +2272,7 @@ TEST_F(WebContentsImplTest, CopyStateFromAndPruneSourceInterstitial) {
       new TestInterstitialPage(contents(), true, url2, &state, &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, url2);
+  interstitial->TestDidNavigate(url2);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_EQ(2, controller().GetEntryCount());
 
@@ -2325,7 +2324,7 @@ TEST_F(WebContentsImplTest, CopyStateFromAndPruneTargetInterstitial) {
                                &deleted);
   TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
-  interstitial->TestDidNavigate(1, url3);
+  interstitial->TestDidNavigate(url3);
   EXPECT_TRUE(interstitial->is_showing());
   EXPECT_EQ(2, other_controller.GetEntryCount());