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

chrome/browser/extensions/unpacked_installer.cc

 // Copyright (c) 2013 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.
 
 #include "chrome/browser/extensions/unpacked_installer.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_util.h"
 #include "base/strings/string_util.h"
@@ skipping 29 matching lines @@
 
 const char kUnpackedExtensionsBlacklistedError[] =
     "Loading of unpacked extensions is disabled by the administrator.";
 
 const char kImportMinVersionNewer[] =
     "'import' version requested is newer than what is installed.";
 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.
 
- private:
+  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(
     const Extension* extension,
     Profile* profile,
     const base::Closure& callback)
     : extension_(extension), callback_(callback) {
   scoped_ptr<extensions::ExtensionInstallUI> ui(
       extensions::CreateExtensionInstallUI(profile));
   install_ui_.reset(new ExtensionInstallPrompt(
       profile, ui->GetDefaultInstallDialogParent()));
 }
 
 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();
+void SimpleExtensionLoadPrompt::OnInstallPromptDone(
+    ExtensionInstallPrompt::Result result) {
+  if (result == ExtensionInstallPrompt::Result::ACCEPTED)
+    callback_.Run();
   delete this;
 }
 
-void SimpleExtensionLoadPrompt::InstallUIAbort(bool user_initiated) {
-  delete this;
-}
-
 }  // namespace
 
 namespace extensions {
 
 // static
 scoped_refptr<UnpackedInstaller> UnpackedInstaller::Create(
     ExtensionService* extension_service) {
   DCHECK(extension_service);
   return scoped_refptr<UnpackedInstaller>(
       new UnpackedInstaller(extension_service));
@@ skipping 252 matching lines @@
   service_weak_->OnExtensionInstalled(
       extension(), syncer::StringOrdinal(), kInstallFlagInstallImmediately);
 
   if (!callback_.is_null()) {
     callback_.Run(extension(), extension_path_, std::string());
     callback_.Reset();
   }
 }
 
 }  // namespace extensions