cbui: migrate ProfileSigninConfirmationDialogViews to set_button_label

This dialog had a two-phase construction to supply some state that
logically was part of construction but was not yet available at
construction time. This change collapses construction of this dialog
into a single logical phase. Specifically:

1) ProfileSigninConfirmationDialogViews::ShowDialog now defers
   construction of the dialog until after
   ui::CheckShouldPromptForNewProfile
1) CheckShouldPromptForNewProfile now takes a base::OnceCallback instead
   of a base::Callback, to allow ownership of the
   ProfileSigninConfirmationDelegate to be passed through it
3) ProfileSigninConfirmationDialogViews::Show now both constructs and
   shows the ProfileSigninConfirmationDialogViews, passing in the
   prompt_for_new_profile member
4) The prompt_for_new_profile member is now made const
5) That makes the return values of GetDialogButtonLabel also constant
   after construction, which opens up migration to set_button_label

Bug: 1011446
Change-Id: I975d425546db6e2d231f0003d38f0f3fac64ad88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869489Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707480}

chrome/browser/ui/sync/profile_signin_confirmation_helper.cc

@@ -85,20 +85,21 @@ bool HasSyncedExtensions(Profile* profile) {
 
 void CheckShouldPromptForNewProfile(
     Profile* profile,
-    const base::Callback<void(bool)>& return_result) {
+    base::OnceCallback<void(bool)> return_result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (HasBeenShutdown(profile) ||
       HasBookmarks(profile) ||
       HasSyncedExtensions(profile)) {
-    return_result.Run(true);
+    std::move(return_result).Run(true);
     return;
   }
   history::HistoryService* service =
       HistoryServiceFactory::GetForProfileWithoutCreating(profile);
   // Fire asynchronous queries for profile data.
   browser_sync::SigninConfirmationHelper* helper =
-      new browser_sync::SigninConfirmationHelper(service, return_result);
+      new browser_sync::SigninConfirmationHelper(service,
+                                                 std::move(return_result));
   helper->CheckHasHistory(kHistoryEntriesBeforeNewProfilePrompt);
   helper->CheckHasTypedURLs();
 }

chrome/browser/ui/sync/profile_signin_confirmation_helper.h

@@ -29,9 +29,8 @@ bool HasSyncedExtensions(Profile* profile);
 
 // Determines whether the user should be prompted to create a new
 // profile before signin.
-void CheckShouldPromptForNewProfile(
-    Profile* profile,
-    const base::Callback<void(bool)>& cb);
+void CheckShouldPromptForNewProfile(Profile* profile,
+                                    base::OnceCallback<void(bool)> cb);
 
 // Handles user input from confirmation dialog.
 class ProfileSigninConfirmationDelegate {

chrome/browser/ui/sync/profile_signin_confirmation_helper_unittest.cc

@@ -62,12 +62,13 @@ void GetValueAndQuit(T* result, const base::Closure& quit, T actual) {
   quit.Run();
 }
 
-template<typename T>
+template <typename T>
 T GetCallbackResult(
-    const base::Callback<void(const base::Callback<void(T)>&)>& callback) {
+    const base::Callback<void(base::OnceCallback<void(T)>)>& callback) {
   T result = false;
   base::RunLoop loop;
-  callback.Run(base::Bind(&GetValueAndQuit<T>, &result, loop.QuitClosure()));
+  callback.Run(
+      base::BindOnce(&GetValueAndQuit<T>, &result, loop.QuitClosure()));
   loop.Run();
   return result;
 }

chrome/browser/ui/views/sync/profile_signin_confirmation_dialog_views.cc

@@ -47,18 +47,46 @@
 ProfileSigninConfirmationDialogViews::ProfileSigninConfirmationDialogViews(
     Browser* browser,
     const std::string& username,
-    std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate)
+    std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate,
+    bool prompt_for_new_profile)
     : browser_(browser),
       username_(username),
       delegate_(std::move(delegate)),
-      prompt_for_new_profile_(true) {
+      prompt_for_new_profile_(prompt_for_new_profile) {
   DialogDelegate::set_default_button(ui::DIALOG_BUTTON_NONE);
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_OK,
+      l10n_util::GetStringUTF16(prompt_for_new_profile_
+                                    ? IDS_ENTERPRISE_SIGNIN_CREATE_NEW_PROFILE
+                                    : IDS_ENTERPRISE_SIGNIN_CONTINUE));
+  DialogDelegate::set_button_label(
+      ui::DIALOG_BUTTON_CANCEL,
+      l10n_util::GetStringUTF16(IDS_ENTERPRISE_SIGNIN_CANCEL));
+
+  if (prompt_for_new_profile) {
+    DialogDelegate::SetExtraView(views::MdTextButton::CreateSecondaryUiButton(
+        this, l10n_util::GetStringUTF16(IDS_ENTERPRISE_SIGNIN_CONTINUE)));
+  }
+
   chrome::RecordDialogCreation(
       chrome::DialogIdentifier::PROFILE_SIGNIN_CONFIRMATION);
 }
 
 ProfileSigninConfirmationDialogViews::~ProfileSigninConfirmationDialogViews() {}
 
+// static
+void ProfileSigninConfirmationDialogViews::Show(
+    Browser* browser,
+    const std::string& username,
+    std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate,
+    bool prompt) {
+  auto dialog = std::make_unique<ProfileSigninConfirmationDialogViews>(
+      browser, username, std::move(delegate), prompt);
+  constrained_window::CreateBrowserModalDialogViews(
+      dialog.release(), browser->window()->GetNativeWindow())
+      ->Show();
+}
+
 // static
 void ProfileSigninConfirmationDialogViews::ShowDialog(
     Browser* browser,
@@ -74,25 +102,12 @@ void ProfileSigninConfirmationDialogViews::ShowDialog(
   // is fixed.
   ProfileMenuView::Hide();
 
-  ProfileSigninConfirmationDialogViews* dialog =
-      new ProfileSigninConfirmationDialogViews(browser, username,
-                                               std::move(delegate));
+  // Checking whether to show the prompt is sometimes asynchronous. Defer
+  // constructing the dialog (in ::Show) until that check completes.
   ui::CheckShouldPromptForNewProfile(
       profile,
-      // This callback is guaranteed to be invoked, and once it is, the dialog
-      // owns itself.
-      base::Bind(&ProfileSigninConfirmationDialogViews::Show,
-                 base::Unretained(dialog)));
-}
-
-void ProfileSigninConfirmationDialogViews::Show(bool prompt_for_new_profile) {
-  prompt_for_new_profile_ = prompt_for_new_profile;
-  if (prompt_for_new_profile) {
-    DialogDelegate::SetExtraView(views::MdTextButton::CreateSecondaryUiButton(
-        this, l10n_util::GetStringUTF16(IDS_ENTERPRISE_SIGNIN_CONTINUE)));
-  }
-  constrained_window::CreateBrowserModalDialogViews(
-      this, browser_->window()->GetNativeWindow())->Show();
+      base::BindOnce(&ProfileSigninConfirmationDialogViews::Show,
+                     base::Unretained(browser), username, std::move(delegate)));
 }
 
 base::string16 ProfileSigninConfirmationDialogViews::GetWindowTitle() const {
@@ -100,19 +115,6 @@ base::string16 ProfileSigninConfirmationDialogViews::GetWindowTitle() const {
       IDS_ENTERPRISE_SIGNIN_TITLE);
 }
 
-base::string16 ProfileSigninConfirmationDialogViews::GetDialogButtonLabel(
-    ui::DialogButton button) const {
-  if (button == ui::DIALOG_BUTTON_OK) {
-    // If we're giving the option to create a new profile, then OK is
-    // "Create new profile".  Otherwise it is "Continue signin".
-    return l10n_util::GetStringUTF16(
-        prompt_for_new_profile_ ?
-            IDS_ENTERPRISE_SIGNIN_CREATE_NEW_PROFILE :
-            IDS_ENTERPRISE_SIGNIN_CONTINUE);
-  }
-  return l10n_util::GetStringUTF16(IDS_ENTERPRISE_SIGNIN_CANCEL);
-}
-
 bool ProfileSigninConfirmationDialogViews::Accept() {
   if (delegate_) {
     if (prompt_for_new_profile_)

chrome/browser/ui/views/sync/profile_signin_confirmation_dialog_views.h

@@ -31,16 +31,22 @@ class ProfileSigninConfirmationDialogViews : public views::DialogDelegateView,
       const std::string& username,
       std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate);
 
- private:
   ProfileSigninConfirmationDialogViews(
       Browser* browser,
       const std::string& username,
-      std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate);
+      std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate,
+      bool prompt_for_new_profile);
   ~ProfileSigninConfirmationDialogViews() override;
 
+ private:
+  static void Show(
+      Browser* browser,
+      const std::string& username,
+      std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate,
+      bool prompt_for_new_profile);
+
   // views::DialogDelegateView:
   base::string16 GetWindowTitle() const override;
-  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   bool Accept() override;
   bool Cancel() override;
   ui::ModalType GetModalType() const override;
@@ -58,11 +64,6 @@ class ProfileSigninConfirmationDialogViews : public views::DialogDelegateView,
   // views::ButtonListener:
   void ButtonPressed(views::Button*, const ui::Event& event) override;
 
-  // Shows the dialog and releases ownership of this object. It will
-  // delete itself when the dialog is closed. If |prompt_for_new_profile|
-  // is true, the dialog will offer to create a new profile before signin.
-  void Show(bool prompt_for_new_profile);
-
   // Weak ptr to parent view.
   Browser* const browser_;
 
@@ -73,7 +74,7 @@ class ProfileSigninConfirmationDialogViews : public views::DialogDelegateView,
   std::unique_ptr<ui::ProfileSigninConfirmationDelegate> delegate_;
 
   // Whether the user should be prompted to create a new profile.
-  bool prompt_for_new_profile_;
+  const bool prompt_for_new_profile_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileSigninConfirmationDialogViews);
 };