Swap processes on reload if a hosted app has been installed/uninstalled.

Also ensure the task manager only shows "App" if the process is an app process.

BUG=80621
TEST=AppApiTest.ReloadIntoAppProcess
TEST=TaskManagerBrowserTest.NoticeHostedAppTabs

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

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

8 files changed, 159 insertions, 15 deletions

diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index 7467c04..381fd49 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -189,3 +189,53 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcessRedirectBack) {
   EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(2)->render_view_host()->process());
 }
+
+// Ensure that reloading a URL after installing or uninstalling it as an app
+// correctly swaps the process.  (http://crbug.com/80621)
+IN_PROC_BROWSER_TEST_F(AppApiTest, ReloadIntoAppProcess) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kDisablePopupBlocking);
+
+  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::Replacements replace_host;
+  std::string host_str("localhost");  // must stay in scope with replace_host
+  replace_host.SetHostStr(host_str);
+  GURL base_url = test_server()->GetURL(
+      "files/extensions/api_test/app_process/");
+  base_url = base_url.ReplaceComponents(replace_host);
+
+  // 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(contents->render_view_host()->process()->is_extension_process());
+
+  // Load app and reload page.
+  const Extension* app =
+      LoadExtension(test_data_dir_.AppendASCII("app_process"));
+  ASSERT_TRUE(app);
+  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path1/empty.html"));
+  EXPECT_TRUE(contents->render_view_host()->process()->is_extension_process());
+
+  // Disable app and reload page.
+  DisableExtension(app->id());
+  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path1/empty.html"));
+  EXPECT_FALSE(contents->render_view_host()->process()->is_extension_process());
+
+  // Enable app and reload via JavaScript.
+  EnableExtension(app->id());
+  ASSERT_TRUE(ui_test_utils::ExecuteJavaScript(contents->render_view_host(),
+                                               L"", L"location.reload();"));
+  ui_test_utils::WaitForNavigation(&contents->controller());
+  EXPECT_TRUE(contents->render_view_host()->process()->is_extension_process());
+
+  // Disable app and reload via JavaScript.
+  DisableExtension(app->id());
+  ASSERT_TRUE(ui_test_utils::ExecuteJavaScript(contents->render_view_host(),
+                                               L"", L"location.reload();"));
+  ui_test_utils::WaitForNavigation(&contents->controller());
+  EXPECT_FALSE(contents->render_view_host()->process()->is_extension_process());
+}
diff --git a/chrome/browser/task_manager/task_manager_browsertest.cc b/chrome/browser/task_manager/task_manager_browsertest.cc
index 537efb8..4bd10d4 100644
--- a/chrome/browser/task_manager/task_manager_browsertest.cc
+++ b/chrome/browser/task_manager/task_manager_browsertest.cc
@@ -27,6 +27,7 @@
 #include "chrome/test/ui_test_utils.h"
 #include "content/common/page_transition_types.h"
 #include "grit/generated_resources.h"
+#include "net/base/mock_host_resolver.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -304,7 +305,7 @@ IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeAppTabs) {
   // a tab contents and an extension. The title should start with "App:".
   ASSERT_EQ(TaskManager::Resource::EXTENSION, model()->GetResourceType(2));
   ASSERT_TRUE(model()->GetResourceTabContents(2) != NULL);
-  ASSERT_TRUE(model()->GetResourceExtension(2) != NULL);
+  ASSERT_TRUE(model()->GetResourceExtension(2) == extension);
   string16 prefix = l10n_util::GetStringFUTF16(
       IDS_TASK_MANAGER_APP_PREFIX, string16());
   ASSERT_TRUE(StartsWith(model()->GetResourceTitle(2), prefix, true));
@@ -314,6 +315,56 @@ IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeAppTabs) {
   WaitForResourceChange(2);
 }
 
+IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeHostedAppTabs) {
+  // Show the task manager. This populates the model, and helps with debugging
+  // (you see the task manager).
+  browser()->window()->ShowTaskManager();
+
+  // Browser and the New Tab Page.
+  WaitForResourceChange(2);
+
+  // The app under test acts on URLs whose host is "localhost",
+  // so the URLs we navigate to must have host "localhost".
+  host_resolver()->AddRule("*", "127.0.0.1");
+  ASSERT_TRUE(test_server()->Start());
+  GURL::Replacements replace_host;
+  std::string host_str("localhost");  // must stay in scope with replace_host
+  replace_host.SetHostStr(host_str);
+  GURL base_url = test_server()->GetURL(
+      "files/extensions/api_test/app_process/");
+  base_url = base_url.ReplaceComponents(replace_host);
+
+  // Open a new tab to an app URL before the app is loaded.
+  GURL url(base_url.Resolve("path1/empty.html"));
+  AddTabAtIndex(0, url, PageTransition::TYPED);
+  ui_test_utils::WaitForNavigation(
+      &browser()->GetSelectedTabContents()->controller());
+
+  // Check that the third entry's title starts with "Tab:".
+  string16 tab_prefix = l10n_util::GetStringFUTF16(
+      IDS_TASK_MANAGER_TAB_PREFIX, string16());
+  ASSERT_TRUE(StartsWith(model()->GetResourceTitle(2), tab_prefix, true));
+
+  // Load the hosted app and make sure it still starts with "Tab:",
+  // since it hasn't changed to an app process yet.
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("api_test").AppendASCII("app_process")));
+  ASSERT_TRUE(StartsWith(model()->GetResourceTitle(2), tab_prefix, true));
+
+  // Now reload and check that the third entry's title now starts with "App:".
+  ui_test_utils::NavigateToURL(browser(), url);
+  string16 app_prefix = l10n_util::GetStringFUTF16(
+      IDS_TASK_MANAGER_APP_PREFIX, string16());
+  ASSERT_TRUE(StartsWith(model()->GetResourceTitle(2), app_prefix, true));
+
+  // Disable extension and reload page.
+  DisableExtension(last_loaded_extension_id_);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // The third entry's title should be back to a normal tab.
+  ASSERT_TRUE(StartsWith(model()->GetResourceTitle(2), tab_prefix, true));
+}
+
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeNotificationChanges) {
   EXPECT_EQ(0, model()->ResourceCount());
 
diff --git a/chrome/browser/task_manager/task_manager_resource_providers.cc b/chrome/browser/task_manager/task_manager_resource_providers.cc
index 99cb938..f3989c0 100644
--- a/chrome/browser/task_manager/task_manager_resource_providers.cc
+++ b/chrome/browser/task_manager/task_manager_resource_providers.cc
@@ -197,9 +197,11 @@ TaskManager::Resource::Type TaskManagerTabContentsResource::GetType() const {
 
 string16 TaskManagerTabContentsResource::GetTitle() const {
   // Fall back on the URL if there's no title.
-  string16 tab_title = tab_contents_->tab_contents()->GetTitle();
+  TabContents* contents = tab_contents_->tab_contents();
+  string16 tab_title = contents->GetTitle();
+  GURL url = contents->GetURL();
   if (tab_title.empty()) {
-    tab_title = UTF8ToUTF16(tab_contents_->tab_contents()->GetURL().spec());
+    tab_title = UTF8ToUTF16(url.spec());
     // Force URL to be LTR.
     tab_title = base::i18n::GetDisplayStringInLTRDirectionality(tab_title);
   } else {
@@ -215,13 +217,17 @@ string16 TaskManagerTabContentsResource::GetTitle() const {
     base::i18n::AdjustStringForLocaleDirection(&tab_title);
   }
 
+  // Only classify as an app if the URL is an app and the tab is hosting an
+  // extension process.  (It's possible to be showing the URL from before it
+  // was installed as an app.)
   ExtensionService* extensions_service =
       tab_contents_->profile()->GetExtensionService();
+  bool is_app = extensions_service->IsInstalledApp(url) &&
+      contents->GetRenderProcessHost()->is_extension_process();
 
   int message_id = GetMessagePrefixID(
-      extensions_service->IsInstalledApp(
-          tab_contents_->tab_contents()->GetURL()),
-      tab_contents_->tab_contents()->HostsExtension(),
+      is_app,
+      contents->HostsExtension(),
       tab_contents_->profile()->IsOffTheRecord(),
       IsPrerendering());
   return l10n_util::GetStringFUTF16(message_id, tab_title);
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 05a62ac..9e6a913 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -558,6 +558,16 @@ bool ChromeContentRendererClient::CrossesExtensionExtents(WebFrame* frame,
   if (old_url.is_empty() && frame->opener())
     old_url = frame->opener()->url();
 
+  // If this is a reload, check whether it has the wrong process type.  We
+  // should send it to the browser if it's an extension URL (e.g., hosted app)
+  // in a normal process, or if it's a process for an extension that has been
+  // uninstalled.
+  if (old_url == new_url) {
+    bool is_extension_url = !!extensions->GetByURL(new_url);
+    if (is_extension_url != extension_dispatcher_->is_extension_process())
+      return true;
+  }
+
   return !extensions->InSameExtent(old_url, new_url);
 }
 
diff --git a/content/browser/site_instance.cc b/content/browser/site_instance.cc
index f96e3b2..610de02 100644
--- a/content/browser/site_instance.cc
+++ b/content/browser/site_instance.cc
@@ -116,6 +116,18 @@ SiteInstance* SiteInstance::GetRelatedSiteInstance(const GURL& url) {
   return browsing_instance_->GetSiteInstanceForURL(url);
 }
 
+bool SiteInstance::HasWrongProcessForURL(const GURL& url) const {
+  // Having no process isn't a problem, since we'll assign it correctly.
+  if (!HasProcess())
+    return false;
+
+  // If the effective URL is an extension (e.g., for hosted apps) but the
+  // process is not (or vice versa), make sure we notice and fix it.
+  GURL effective_url = GetEffectiveURL(browsing_instance_->profile(), url);
+  return effective_url.SchemeIs(chrome::kExtensionScheme) !=
+      process_->is_extension_process();
+}
+
 /*static*/
 SiteInstance* SiteInstance::CreateSiteInstance(Profile* profile) {
   return new SiteInstance(new BrowsingInstance(profile));
diff --git a/content/browser/site_instance.h b/content/browser/site_instance.h
index 3b6346d..ebe1b3e 100644
--- a/content/browser/site_instance.h
+++ b/content/browser/site_instance.h
@@ -105,6 +105,11 @@ class SiteInstance : public base::RefCounted<SiteInstance>,
   // Darin suggests.
   SiteInstance* GetRelatedSiteInstance(const GURL& url);
 
+  // Returns whether this SiteInstance has a process that is the wrong type for
+  // the given URL.  If so, the browser should force a process swap when
+  // navigating to the URL.
+  bool HasWrongProcessForURL(const GURL& url) const;
+
   // Factory method to create a new SiteInstance.  This will create a new
   // new BrowsingInstance, so it should only be used when creating a new tab
   // from scratch (or similar circumstances).  Callers should ensure that
diff --git a/content/browser/tab_contents/navigation_entry.h b/content/browser/tab_contents/navigation_entry.h
index 671cc5b..4c118b3 100644
--- a/content/browser/tab_contents/navigation_entry.h
+++ b/content/browser/tab_contents/navigation_entry.h
@@ -206,7 +206,7 @@ class NavigationEntry {
   //
   // Note that the SiteInstance should usually not be changed after it is set,
   // but this may happen if the NavigationEntry was cloned and needs to use a
-  // different SiteInstance.
+  // different SiteInstance, or if a hosted app is installed or uninstalled.
   void set_site_instance(SiteInstance* site_instance);
   SiteInstance* site_instance() const {
     return site_instance_;
diff --git a/content/browser/tab_contents/render_view_host_manager.cc b/content/browser/tab_contents/render_view_host_manager.cc
index accb225..d827a087 100644
--- a/content/browser/tab_contents/render_view_host_manager.cc
+++ b/content/browser/tab_contents/render_view_host_manager.cc
@@ -393,9 +393,19 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
     SiteInstance* curr_instance) {
   // NOTE: This is only called when ShouldTransitionCrossSite is true.
 
-  // If the entry has an instance already, we should use it.
-  if (entry.site_instance())
-    return entry.site_instance();
+  const GURL& dest_url = entry.url();
+  NavigationController& controller = delegate_->GetControllerForRenderManager();
+  Profile* profile = controller.profile();
+
+  // If the entry has an instance already we should use it, unless the URL
+  // 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))
+      return curr_instance->GetRelatedSiteInstance(dest_url);
+    else
+      return entry.site_instance();
+  }
 
   // (UGLY) HEURISTIC, process-per-site only:
   //
@@ -411,10 +421,6 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
       entry.transition_type() == PageTransition::GENERATED)
     return curr_instance;
 
-  const GURL& dest_url = entry.url();
-  NavigationController& controller = delegate_->GetControllerForRenderManager();
-  Profile* profile = controller.profile();
-
   // If we haven't used our SiteInstance (and thus RVH) yet, then we can use it
   // for this entry.  We won't commit the SiteInstance to this site until the
   // navigation commits (in DidNavigate), unless the navigation entry was
@@ -480,7 +486,11 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
   const GURL& current_url = (curr_entry) ? curr_entry->url() :
       curr_instance->site();
 
-  if (SiteInstance::IsSameWebSite(profile, current_url, dest_url)) {
+  // Use the current SiteInstance for same site navigations, as long as the
+  // process type is correct.  (The URL may have been installed as an app since
+  // the last time we visited it.)
+  if (SiteInstance::IsSameWebSite(profile, current_url, dest_url) &&
+      !curr_instance->HasWrongProcessForURL(dest_url)) {
     return curr_instance;
   } else if (ShouldSwapProcessesForNavigation(curr_entry, &entry)) {
     // When we're swapping, we need to force the site instance AND browsing