Merge 114862 - Make page IDs be specific to a RenderView rather than global to its process.

This avoids races that cause the browser to kill the renderer.

BUG=106616
TEST=Restore Chrome with an extension's options page showing.

Review URL: http://codereview.chromium.org/8910020

TBR=creis@chromium.org
Review URL: http://codereview.chromium.org/9007036

git-svn-id: svn://svn.chromium.org/chrome/branches/963/src@115254 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 41 insertions, 25 deletions

diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc
index da5957d..9eda8a35 100644
--- a/chrome/browser/tab_contents/web_contents_unittest.cc
+++ b/chrome/browser/tab_contents/web_contents_unittest.cc
@@ -235,6 +235,29 @@ TEST_F(TabContentsTest, NTPViewSource) {
   EXPECT_EQ(ASCIIToUTF16(kUrl), contents()->GetTitle());
 }
 
+// Test to ensure UpdateMaxPageID is working properly.
+TEST_F(TabContentsTest, UpdateMaxPageID) {
+  SiteInstance* instance1 = contents()->GetSiteInstance();
+  scoped_refptr<SiteInstance> instance2(SiteInstance::CreateSiteInstance(NULL));
+
+  // Starts at -1.
+  EXPECT_EQ(-1, contents()->GetMaxPageID());
+  EXPECT_EQ(-1, contents()->GetMaxPageIDForSiteInstance(instance1));
+  EXPECT_EQ(-1, contents()->GetMaxPageIDForSiteInstance(instance2));
+
+  // Make sure max_page_id_ is monotonically increasing per SiteInstance.
+  contents()->UpdateMaxPageID(3);
+  contents()->UpdateMaxPageID(1);
+  EXPECT_EQ(3, contents()->GetMaxPageID());
+  EXPECT_EQ(3, contents()->GetMaxPageIDForSiteInstance(instance1));
+  EXPECT_EQ(-1, contents()->GetMaxPageIDForSiteInstance(instance2));
+
+  contents()->UpdateMaxPageIDForSiteInstance(instance2, 7);
+  EXPECT_EQ(3, contents()->GetMaxPageID());
+  EXPECT_EQ(3, contents()->GetMaxPageIDForSiteInstance(instance1));
+  EXPECT_EQ(7, contents()->GetMaxPageIDForSiteInstance(instance2));
+}
+
 // Test simple same-SiteInstance navigation.
 TEST_F(TabContentsTest, SimpleNavigation) {
   TestRenderViewHost* orig_rvh = rvh();
diff --git a/chrome/browser/ui/webui/ntp/new_tab_ui_browsertest.cc b/chrome/browser/ui/webui/ntp/new_tab_ui_browsertest.cc
index 39571e2..4a79f8e 100644
--- a/chrome/browser/ui/webui/ntp/new_tab_ui_browsertest.cc
+++ b/chrome/browser/ui/webui/ntp/new_tab_ui_browsertest.cc
@@ -45,8 +45,7 @@ IN_PROC_BROWSER_TEST_F(NewTabUIBrowserTest, LoadNTPInExistingProcess) {
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), GURL(chrome::kChromeUINewTabURL), NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
-  EXPECT_EQ(1,
-            browser()->GetTabContentsAt(1)->GetSiteInstance()->max_page_id());
+  EXPECT_EQ(1, browser()->GetTabContentsAt(1)->GetMaxPageID());
 
   // Navigate that tab to another site.  This allows the NTP process to exit,
   // but it keeps the NTP SiteInstance (and its max_page_id) alive in history.
@@ -61,34 +60,30 @@ IN_PROC_BROWSER_TEST_F(NewTabUIBrowserTest, LoadNTPInExistingProcess) {
     process_exited_observer.Wait();
   }
 
-  // Create and close another two NTPs to inflate the max_page_id.
+  // Creating a NTP in another tab should not be affected, since page IDs
+  // are now specific to a tab.
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), GURL(chrome::kChromeUINewTabURL), NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
-  EXPECT_EQ(2,
-            browser()->GetTabContentsAt(2)->GetSiteInstance()->max_page_id());
-  browser()->CloseTab();
-  ui_test_utils::NavigateToURLWithDisposition(
-      browser(), GURL(chrome::kChromeUINewTabURL), NEW_FOREGROUND_TAB,
-      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
-  EXPECT_EQ(3,
-            browser()->GetTabContentsAt(2)->GetSiteInstance()->max_page_id());
+  EXPECT_EQ(1, browser()->GetTabContentsAt(2)->GetMaxPageID());
   browser()->CloseTab();
 
   // Open another Web UI page in a new tab.
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), GURL(chrome::kChromeUISettingsURL), NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
-  EXPECT_EQ(1,
-            browser()->GetTabContentsAt(2)->GetSiteInstance()->max_page_id());
+  EXPECT_EQ(1, browser()->GetTabContentsAt(2)->GetMaxPageID());
 
