Don't close tabs from crashed extensions with background pages.

Make the crashed extension reload when the sad tab is reloaded.
('Extensions' includes packaged apps.)

BUG=71629
BUG=94177
TEST=Added ExtensionCrashRecoveryTest.ReloadTabsWithBackgroundPage. Manually: Open a packaged app with a background page. Kill it from Task Manager. Its tab should not close. Reload the tab and it should show the app (and the crash notification bubble should be gone).


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

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

chrome/browser/background/background_contents_service.cc

@@ -97,7 +97,7 @@ class CrashNotificationDelegate : public NotificationDelegate {
 
 void ShowBalloon(const Extension* extension, Profile* profile) {
   string16 message = l10n_util::GetStringFUTF16(
-      extension->is_hosted_app() ?  IDS_BACKGROUND_CRASHED_APP_BALLOON_MESSAGE :
+      extension->is_app() ?  IDS_BACKGROUND_CRASHED_APP_BALLOON_MESSAGE :
       IDS_BACKGROUND_CRASHED_EXTENSION_BALLOON_MESSAGE,
       UTF8ToUTF16(extension->name()));
   string16 content_url = DesktopNotificationService::CreateDataUrl(
@@ -298,8 +298,8 @@ void BackgroundContentsService::Observe(
     }
     case chrome::NOTIFICATION_EXTENSION_UNLOADED:
       switch (content::Details<UnloadedExtensionInfo>(details)->reason) {
-        // Intentionally fall through.
-        case extension_misc::UNLOAD_REASON_DISABLE:
+        case extension_misc::UNLOAD_REASON_DISABLE:    // Fall through.
+        case extension_misc::UNLOAD_REASON_TERMINATE:  // Fall through.
         case extension_misc::UNLOAD_REASON_UNINSTALL:
           ShutdownAssociatedBackgroundContents(
               ASCIIToUTF16(content::Details<UnloadedExtensionInfo>(details)->

chrome/browser/extensions/extension_crash_recovery_browsertest.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
@@ -453,3 +454,42 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, MAYBE_CrashAndUnloadAll) {
   ASSERT_EQ(crash_size_before,
             GetExtensionService()->terminated_extensions()->size());
 }
+
+// Test that when an extension with a background page that has a tab open
+// crashes, the tab stays open, and reloading it reloads the extension.
+// Regression test for issue 71629.
+IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest,
+                       ReloadTabsWithBackgroundPage) {
+  TabStripModel* tab_strip = browser()->tabstrip_model();
+  const size_t size_before = GetExtensionService()->extensions()->size();
+  const size_t crash_size_before =
+      GetExtensionService()->terminated_extensions()->size();
+  LoadTestExtension();
+
+  // Open a tab extension.
+  browser()->NewTab();
+  ui_test_utils::NavigateToURL(
+      browser(),
+      GURL("chrome-extension://" + first_extension_id_ + "/background.html"));
+
+  const int tabs_before = tab_strip->count();
+  CrashExtension(size_before);
+
+  // Tab should still be open, and extension should be crashed.
+  EXPECT_EQ(tabs_before, tab_strip->count());
+  EXPECT_EQ(size_before, GetExtensionService()->extensions()->size());
+  EXPECT_EQ(crash_size_before + 1,
+            GetExtensionService()->terminated_extensions()->size());
+
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        content::Source<NavigationController>(
+            &browser()->GetSelectedTabContentsWrapper()->controller()));
+    browser()->Reload(CURRENT_TAB);
+    observer.Wait();
+  }
+  // Extension should now be loaded.
+  ASSERT_EQ(size_before + 1, GetExtensionService()->extensions()->size());
+  ASSERT_EQ(0U, CountBalloons());
+}

chrome/browser/extensions/extension_info_map.cc

@@ -64,11 +64,13 @@ void ExtensionInfoMap::RemoveExtension(const std::string& extension_id,
   CheckOnValidThread();
   const Extension* extension = extensions_.GetByID(extension_id);
   extra_data_.erase(extension_id);  // we don't care about disabled extra data
+  bool was_uninstalled = (reason != extension_misc::UNLOAD_REASON_DISABLE &&
+                          reason != extension_misc::UNLOAD_REASON_TERMINATE);
   if (extension) {
-    if (reason == extension_misc::UNLOAD_REASON_DISABLE)
+    if (!was_uninstalled)
       disabled_extensions_.Insert(extension);
     extensions_.Remove(extension_id);
-  } else if (reason != extension_misc::UNLOAD_REASON_DISABLE) {
+  } else if (was_uninstalled) {
     // If the extension was uninstalled, make sure it's removed from the map of
     // disabled extensions.
     disabled_extensions_.Remove(extension_id);

chrome/browser/extensions/extension_service.cc

@@ -2122,7 +2122,10 @@ void ExtensionService::TrackTerminatedExtension(const Extension* extension) {
   if (terminated_extension_ids_.insert(extension->id()).second)
     terminated_extensions_.push_back(make_scoped_refptr(extension));
 
-  UnloadExtension(extension->id(), extension_misc::UNLOAD_REASON_DISABLE);
+  // TODO(yoz): Listen to navcontrollers for that extension. Is this a todo?
+
+  // TODO(yoz): make sure this is okay in *ALL* the listeners!
+  UnloadExtension(extension->id(), extension_misc::UNLOAD_REASON_TERMINATE);
 }
 
 void ExtensionService::UntrackTerminatedExtension(const std::string& id) {

chrome/browser/renderer_host/chrome_render_view_host_observer.cc

@@ -139,7 +139,15 @@ const Extension* ChromeRenderViewHostObserver::GetExtension() {
   if (!service)
     return NULL;
 
-  // May be null if somebody typos a chrome-extension:// URL.
+  // Reload the extension if it has crashed.
+  // TODO(yoz): This reload doesn't happen synchronously for unpacked
+  //            extensions. It seems to be fast enough, but there is a race.
+  //            We should delay loading until the extension has reloaded.
+  if (service->GetTerminatedExtension(site.host()))
+    service->ReloadExtension(site.host());
+
+  // May be null if the extension doesn't exist, for example if somebody typos
+  // a chrome-extension:// URL.
   return service->GetExtensionByURL(site);
 }
 

chrome/browser/ui/browser.cc

@@ -4248,24 +4248,28 @@ void Browser::Observe(int type,
       if (window()->GetLocationBar())
         window()->GetLocationBar()->UpdatePageActions();
 
-      // Close any tabs from the unloaded extension.
-      const Extension* extension =
-          content::Details<UnloadedExtensionInfo>(details)->extension;
-      TabStripModel* model = tab_handler_->GetTabStripModel();
-      for (int i = model->count() - 1; i >= 0; --i) {
-        TabContents* tc = model->GetTabContentsAt(i)->tab_contents();
-        bool close_tab_contents =
-            tc->GetURL().SchemeIs(chrome::kExtensionScheme) &&
-            tc->GetURL().host() == extension->id();
-        // We want to close all panels originated by the unloaded extension.
-        close_tab_contents = close_tab_contents || (type_ == TYPE_PANEL &&
-            (web_app::GetExtensionIdFromApplicationName(app_name_) ==
+      // Close any tabs from the unloaded extension, unless it's terminated,
+      // in which case let the sad tabs remain.
+      if (content::Details<UnloadedExtensionInfo>(details)->reason !=
+          extension_misc::UNLOAD_REASON_TERMINATE) {
+        const Extension* extension =
+            content::Details<UnloadedExtensionInfo>(details)->extension;
+        TabStripModel* model = tab_handler_->GetTabStripModel();
+        for (int i = model->count() - 1; i >= 0; --i) {
+          TabContents* tc = model->GetTabContentsAt(i)->tab_contents();
+          bool close_tab_contents =
+              tc->GetURL().SchemeIs(chrome::kExtensionScheme) &&
+              tc->GetURL().host() == extension->id();
+          // We want to close all panels originated by the unloaded extension.
+          close_tab_contents = close_tab_contents ||
+              (type_ == TYPE_PANEL &&
+               (web_app::GetExtensionIdFromApplicationName(app_name_) ==
                 extension->id()));
-        if (close_tab_contents) {
-          CloseTabContents(tc);
+          if (close_tab_contents) {
+            CloseTabContents(tc);
+          }
         }
       }
-
       break;
     }
 

chrome/common/extensions/extension_constants.h

@@ -451,6 +451,7 @@ namespace extension_misc {
     UNLOAD_REASON_DISABLE,    // Extension is being disabled.
     UNLOAD_REASON_UPDATE,     // Extension is being updated to a newer version.
     UNLOAD_REASON_UNINSTALL,  // Extension is being uninstalled.
+    UNLOAD_REASON_TERMINATE,  // Extension has terminated.
   };
 
   // User actions on the sync promo (aka "Sign in to Chrome").