[Extensions] Migrate ExtensionInstallPrompt::Delegate to be a callback

chrome/browser/extensions/unpacked_installer.cc

Index: chrome/browser/extensions/unpacked_installer.cc
diff --git a/chrome/browser/extensions/unpacked_installer.cc b/chrome/browser/extensions/unpacked_installer.cc
index 1849630d04036d21d602d303769218472e917786..035aecff75f90f3725cf63dc15b649098c111ff5 100644
--- a/chrome/browser/extensions/unpacked_installer.cc
+++ b/chrome/browser/extensions/unpacked_installer.cc
@@ -47,23 +47,24 @@ const char kImportMissing[] = "'import' extension is not installed.";
 const char kImportNotSharedModule[] = "'import' is not a shared module.";
 
 // Manages an ExtensionInstallPrompt for a particular extension.
-class SimpleExtensionLoadPrompt : public ExtensionInstallPrompt::Delegate {
+class SimpleExtensionLoadPrompt {
  public:
   SimpleExtensionLoadPrompt(const Extension* extension,
                             Profile* profile,
                             const base::Closure& callback);
-  ~SimpleExtensionLoadPrompt() override;
 
   void ShowPrompt();
 
-  // ExtensionInstallUI::Delegate
-  void InstallUIProceed() override;
-  void InstallUIAbort(bool user_initiated) override;
-
  private:
+  ~SimpleExtensionLoadPrompt();  // Manages its own lifetime.
+
+  void OnInstallPromptDone(ExtensionInstallPrompt::Result result);
+
   scoped_ptr<ExtensionInstallPrompt> install_ui_;
   scoped_refptr<const Extension> extension_;
   base::Closure callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(SimpleExtensionLoadPrompt);

[Avi (use Gerrit), 2015/12/29 03:10:45]

base/macros.h

[Devlin, 2016/01/04 22:59:23]

Should be fine, since macros.h is included in unpacked_installer.h (and we don't
need to iwyu when it's in the file's associated header).

 };
 
 SimpleExtensionLoadPrompt::SimpleExtensionLoadPrompt(
@@ -81,27 +82,30 @@ SimpleExtensionLoadPrompt::~SimpleExtensionLoadPrompt() {
 }
 
 void SimpleExtensionLoadPrompt::ShowPrompt() {
+  // TODO(devlin): The AutoConfirm values should work from the
+  // ExtensionInstallPrompt code, so these should be unnecessary.

[asargent_no_longer_on_chrome, 2015/12/31 00:45:56]

Do you mean we don't need to bother looking at the value of GetAutoConfirmValue,
but rather can just rely on the call to ShowDialog to do the right thing?

That sounds right to me - did you try doing it? 

[Devlin, 2016/01/04 22:59:23]

Haven't tried it yet - tried to keep this small and targeted (as odd as that
sounds from a N-hundred line patch).  I'll do it in a followup, just in case one
or the other breaks something. :)

[asargent_no_longer_on_chrome, 2016/01/05 18:58:54]

Ok

   switch (extensions::ScopedTestDialogAutoConfirm::GetAutoConfirmValue()) {
     case extensions::ScopedTestDialogAutoConfirm::NONE:
+      // Unretained() is safe because this object manages its own lifetime.
       install_ui_->ShowDialog(
-          this, extension_.get(), nullptr,
+          base::Bind(&SimpleExtensionLoadPrompt::OnInstallPromptDone,
+                     base::Unretained(this)),
+          extension_.get(), nullptr,
           ExtensionInstallPrompt::GetDefaultShowDialogCallback());
       break;
     case extensions::ScopedTestDialogAutoConfirm::ACCEPT:
-      InstallUIProceed();
+      OnInstallPromptDone(ExtensionInstallPrompt::Result::ACCEPTED);
       break;
     case extensions::ScopedTestDialogAutoConfirm::CANCEL:
-      InstallUIAbort(false);
+      OnInstallPromptDone(ExtensionInstallPrompt::Result::ABORTED);
       break;
   }
 }
 
-void SimpleExtensionLoadPrompt::InstallUIProceed() {
-  callback_.Run();
-  delete this;
-}
-
-void SimpleExtensionLoadPrompt::InstallUIAbort(bool user_initiated) {
+void SimpleExtensionLoadPrompt::OnInstallPromptDone(
+    ExtensionInstallPrompt::Result result) {
+  if (result == ExtensionInstallPrompt::Result::ACCEPTED)
+    callback_.Run();
   delete this;
 }