Fix "crashed extension" infobar browser crashes.

This is a general rework of how "crashed extension" infobar works
and how the extension is actually recovered after the crash.

TEST=See bug.
http://crbug.com/15888

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

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

14 files changed, 175 insertions, 96 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 2ea148c..c4f5cb2 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -2068,8 +2068,8 @@ each locale. -->
       </message>
 
       <!-- Extension Crashed Info Bar-->
-      <message name="IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON" desc="Title of the restart button in the extension crashed infobar. After the button is clicked, the extension process will be restarted.">
-        Restart
+      <message name="IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON" desc="Title of the reload button in the extension crashed infobar. After the button is clicked, the extension will be reloaded.">
+        Reload
       </message>
       <message name="IDS_EXTENSION_CRASHED_INFOBAR_MESSAGE" desc="Message displayed on the extension crashed infobar.">
         The following extension has crashed : <ph name="EXTENSION_NAME">$1<ex>Buildbot Monitor</ex></ph>
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 562f3b0..21caab1 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_shelf.h"
 #include "chrome/browser/download/download_started_animation.h"
+#include "chrome/browser/extensions/crashed_extension_infobar.h"
 #include "chrome/browser/find_bar.h"
 #include "chrome/browser/find_bar_controller.h"
 #include "chrome/browser/location_bar.h"
@@ -190,6 +191,8 @@ Browser::Browser(Type type, Profile* profile)
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
                  NotificationService::AllSources());
+  registrar_.Add(this, NotificationType::EXTENSION_PROCESS_CRASHED,
+                 NotificationService::AllSources());
   registrar_.Add(this, NotificationType::BROWSER_THEME_CHANGED,
                  NotificationService::AllSources());
 
@@ -2101,6 +2104,18 @@ void Browser::Observe(NotificationType type,
       break;
     }
 
+    case NotificationType::EXTENSION_PROCESS_CRASHED: {
+      TabContents* tab_contents = GetSelectedTabContents();
+      if (!tab_contents)
+        break;
+      ExtensionsService* extensions_service =
+          Source<ExtensionsService>(source).ptr();
+      ExtensionHost* extension_host = Details<ExtensionHost>(details).ptr();
+      tab_contents->AddInfoBar(new CrashedExtensionInfoBarDelegate(
+          tab_contents, extensions_service, extension_host->extension()));
+      break;
+    }
+
     case NotificationType::BROWSER_THEME_CHANGED:
       window()->UserChangedTheme();
       break;
