Making ShowExtensionInstallDialog a callback

chrome/browser/extensions/extension_install_dialog.h

 // Copyright (c) 2012 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_EXTENSION_INSTALL_DIALOG_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_DIALOG_H_
 
 #include <vector>
 
 #include "base/memory/ref_counted.h"
 #include "base/string16.h"
 #include "chrome/browser/extensions/extension_install_prompt.h"
 
-namespace base {
-class DictionaryValue;
-}
+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.

 
-void ShowExtensionInstallDialog(gfx::NativeWindow parent,
-                                content::PageNavigator* navigator,
-                                ExtensionInstallPrompt::Delegate* delegate,
-                                const ExtensionInstallPrompt::Prompt& prompt);
+  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.
-void ShowExtensionInstallDialogImpl(
-    gfx::NativeWindow parent,
-    content::PageNavigator* navigator,
-    ExtensionInstallPrompt::Delegate* delegate,
-    const ExtensionInstallPrompt::Prompt& prompt);
+  // 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_