-  // At this point, opening another NTP will use the old SiteInstance (with a
-  // large max_page_id) in the existing Web UI process (with a small page ID).
-  // Make sure we don't confuse this as an existing navigation to an unknown
-  // entry.
+  // At this point, opening another NTP will use the old SiteInstance in the
+  // existing Web UI process, but the page IDs shouldn't affect each other.
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), GURL(chrome::kChromeUINewTabURL), NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
-  EXPECT_EQ(4,
-            browser()->GetTabContentsAt(3)->GetSiteInstance()->max_page_id());
+  EXPECT_EQ(1, browser()->GetTabContentsAt(3)->GetMaxPageID());
+
+  // Only navigating to the NTP in the original tab should have a higher
+  // page ID.
+  browser()->ActivateTabAt(1, true);
+  ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
+  EXPECT_EQ(2, browser()->GetTabContentsAt(1)->GetMaxPageID());
 }
diff --git a/chrome/browser/visitedlink/visitedlink_unittest.cc b/chrome/browser/visitedlink/visitedlink_unittest.cc
index 487c73d..2c5218a 100644
--- a/chrome/browser/visitedlink/visitedlink_unittest.cc
+++ b/chrome/browser/visitedlink/visitedlink_unittest.cc
@@ -660,7 +660,7 @@ TEST_F(VisitedLinkEventsTest, Coalescense) {
 
 TEST_F(VisitedLinkEventsTest, Basics) {
   VisitedLinkMaster* master = profile()->GetVisitedLinkMaster();
-  rvh()->CreateRenderView(string16());
+  rvh()->CreateRenderView(string16(), -1);
 
   // Add a few URLs.
   master->AddURL(GURL("http://acidtests.org/"));
@@ -684,7 +684,7 @@ TEST_F(VisitedLinkEventsTest, Basics) {
 
 TEST_F(VisitedLinkEventsTest, TabVisibility) {
   VisitedLinkMaster* master = profile()->GetVisitedLinkMaster();
-  rvh()->CreateRenderView(string16());
+  rvh()->CreateRenderView(string16(), -1);
 
   // Simulate tab becoming inactive.
   rvh()->WasHidden();
diff --git a/chrome/test/base/browser_with_test_window_test.cc b/chrome/test/base/browser_with_test_window_test.cc
index f4077a5..1751ac5 100644
--- a/chrome/test/base/browser_with_test_window_test.cc
+++ b/chrome/test/base/browser_with_test_window_test.cc
@@ -74,8 +74,6 @@ void BrowserWithTestWindowTest::CommitPendingLoad(
 
   TestRenderViewHost* old_rvh =
       TestRenderViewHostForTab(controller->tab_contents());
-  MockRenderProcessHost* mock_rph = static_cast<MockRenderProcessHost*>(
-    old_rvh->process());
 
   TestRenderViewHost* pending_rvh = static_cast<TestRenderViewHost*>(
       controller->tab_contents()->render_manager_for_testing()->
@@ -100,10 +98,10 @@ void BrowserWithTestWindowTest::CommitPendingLoad(
         controller->pending_entry()->transition_type());
   } else {
     test_rvh->SendNavigateWithTransition(
-        mock_rph->max_page_id() + 1,
+        controller->tab_contents()->
+            GetMaxPageIDForSiteInstance(test_rvh->site_instance()) + 1,
         controller->pending_entry()->url(),
         controller->pending_entry()->transition_type());
-    mock_rph->UpdateMaxPageID(mock_rph->max_page_id() + 1);
   }
 
   // Simulate the SwapOut_ACK that fires if you commit a cross-site navigation