diff --git a/chrome/browser/extensions/crashed_extension_infobar.cc b/chrome/browser/extensions/crashed_extension_infobar.cc
new file mode 100644
index 0000000..80ab865
--- /dev/null
+++ b/chrome/browser/extensions/crashed_extension_infobar.cc
@@ -0,0 +1,56 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/crashed_extension_infobar.h"
+
+#include "app/l10n_util.h"
+#include "app/resource_bundle.h"
+#include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/common/extensions/extension.h"
+#include "grit/browser_resources.h"
+#include "grit/generated_resources.h"
+#include "grit/theme_resources.h"
+
+CrashedExtensionInfoBarDelegate::CrashedExtensionInfoBarDelegate(
+    TabContents* tab_contents,
+    ExtensionsService* extensions_service,
+    const Extension* extension)
+    : ConfirmInfoBarDelegate(tab_contents),
+      extensions_service_(extensions_service),
+      extension_id_(extension->id()),
+      extension_name_(extension->name()) {
+  DCHECK(extensions_service_);
+  DCHECK(!extension_id_.empty());
+}
+
+std::wstring CrashedExtensionInfoBarDelegate::GetMessageText() const {
+  return l10n_util::GetStringF(IDS_EXTENSION_CRASHED_INFOBAR_MESSAGE,
+      UTF8ToWide(extension_name_));
+}
+
+void CrashedExtensionInfoBarDelegate::InfoBarClosed() {
+  delete this;
+}
+
+SkBitmap* CrashedExtensionInfoBarDelegate::GetIcon() const {
+  // TODO(erikkay): Create extension-specific icon. http://crbug.com/14591
+  return ResourceBundle::GetSharedInstance().GetBitmapNamed(
+      IDR_INFOBAR_PLUGIN_CRASHED);
+}
+
+int CrashedExtensionInfoBarDelegate::GetButtons() const {
+  return BUTTON_OK;
+}
+
+std::wstring CrashedExtensionInfoBarDelegate::GetButtonLabel(
+    ConfirmInfoBarDelegate::InfoBarButton button) const {
+  if (button == BUTTON_OK)
+    return l10n_util::GetString(IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON);
+  return ConfirmInfoBarDelegate::GetButtonLabel(button);
+}
+
+bool CrashedExtensionInfoBarDelegate::Accept() {
+  extensions_service_->ReloadExtension(extension_id_);
+  return true;
+}
diff --git a/chrome/browser/extensions/crashed_extension_infobar.h b/chrome/browser/extensions/crashed_extension_infobar.h
new file mode 100644
index 0000000..4a4e625
--- /dev/null
+++ b/chrome/browser/extensions/crashed_extension_infobar.h
@@ -0,0 +1,46 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_CRASHED_EXTENSION_INFOBAR_H_
+#define CHROME_BROWSER_EXTENSIONS_CRASHED_EXTENSION_INFOBAR_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "chrome/browser/tab_contents/infobar_delegate.h"
+
+class Extension;
+class ExtensionsService;
+class SkBitmap;
+
+// This infobar will be displayed when an extension process crashes. It allows
+// the user to reload the crashed extension.
+class CrashedExtensionInfoBarDelegate : public ConfirmInfoBarDelegate {
+ public:
+  // |tab_contents| should point to the TabContents the infobar will be added
+  // to. |extension| should be the crashed extension, and |extensions_service|
+  // the ExtensionsService which manages that extension.
+  CrashedExtensionInfoBarDelegate(TabContents* tab_contents,
+                                  ExtensionsService* extensions_service,
+                                  const Extension* extension);
+
+  // ConfirmInfoBarDelegate
+  virtual std::wstring GetMessageText() const;
+  virtual void InfoBarClosed();
+  virtual SkBitmap* GetIcon() const;
+  virtual int GetButtons() const;
+  virtual std::wstring GetButtonLabel(
+      ConfirmInfoBarDelegate::InfoBarButton button) const;
+  virtual bool Accept();
+
+ private:
+  ExtensionsService* extensions_service_;
+
+  const std::string extension_id_;
+  const std::string extension_name_;
+
+  DISALLOW_COPY_AND_ASSIGN(CrashedExtensionInfoBarDelegate);
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_CRASHED_EXTENSION_INFOBAR_H_
diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index 898aeaa..010f841 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/extensions/extension_host.h"
 
-#include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/string_util.h"
 #include "chrome/browser/browser.h"
@@ -17,9 +16,6 @@
 #include "chrome/browser/renderer_host/render_widget_host.h"
 #include "chrome/browser/renderer_host/render_widget_host_view.h"
 #include "chrome/browser/renderer_host/site_instance.h"
-#include "chrome/browser/tab_contents/infobar_delegate.h"
-#include "chrome/browser/tab_contents/tab_contents.h"
-#include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/common/bindings_policy.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/notification_service.h"
@@ -29,57 +25,9 @@
 #include "chrome/common/url_constants.h"
 
 #include "grit/browser_resources.h"
-#include "grit/generated_resources.h"
-#include "grit/theme_resources.h"
 
 #include "webkit/glue/context_menu.h"
 
-namespace {
-
-class CrashedExtensionInfobarDelegate : public ConfirmInfoBarDelegate {
- public:
-  CrashedExtensionInfobarDelegate(TabContents* tab_contents,
-                                  ExtensionHost* extension_host)
-      : ConfirmInfoBarDelegate(tab_contents),
-        extension_host_(extension_host) {
-  }
-
-  virtual std::wstring GetMessageText() const {
-    return l10n_util::GetStringF(IDS_EXTENSION_CRASHED_INFOBAR_MESSAGE,
-        UTF8ToWide(extension_host_->extension()->name()));
-  }
-
-  virtual SkBitmap* GetIcon() const {
-    // TODO(erikkay): Create extension-specific icon. http://crbug.com/14591
-    return ResourceBundle::GetSharedInstance().GetBitmapNamed(
-        IDR_INFOBAR_PLUGIN_CRASHED);
-  }
-
-  virtual int GetButtons() const {
-    return BUTTON_OK;
-  }
-
-  virtual std::wstring GetButtonLabel(
-      ConfirmInfoBarDelegate::InfoBarButton button) const {
-    if (button == BUTTON_OK)
-      return l10n_util::GetString(IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON);
-    return ConfirmInfoBarDelegate::GetButtonLabel(button);
-  }
-
-  virtual bool Accept() {
-    extension_host_->RecoverCrashedExtension();
-    return true;
-  }
-
- private:
-  ExtensionHost* extension_host_;
-
-  DISALLOW_COPY_AND_ASSIGN(CrashedExtensionInfobarDelegate);
-};
-
-}  // namespace
-
-
 // static
 bool ExtensionHost::enable_dom_automation_ = false;
 
@@ -162,21 +110,6 @@ void ExtensionHost::Observe(NotificationType type,
   NavigateToURL(url_);
 }
 
-void ExtensionHost::RecoverCrashedExtension() {
-  DCHECK(!IsRenderViewLive());
-#if defined(TOOLKIT_VIEWS)
-  if (view_.get()) {
-    // The view should call us back to CreateRenderView, which is the place
-    // where we create the render process and fire notification.
-    view_->RecoverCrashedExtension();
-  } else {
-    CreateRenderView(NULL);
-  }
-#else
-  CreateRenderView(NULL);
-#endif
-}
-
 void ExtensionHost::UpdatePreferredWidth(int pref_width) {
 #if defined(TOOLKIT_VIEWS) || defined(OS_LINUX)
   if (view_.get())
@@ -186,17 +119,9 @@ void ExtensionHost::UpdatePreferredWidth(int pref_width) {
 
 void ExtensionHost::RenderViewGone(RenderViewHost* render_view_host) {
   DCHECK_EQ(render_view_host_, render_view_host);
-  Browser* browser = GetBrowser();
-  if (browser) {
-    TabContents* current_tab = browser->GetSelectedTabContents();
-    if (current_tab) {
-      current_tab->AddInfoBar(
-          new CrashedExtensionInfobarDelegate(current_tab, this));
-    }
-  }
   NotificationService::current()->Notify(
       NotificationType::EXTENSION_PROCESS_CRASHED,
-      Source<Profile>(profile_),
+      Source<ExtensionsService>(profile_->GetExtensionsService()),
       Details<ExtensionHost>(this));
 }
 
diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h
index 1a9ffed..8b8dbb8 100644
--- a/chrome/browser/extensions/extension_host.h
+++ b/chrome/browser/extensions/extension_host.h
@@ -76,10 +76,6 @@ class ExtensionHost : public RenderViewHostDelegate,
   // Sets |url_| and navigates |render_view_host_|.
   void NavigateToURL(const GURL& url);
 
-  // Restarts extension's renderer process. Can be called only if the renderer
-  // process crashed.
-  void RecoverCrashedExtension();
-
   // RenderViewHostDelegate implementation.
   virtual RenderViewHostDelegate::View* GetViewDelegate();
   virtual const GURL& GetURL() const { return url_; }
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 7b4d061..cd97918 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -232,6 +232,18 @@ void ExtensionPrefs::OnExtensionUninstalled(const Extension* extension,
   }
 }
 
+FilePath ExtensionPrefs::GetExtensionPath(const std::string& extension_id) {
+  const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref);
+  if (!dict || dict->GetSize() == 0)
+    return FilePath();
+
+  std::wstring path;
+  if (!dict->GetString(ASCIIToWide(extension_id) + L"." + kPrefPath, &path))
+    return FilePath();
+
+  return install_directory_.Append(FilePath::FromWStringHack(path));
+}
+
 bool ExtensionPrefs::UpdateExtensionPref(const std::string& extension_id,
                                          const std::wstring& key,
                                          Value* data_value) {
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 277b9c3..a22601e 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -43,6 +43,9 @@ class ExtensionPrefs {
   void OnExtensionUninstalled(const Extension* extension,
                               bool external_uninstall);
 
+  // Returns extension path based on extension ID, or empty FilePath on error.
+  FilePath GetExtensionPath(const std::string& extension_id);
+
   // Returns base extensions install directory.
   const FilePath& install_directory() const { return install_directory_; }
 
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 1b83a61..7e4c87f 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -80,6 +80,9 @@ ExtensionsService::ExtensionsService(Profile* profile,
   }
 
   backend_ = new ExtensionsServiceBackend(install_directory_, frontend_loop);
+
+  registrar_.Add(this, NotificationType::EXTENSION_PROCESS_CRASHED,
+                 Source<ExtensionsService>(this));
 }
 
 ExtensionsService::~ExtensionsService() {
@@ -132,11 +135,13 @@ void ExtensionsService::UpdateExtension(const std::string& id,
 }
 
 void ExtensionsService::ReloadExtension(const std::string& extension_id) {
-  Extension* extension = GetExtensionById(extension_id);
-  FilePath extension_path = extension->path();
+  // Unload the extension if it's loaded.
+  if (GetExtensionById(extension_id))
+    UnloadExtension(extension_id);
 
-  UnloadExtension(extension_id);
-  LoadExtension(extension_path);
+  // At this point we have to reconstruct the path from prefs, because
+  // we have no information about this extension in memory.
+  LoadExtension(extension_prefs_->GetExtensionPath(extension_id));
 }
 
 void ExtensionsService::UninstallExtension(const std::string& extension_id,
@@ -387,6 +392,25 @@ void ExtensionsService::OnExternalExtensionFound(const std::string& id,
                       NULL);  // no client (silent install)
 }
 
+void ExtensionsService::Observe(NotificationType type,
+                                const NotificationSource& source,
+                                const NotificationDetails& details) {
+  switch (type.value) {
+    case NotificationType::EXTENSION_PROCESS_CRASHED: {
+        DCHECK_EQ(this, Source<ExtensionsService>(source).ptr());
+        ExtensionHost* host = Details<ExtensionHost>(details).ptr();
+
+        // Unload the entire extension to make sure its state is consistent
+        // (either fully operational, or fully unloaded, but not half-crashed).
+        UnloadExtension(host->extension()->id());
+      }
+      break;
+
+    default:
+      NOTREACHED();
+  }
+}
+
 
 // ExtensionsServicesBackend
 
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 640700e..1027c21 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -22,6 +22,7 @@
 #include "chrome/browser/extensions/external_extension_provider.h"
 #include "chrome/browser/extensions/sandboxed_extension_unpacker.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/notification_registrar.h"
 
 class Browser;
 class DictionaryValue;
@@ -50,6 +51,7 @@ class ExtensionUpdateService {
 // Manages installed and running Chromium extensions.
 class ExtensionsService
     : public ExtensionUpdateService,
+      public NotificationObserver,
       public base::RefCountedThreadSafe<ExtensionsService> {
  public:
 
@@ -194,6 +196,11 @@ class ExtensionsService
   // Whether the extension service is ready.
   bool is_ready() { return ready_; }
 
+  // NotificationObserver
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
  private:
   // The profile this ExtensionsService is part of.
   Profile* profile_;
@@ -222,6 +229,8 @@ class ExtensionsService
   // Is the service ready to go?
   bool ready_;
 
+  NotificationRegistrar registrar_;
+
   // Our extension updater, if updates are turned on.
   scoped_refptr<ExtensionUpdater> updater_;
 
diff --git a/chrome/browser/views/extensions/extension_view.cc b/chrome/browser/views/extensions/extension_view.cc
index 63f37db..7b39336 100644
--- a/chrome/browser/views/extensions/extension_view.cc
+++ b/chrome/browser/views/extensions/extension_view.cc
@@ -145,11 +145,6 @@ void ExtensionView::ViewHierarchyChanged(bool is_add,
     CreateWidgetHostView();
 }
 
-void ExtensionView::RecoverCrashedExtension() {
-  CleanUp();
-  CreateWidgetHostView();
-}
-
 void ExtensionView::HandleMouseEvent() {
   if (container_)
     container_->OnExtensionMouseEvent(this);
diff --git a/chrome/browser/views/extensions/extension_view.h b/chrome/browser/views/extensions/extension_view.h
index 75f7a3f..1e08f79 100644
--- a/chrome/browser/views/extensions/extension_view.h
+++ b/chrome/browser/views/extensions/extension_view.h
@@ -59,10 +59,6 @@ class ExtensionView : public views::NativeViewHost {
   virtual void ViewHierarchyChanged(bool is_add,
                                     views::View *parent, views::View *child);
 
-  // Call after extension process crash to re-initialize view, so that
-  // extension content can be rendered again.
-  void RecoverCrashedExtension();
-
  private:
   friend class ExtensionHost;
 
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 122c1e5..b98860f 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -976,6 +976,8 @@
         'browser/download/save_types.h',
         'browser/encoding_menu_controller.cc',
         'browser/encoding_menu_controller.h',
+        'browser/extensions/crashed_extension_infobar.cc',
+        'browser/extensions/crashed_extension_infobar.h',
         'browser/extensions/crx_installer.cc',
         'browser/extensions/crx_installer.h',
         'browser/extensions/extension_bookmarks_module.cc',
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index 2a04ffe..5938992 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -629,7 +629,7 @@ class NotificationType {
     EXTENSION_PROCESS_CREATED,
 
     // Sent when extension render process crashes. The details are
-    // an ExtensionHost*.
+    // an ExtensionHost* and the source is an ExtensionsService*.
     EXTENSION_PROCESS_CRASHED,
 
     // Sent when the contents or order of toolstrips in the shelf model change.