Revert 108186 - Ensure forced process swaps use the correct page_id and SiteInstance.

BUG=102408
TEST=See bug

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

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108567 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 16 insertions, 83 deletions

diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index fd2f206..0719038 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -369,58 +369,6 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, ReloadIntoAppProcess) {
       contents->render_view_host()->process()->id()));
 }
 
-// Ensure that page_ids are handled correctly when we force a process swap
-// for an installed or uninstalled app.  (http://crbug.com/102408)
-IN_PROC_BROWSER_TEST_F(AppApiTest, BackToAppProcess) {
-  ExtensionProcessManager* extension_process_manager =
-      browser()->profile()->GetExtensionProcessManager();
-
-  host_resolver()->AddRule("*", "127.0.0.1");
-  ASSERT_TRUE(test_server()->Start());
-
-  // The app under test acts on URLs whose host is "localhost",
-  // so the URLs we navigate to must have host "localhost".
-  GURL base_url = GetTestBaseURL("app_process");
-
-  // Load an app URL before loading the app.
-  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path1/empty.html"));
-  TabContents* contents = browser()->GetTabContentsAt(0);
-  EXPECT_FALSE(extension_process_manager->IsExtensionProcess(
-      contents->render_view_host()->process()->id()));
-  int orig_page_id = contents->controller().GetLastCommittedEntry()->page_id();
-
-  // Navigate to a second app URL before loading the app.
-  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path2/empty.html"));
-  EXPECT_FALSE(extension_process_manager->IsExtensionProcess(
-      contents->render_view_host()->process()->id()));
-  EXPECT_EQ(orig_page_id + 1,
-            contents->controller().GetLastCommittedEntry()->page_id());
-
-  // Load app and go back.  We expect a process swap, but we also expect the
-  // same page_id to be used and the SiteInstance to be updated.
-  const Extension* app =
-      LoadExtension(test_data_dir_.AppendASCII("app_process"));
-  ASSERT_TRUE(app);
-  ui_test_utils::WindowedNotificationObserver back_observer(
-      content::NOTIFICATION_LOAD_STOP,
-      content::Source<NavigationController>(
-          &browser()->GetSelectedTabContentsWrapper()->controller()));
-  browser()->GoBack(CURRENT_TAB);
-  back_observer.Wait();
-  EXPECT_TRUE(extension_process_manager->IsExtensionProcess(
-      contents->render_view_host()->process()->id()));
-  EXPECT_EQ(orig_page_id,
-            contents->controller().GetLastCommittedEntry()->page_id());
-
-  // Now navigate to a different app URL via the renderer process.
-  // The NavigationController should recognize it as a new navigation.
-  NavigateTabHelper(contents, base_url.Resolve("path1/simple.html"));
-  EXPECT_TRUE(extension_process_manager->IsExtensionProcess(
-      contents->render_view_host()->process()->id()));
-  EXPECT_EQ(orig_page_id + 2,
-            contents->controller().GetLastCommittedEntry()->page_id());
-}
-
 
 // Tests that if we have a non-app process (path3/container.html) that has an
 // iframe with  a URL in the app's extent (path1/iframe.html), then opening a
