Handle termination/crashes of an extension hosted in the ExtensionDialog.

chrome/browser/ui/views/select_file_dialog_extension.cc

Index: chrome/browser/ui/views/select_file_dialog_extension.cc
diff --git a/chrome/browser/ui/views/select_file_dialog_extension.cc b/chrome/browser/ui/views/select_file_dialog_extension.cc
index a4f923a2a1d35e09a4e82e4a66000668bb5bb8ec..fe394b3c49f03d71ab4c771f010daba95a5658b0 100644
--- a/chrome/browser/ui/views/select_file_dialog_extension.cc
+++ b/chrome/browser/ui/views/select_file_dialog_extension.cc
@@ -4,11 +4,15 @@
 
 #include "chrome/browser/ui/views/select_file_dialog_extension.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
+#include "base/message_loop.h"
 #include "chrome/browser/extensions/extension_file_browser_private_api.h"
 #include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/file_manager_util.h"
 #include "chrome/browser/sessions/restore_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
@@ -92,6 +96,7 @@ SelectFileDialogExtension::SelectFileDialogExtension(Listener* listener)
       has_multiple_file_type_choices_(false),
       tab_id_(0),
       owner_window_(0),
+      owner_browser_(0),
       selection_type_(CANCEL),
       selection_index_(0),
       params_(NULL) {
@@ -115,6 +120,7 @@ void SelectFileDialogExtension::ListenerDestroyed() {
 
 void SelectFileDialogExtension::ExtensionDialogClosing(
     ExtensionDialog* dialog) {
+  owner_browser_ = NULL;
   owner_window_ = NULL;
   // Release our reference to the dialog to allow it to close.
   extension_dialog_ = NULL;
@@ -123,6 +129,29 @@ void SelectFileDialogExtension::ExtensionDialogClosing(
   NotifyListener();
 }
 
+void SelectFileDialogExtension::ExtensionDialogTerminated(
+    ExtensionDialog* dialog) {
+  LOG(ERROR) << "Select File Dialog Extension crashed.";
+
+  // The extension would have been unloaded because of the termination,

[James Cook, 2012/02/08 05:32:31]

I'm not particularly knowledgeable about our extension mechanisms, but could we
just leave the extension unloaded, then reload it the next time the user tries
to open the file manager or a file picker?

It feels odd to me to do this in a posted task.

[rkc, 2012/02/09 00:24:22]

At this time there is no method of knowing what 'state' an extension is in other
than if it's enabled.

Adding functionality to check if an extension is in a terminated state or not is
a possibility but I'll defer a decision on this till Aaron is done investigating
the possibility of not needing to reload crashed extensions at all.


On 2012/02/08 05:32:31, James Cook (Chromium) wrote:

[Yoyo Zhou, 2012/02/09 22:18:03]

Likewise, I'm not sure why this posted task is necessary.

On 2012/02/09 00:24:22, Rahul Chaturvedi wrote:
GetTerminatedExtension is supposed to do this (I'm not certain it works for
component extensions).

[rkc, 2012/02/10 00:35:42]

So we need to at least close the dialog window, which otherwise remains open
with no content in it :)

I tested this with removing the explicit reload code (the posted task) and the
extension doesn't reload. With this code removed, the crashed extension
notification does come up, clicking the "reload" option on which does reload the
extension and allows the select file dialog to come up again, but not otherwise.

+  // reload it.
+  std::string extension_id = dialog->host()->extension()->id();
+  // Reload the extension after a bit; the extension may not have been unloaded
+  // yet. We don't want to try to reload the extension only to have the Unload
+  // code execute after us and re-unload the extension.
+  if (owner_browser_) {
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &ExtensionService::ReloadExtension,
+            base::Unretained(owner_browser_->profile()->
+                             GetExtensionService()),
+            extension_id));
+  }
+
+  dialog->Close();
+}
+
 // static
 void SelectFileDialogExtension::OnFileSelected(
     int32 tab_id, const FilePath& path, int index) {
@@ -211,15 +240,15 @@ void SelectFileDialogExtension::SelectFileImpl(
     return;
   }
   // Extension background pages may not supply an owner_window.
-  Browser* owner_browser = (owner_window ?
+  owner_browser_ = (owner_window ?
       BrowserList::FindBrowserWithWindow(owner_window) :
       BrowserList::GetLastActive());
-  if (!owner_browser) {
+  if (!owner_browser_) {
     NOTREACHED() << "Can't find owning browser";
     return;
   }
 
-  TabContentsWrapper* tab = owner_browser->GetSelectedTabContentsWrapper();
+  TabContentsWrapper* tab = owner_browser_->GetSelectedTabContentsWrapper();
 
   // Check if we have another dialog opened in the tab. It's unlikely, but
   // possible.
@@ -231,7 +260,7 @@ void SelectFileDialogExtension::SelectFileImpl(
 
   FilePath virtual_path;
   if (!file_manager_util::ConvertFileToRelativeFileSystemPath(
-          owner_browser->profile(), default_path, &virtual_path)) {
+          owner_browser_->profile(), default_path, &virtual_path)) {
     virtual_path = default_path.BaseName();
   }
 
@@ -243,7 +272,7 @@ void SelectFileDialogExtension::SelectFileImpl(
       default_extension);
 
   ExtensionDialog* dialog = ExtensionDialog::Show(file_browser_url,
-      owner_browser, tab->web_contents(),
+      owner_browser_, tab->web_contents(),
       kFileManagerWidth, kFileManagerHeight,
 #if defined(USE_AURA)
       file_manager_util::GetTitleFromType(type),