Change when callback is set for ParentPermissionDialog

This is needed due to a specific requirements for integration with the
extensions Management API, which needs to first create the dialog, then
create the callback that takes ownership of the dialog instance, then
show the dialog.

That order isn't possible when the dialog's callback is set in
its constructor.

Also remove unused ShowParentPermissionDialog...() fns from
SupervisedUserService since we need to create the dialogs directly to
achieve the pattern described above.

Bug:957832

Change-Id: I9852a4ecbe12babfadd3a806513713d2085b3d21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2088952
Auto-Submit: Dan S <danan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Dan S <danan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747827}

chrome/browser/supervised_user/supervised_user_service.cc

@@ -420,42 +420,6 @@ bool SupervisedUserService::IsExtensionAllowed(
   return GetExtensionState(extension) ==
          SupervisedUserService::ExtensionState::ALLOWED;
 }
-
-std::unique_ptr<ParentPermissionDialog>
-SupervisedUserService::ShowParentPermissionDialog(
-    const base::string16& message,
-    content::WebContents* contents,
-    const SkBitmap& icon,
-    ParentPermissionDialog::DoneCallback callback,
-    bool reprompt_after_incorrect_credential) {
-  auto parent_permission_dialog =
-      std::make_unique<ParentPermissionDialog>(profile_, std::move(callback));
-
-  parent_permission_dialog->set_reprompt_after_incorrect_credential(
-      reprompt_after_incorrect_credential);
-  parent_permission_dialog->ShowPrompt(contents, message, icon);
-
-  return parent_permission_dialog;
-}
-
-std::unique_ptr<ParentPermissionDialog>
-SupervisedUserService::ShowParentPermissionDialogForExtension(
-    const extensions::Extension* extension,
-    content::WebContents* contents,
-    const SkBitmap& icon,
-    ParentPermissionDialog::DoneCallback callback,
-    bool reprompt_after_incorrect_credential) {
-  auto parent_permission_dialog =
-      std::make_unique<ParentPermissionDialog>(profile_, std::move(callback));
-
-  parent_permission_dialog->set_reprompt_after_incorrect_credential(
-      reprompt_after_incorrect_credential);
-  parent_permission_dialog->ShowPromptForExtensionInstallation(contents,
-                                                               extension, icon);
-
-  return parent_permission_dialog;
-}
-
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
 void SupervisedUserService::SetActive(bool active) {

chrome/browser/supervised_user/supervised_user_service.h

@@ -53,11 +53,6 @@ class Version;
 }
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-class SkBitmap;
-namespace content {
-class WebContents;
-}
-
 namespace extensions {
 class Extension;
 }
@@ -224,25 +219,6 @@ class SupervisedUserService : public KeyedService,
 
   bool IsExtensionAllowed(const extensions::Extension& extension) const;
 
-  // Show a parent permission dialog displaying |message| and call |callback|
-  // when it completes.
-  std::unique_ptr<ParentPermissionDialog> ShowParentPermissionDialog(
-      const base::string16& message,
-      content::WebContents* contents,
-      const SkBitmap& icon,
-      ParentPermissionDialog::DoneCallback callback,
-      bool reprompt_after_incorrect_credential);
-
-  // Show a parent permission dialog for |extension| and call |callback| when it
-  // completes.
-  std::unique_ptr<ParentPermissionDialog>
-  ShowParentPermissionDialogForExtension(
-      const extensions::Extension* extension,
-      content::WebContents* contents,
-      const SkBitmap& icon,
-      ParentPermissionDialog::DoneCallback callback,
-      bool reprompt_after_incorrect_credential);
-
   void MarkExtensionMatureForTesting(const std::string& extension_id,
                                      bool maturity_rating);
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)

chrome/browser/ui/supervised_user/parent_permission_dialog.cc

@@ -39,10 +39,8 @@ const gfx::ImageSkia& GetDefaultIconBitmapForMaxScaleFactor(bool is_app) {
 signin::IdentityManager* test_identity_manager = nullptr;
 }  // namespace
 
