Fix bug where AppInfoDialog was outliving ExtensionSettingsHandler

chrome/browser/ui/webui/extensions/extension_settings_handler.cc

Index: chrome/browser/ui/webui/extensions/extension_settings_handler.cc
diff --git a/chrome/browser/ui/webui/extensions/extension_settings_handler.cc b/chrome/browser/ui/webui/extensions/extension_settings_handler.cc
index 5910bb776652e8a057b5c424ba7c20e3994b4c5c..67fcd36c89ee3fe368c63bfba2cb5b899d35012c 100644
--- a/chrome/browser/ui/webui/extensions/extension_settings_handler.cc
+++ b/chrome/browser/ui/webui/extensions/extension_settings_handler.cc
@@ -139,13 +139,14 @@ ExtensionPage::ExtensionPage(const GURL& url,
       generated_background_page(generated_background_page) {
 }
 
-// On Mac, the install prompt is not modal. This means that the user can
-// navigate while the dialog is up, causing the dialog handler to outlive the
-// ExtensionSettingsHandler. That's a problem because the dialog framework will
-// try to contact us back once the dialog is closed, which causes a crash.
-// This class is designed to broker the message between the two objects, while
-// managing its own lifetime so that it can outlive the ExtensionSettingsHandler
-// and (when doing so) gracefully ignore the message from the dialog.
+// On Mac (and Linux with Unity), the install prompt is not modal. This means

[not at google - send to devlin, 2014/11/03 17:37:49]

How about just "The install prompt is not necessarily modal (e.g. Mac, Linux
Unity)".

Cover your bases for the future.

[sashab, 2014/11/04 01:32:20]

Nice suggestion; done.

+// that the user can navigate while the dialog is up, causing the dialog handler
+// to outlive the ExtensionSettingsHandler. That's a problem because the dialog
+// framework will try to contact us back once the dialog is closed, which causes
+// a crash. This class is designed to broker the message between the two
+// objects, while managing its own lifetime so that it can outlive the
+// ExtensionSettingsHandler and (when doing so) gracefully ignore the message
+// from the dialog.
 class BrokerDelegate : public ExtensionInstallPrompt::Delegate {
  public:
   explicit BrokerDelegate(
@@ -165,6 +166,12 @@ class BrokerDelegate : public ExtensionInstallPrompt::Delegate {
     delete this;
   };
 
+  void AppInfoDialogClosed() {
+    if (delegate_)
+      delegate_->AppInfoDialogClosed();
+    delete this;
+  }
+
  private:
   base::WeakPtr<ExtensionSettingsHandler> delegate_;
 
@@ -1208,9 +1215,11 @@ void ExtensionSettingsHandler::HandlePermissionsMessage(
 
   if (!extension_id_prompting_.empty())
     return;  // Only one prompt at a time.
-
   extension_id_prompting_ = extension->id();
 
+  // The BrokerDelegate manages its own lifetime.
+  BrokerDelegate* broker_delegate = new BrokerDelegate(AsWeakPtr());
+
   // Show the new-style extensions dialog when the flag is set. The flag cannot
   // be set on Mac platforms.
   if (ShouldDisplayExtensionInfoDialog()) {
@@ -1221,13 +1230,13 @@ void ExtensionSettingsHandler::HandlePermissionsMessage(
     // Display the dialog at a size similar to the app list.
     const int kAppInfoDialogWidth = 380;
     const int kAppInfoDialogHeight = 490;
+
     ShowAppInfoInNativeDialog(
         web_contents()->GetTopLevelNativeWindow(),
         gfx::Size(kAppInfoDialogWidth, kAppInfoDialogHeight),
-        Profile::FromWebUI(web_ui()),
-        extension,
-        base::Bind(&ExtensionSettingsHandler::AppInfoDialogClosed,
-                   base::Unretained(this)));
+        Profile::FromWebUI(web_ui()), extension,
+        base::Bind(&BrokerDelegate::AppInfoDialogClosed,
+                   base::Unretained(broker_delegate)));
   } else {
     prompt_.reset(new ExtensionInstallPrompt(web_contents()));
     std::vector<base::FilePath> retained_file_paths;
@@ -1248,10 +1257,7 @@ void ExtensionSettingsHandler::HandlePermissionsMessage(
               ->GetPermissionMessageStrings(extension_id_prompting_);
     }
 
-    // The BrokerDelegate manages its own lifetime.
-    prompt_->ReviewPermissions(new BrokerDelegate(AsWeakPtr()),
-                               extension,
-                               retained_file_paths,
+    prompt_->ReviewPermissions(broker_delegate, extension, retained_file_paths,
                                retained_device_messages);
   }
 }