Making ShowExtensionInstallDialog a callback

chrome/browser/extensions/extension_install_dialog.h

Index: chrome/browser/extensions/extension_install_dialog.h
diff --git a/chrome/browser/extensions/extension_install_dialog.h b/chrome/browser/extensions/extension_install_dialog.h
index bc4e85248b09fd1d74ab1325ea958279ea9cd5d6..d7d73f2a1f56817f7caa68c0e7dd1a6489444726 100644
--- a/chrome/browser/extensions/extension_install_dialog.h
+++ b/chrome/browser/extensions/extension_install_dialog.h
@@ -11,20 +11,24 @@
 #include "base/string16.h"
 #include "chrome/browser/extensions/extension_install_prompt.h"
 
-namespace base {
-class DictionaryValue;
-}
-
-void ShowExtensionInstallDialog(gfx::NativeWindow parent,
-                                content::PageNavigator* navigator,
-                                ExtensionInstallPrompt::Delegate* delegate,
-                                const ExtensionInstallPrompt::Prompt& prompt);
-
-// The implementations of this function are platform-specific.
-void ShowExtensionInstallDialogImpl(
-    gfx::NativeWindow parent,
-    content::PageNavigator* navigator,
-    ExtensionInstallPrompt::Delegate* delegate,
-    const ExtensionInstallPrompt::Prompt& prompt);
+class ExtensionInstallDialog : public base::RefCounted<ExtensionInstallDialog> {

[Aaron Boodman, 2012/10/11 02:42:03]

RefCounted is a design smell. Why does this need to be refcounted?

[sail, 2012/10/11 02:46:58]

So if this isn't refcounted then it's not clear how this object would get
deleted.
If we assume it deletes itself in Show() then case will leak:
   prompt->ConfirmInstall(..., new ExtensionInstallDialog);

Prompt::ConfirmInstall(..., dialog) {
  if (skip) {
     // dialog is leaked here
  } else {
     dialog->Show();
  }
}

We could have have ExtensionInstallPrompt take owner ship then when should the
dialog be deleted?

+ public:
+  ExtensionInstallDialog();

[Aaron Boodman, 2012/10/11 02:42:03]

Make this protected.

+
+  virtual void ShowExtensionInstallDialog(
+      gfx::NativeWindow parent,
+      content::PageNavigator* navigator,
+      ExtensionInstallPrompt::Delegate* delegate,
+      const ExtensionInstallPrompt::Prompt& prompt) = 0;
+
+  // The implementations of this function are platform-specific.
+  static scoped_refptr<ExtensionInstallDialog> CreateDefaultImpl();
+
+ protected:
+  friend class base::RefCounted<ExtensionInstallDialog>;
+  virtual ~ExtensionInstallDialog();
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialog);
+};
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_DIALOG_H_