-ParentPermissionDialog::ParentPermissionDialog(Profile* profile,
-                                               DoneCallback callback)
-    : profile_(profile), callback_(std::move(callback)) {
-  DCHECK(callback_);
+ParentPermissionDialog::ParentPermissionDialog(Profile* profile)
+    : profile_(profile) {
   DCHECK(profile_);
   DCHECK(profile_->IsChild());
 }
@@ -55,7 +53,10 @@ ParentPermissionDialog::~ParentPermissionDialog() {
 
 void ParentPermissionDialog::ShowPrompt(content::WebContents* web_contents,
                                         const base::string16& message,
-                                        const SkBitmap& icon) {
+                                        const SkBitmap& icon,
+                                        DoneCallback callback) {
+  callback_ = std::move(callback);
+  DCHECK(callback_);
   DCHECK(!web_contents_);
   web_contents_ = web_contents;
   message_ = message;
@@ -72,7 +73,10 @@ void ParentPermissionDialog::ShowPrompt(content::WebContents* web_contents,
 void ParentPermissionDialog::ShowPromptForExtensionInstallation(
     content::WebContents* web_contents,
     const extensions::Extension* extension,
-    const SkBitmap& fallback_icon) {
+    const SkBitmap& fallback_icon,
+    DoneCallback callback) {
+  callback_ = std::move(callback);
+  DCHECK(callback_);
   DCHECK(!web_contents_);
   web_contents_ = web_contents;
   extension_ = extension;

chrome/browser/ui/supervised_user/parent_permission_dialog.h

@@ -76,7 +76,7 @@ class ParentPermissionDialog : public GaiaAuthConsumer {
 
   using DoneCallback = base::OnceCallback<void(Result result)>;
 
-  ParentPermissionDialog(Profile* profile, DoneCallback callback);
+  explicit ParentPermissionDialog(Profile* profile);
 
   ParentPermissionDialog(const ParentPermissionDialog&) = delete;
   ParentPermissionDialog& operator=(const ParentPermissionDialog&) = delete;
@@ -88,7 +88,8 @@ class ParentPermissionDialog : public GaiaAuthConsumer {
   // |icon| specifies the icon to be displayed. It can be empty.
   void ShowPrompt(content::WebContents* web_contents,
                   const base::string16& message,
-                  const SkBitmap& icon);
+                  const SkBitmap& icon,
+                  DoneCallback callback);
 
   // Shows the Parent Permission Dialog for the specified extension
   // installation. The dialog's message will be generated from the extension
@@ -99,7 +100,8 @@ class ParentPermissionDialog : public GaiaAuthConsumer {
   void ShowPromptForExtensionInstallation(
       content::WebContents* web_contents,
       const extensions::Extension* extension,
-      const SkBitmap& fallback_icon);
+      const SkBitmap& fallback_icon,
+      DoneCallback callback);
 
   // Sets whether the prompt is shown again automatically after an
   // incorrect credential.  This defaults to true, and is only disabled for

chrome/browser/ui/supervised_user/parent_permission_dialog_browsertest.cc

@@ -87,18 +87,19 @@ class ParentPermissionDialogBrowserTest
   void ShowPrompt() {
     base::RunLoop run_loop;
 
-    parent_permission_dialog_ = std::make_unique<ParentPermissionDialog>(
-        browser()->profile(),
-        base::BindOnce(
-            &ParentPermissionDialogBrowserTest::OnParentPermissionDialogDone,
-            base::Unretained(this), run_loop.QuitClosure()));
+    parent_permission_dialog_ =
+        std::make_unique<ParentPermissionDialog>(browser()->profile());
+
+    ParentPermissionDialog::DoneCallback callback = base::BindOnce(
+        &ParentPermissionDialogBrowserTest::OnParentPermissionDialogDone,
+        base::Unretained(this), run_loop.QuitClosure());
 
     parent_permission_dialog_->set_reprompt_after_incorrect_credential(false);
     SkBitmap icon =
         *gfx::Image(extensions::util::GetDefaultExtensionIcon()).ToSkBitmap();
     parent_permission_dialog_->ShowPrompt(
         browser()->tab_strip_model()->GetActiveWebContents(),
-        base::UTF8ToUTF16("Test Prompt Message"), icon);
+        base::UTF8ToUTF16("Test Prompt Message"), icon, std::move(callback));
     run_loop.Run();
   }
 
@@ -106,18 +107,19 @@ class ParentPermissionDialogBrowserTest
       scoped_refptr<const extensions::Extension> extension) {
     base::RunLoop run_loop;
 
-    parent_permission_dialog_ = std::make_unique<ParentPermissionDialog>(
-        browser()->profile(),
-        base::BindOnce(
-            &ParentPermissionDialogBrowserTest::OnParentPermissionDialogDone,
-            base::Unretained(this), run_loop.QuitClosure()));
+    parent_permission_dialog_ =
+        std::make_unique<ParentPermissionDialog>(browser()->profile());
+
+    ParentPermissionDialog::DoneCallback callback = base::BindOnce(
+        &ParentPermissionDialogBrowserTest::OnParentPermissionDialogDone,
+        base::Unretained(this), run_loop.QuitClosure());
 
     parent_permission_dialog_->set_reprompt_after_incorrect_credential(false);
     SkBitmap icon =
         *gfx::Image(extensions::util::GetDefaultExtensionIcon()).ToSkBitmap();
     parent_permission_dialog_->ShowPromptForExtensionInstallation(
         browser()->tab_strip_model()->GetActiveWebContents(), extension.get(),
-        icon);
+        icon, std::move(callback));
     run_loop.Run();
   }