diff --git a/chrome/test/data/extensions/api_test/app_process/path1/simple.html b/chrome/test/data/extensions/api_test/app_process/path1/simple.html
deleted file mode 100644
index 02af640..0000000
--- a/chrome/test/data/extensions/api_test/app_process/path1/simple.html
+++ /dev/null
@@ -1 +0,0 @@
-<title>Simple page</title>
diff --git a/content/browser/tab_contents/render_view_host_manager.cc b/content/browser/tab_contents/render_view_host_manager.cc
index adfa501..2627862 100644
--- a/content/browser/tab_contents/render_view_host_manager.cc
+++ b/content/browser/tab_contents/render_view_host_manager.cc
@@ -391,17 +391,10 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
   // is part of an app that has been installed or uninstalled since the last
   // visit.
   if (entry.site_instance()) {
-    if (entry.site_instance()->HasWrongProcessForURL(dest_url)) {
-      // If we need to swap to a different SiteInstance, the new one should have
-      // the same max_page_id as the current one so that it identifies new vs
-      // existing navigations correctly.  We also need to update the entry's
-      // SiteInstance, which we will do in TabContents::NavigateToEntry.
-      SiteInstance* new_instance =
-          curr_instance->GetRelatedSiteInstance(dest_url);
-      new_instance->UpdateMaxPageID(curr_instance->max_page_id());
-      return new_instance;
-    }
-    return entry.site_instance();
+    if (entry.site_instance()->HasWrongProcessForURL(dest_url))
+      return curr_instance->GetRelatedSiteInstance(dest_url);
+    else
+      return entry.site_instance();
   }
 
   // (UGLY) HEURISTIC, process-per-site only:
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index ecc89ff..e920f98 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -571,34 +571,27 @@ TabContents* TabContents::OpenURL(const OpenURLParams& params) {
 
 bool TabContents::NavigateToPendingEntry(
     NavigationController::ReloadType reload_type) {
-  return NavigateToEntry(controller_.pending_entry(), reload_type);
+  return NavigateToEntry(*controller_.pending_entry(), reload_type);
 }
 
 bool TabContents::NavigateToEntry(
-    NavigationEntry* entry,
+    const NavigationEntry& entry,
     NavigationController::ReloadType reload_type) {
   // The renderer will reject IPC messages with URLs longer than
   // this limit, so don't attempt to navigate with a longer URL.
-  if (entry->url().spec().size() > content::kMaxURLChars)
+  if (entry.url().spec().size() > content::kMaxURLChars)
     return false;
 
-  RenderViewHost* dest_render_view_host = render_manager_.Navigate(*entry);
+  RenderViewHost* dest_render_view_host = render_manager_.Navigate(entry);
   if (!dest_render_view_host)
     return false;  // Unable to create the desired render view host.
 
-  // If we were forced to swap the entry's existing SiteInstance, we need to
-  // update it before the navigation begins so that we can find it when the
-  // navigation commits.
-  if (entry->site_instance() &&
-      entry->site_instance() != dest_render_view_host->site_instance())
-    entry->set_site_instance(dest_render_view_host->site_instance());
-
   // For security, we should never send non-Web-UI URLs to a Web UI renderer.
   // Double check that here.
   int enabled_bindings = dest_render_view_host->enabled_bindings();
   bool is_allowed_in_web_ui_renderer = content::GetContentClient()->
       browser()->GetWebUIFactory()->IsURLAcceptableForWebUI(browser_context(),
-                                                            entry->url());
+                                                            entry.url());
   CHECK(!(enabled_bindings & content::BINDINGS_POLICY_WEB_UI) ||
         is_allowed_in_web_ui_renderer);
 
@@ -607,7 +600,7 @@ bool TabContents::NavigateToEntry(
   if (devtools_manager) {  // NULL in unit tests.
     devtools_manager->OnNavigatingToPendingEntry(render_view_host(),
                                                  dest_render_view_host,
-                                                 entry->url());
+                                                 entry.url());
   }
 
   // Used for page load time metrics.
@@ -615,24 +608,24 @@ bool TabContents::NavigateToEntry(
 
   // Navigate in the desired RenderViewHost.
   ViewMsg_Navigate_Params navigate_params;
-  MakeNavigateParams(*entry, controller_, delegate_, reload_type,
+  MakeNavigateParams(entry, controller_, delegate_, reload_type,
                      &navigate_params);
   dest_render_view_host->Navigate(navigate_params);
 
-  if (entry->page_id() == -1) {
+  if (entry.page_id() == -1) {
     // HACK!!  This code suppresses javascript: URLs from being added to
     // session history, which is what we want to do for javascript: URLs that
     // do not generate content.  What we really need is a message from the
     // renderer telling us that a new page was not created.  The same message
     // could be used for mailto: URLs and the like.
-    if (entry->url().SchemeIs(chrome::kJavaScriptScheme))
+    if (entry.url().SchemeIs(chrome::kJavaScriptScheme))
       return false;
   }
 
   // Notify observers about navigation.
   FOR_EACH_OBSERVER(TabContentsObserver,
                     observers_,
-                    NavigateToPendingEntry(entry->url(), reload_type));
+                    NavigateToPendingEntry(entry.url(), reload_type));
 
   if (delegate_)
     delegate_->DidNavigateToPendingEntry(this);
@@ -1097,7 +1090,7 @@ void TabContents::OnGoToEntryAtOffset(int offset) {
         content::PageTransitionFromInt(
             entry->transition_type() |
             content::PAGE_TRANSITION_FORWARD_BACK));
-    NavigateToEntry(entry, NavigationController::NO_RELOAD);
+    NavigateToEntry(*entry, NavigationController::NO_RELOAD);
 
     // If the entry is being restored and doesn't have a SiteInstance yet, fill
     // it in now that we know. This allows us to find the entry when it commits.
diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h
index 11c0a82..9c7d2b4 100644
--- a/content/browser/tab_contents/tab_contents.h
+++ b/content/browser/tab_contents/tab_contents.h
@@ -592,7 +592,7 @@ class CONTENT_EXPORT TabContents : public PageNavigator,
   // 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(NavigationEntry* entry,
+  bool NavigateToEntry(const NavigationEntry& entry,
                        NavigationController::ReloadType reload_type);
 
   // Sets the history for this tab_contents to |history_length| entries, and