Base DialogModel close-x on dialog modality

Removes explicit SetShowCloseButton calls. Leaves an ability to more
explicitly override the close-x (OverrideShowCloseButton) to avoid
changing existing dialogs as they are converted.

Making all existing dialogs respect dialog modality and removing this
override is something I'll try to negotiate separately, but this can at
least be correct by default.

Bug: 1106422
Change-Id: Ib8424e5c45fadee58aa07e5a2d26bd117054ebbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2465805
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816445}

chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc

@@ -176,7 +176,6 @@ void BookmarkBubbleView::ShowBubble(
 
   auto dialog_model =
       ui::DialogModel::Builder(std::move(bubble_delegate_unique))
-          .SetShowCloseButton(true)
           .SetTitle(l10n_util::GetStringUTF16(
               already_bookmarked ? IDS_BOOKMARK_BUBBLE_PAGE_BOOKMARK
                                  : IDS_BOOKMARK_BUBBLE_PAGE_BOOKMARKED))

chrome/browser/ui/views/outdated_upgrade_bubble_view.cc

@@ -115,7 +115,6 @@ void OutdatedUpgradeBubbleView::ShowBubble(views::View* anchor_view,
 
   auto dialog_model =
       ui::DialogModel::Builder()
-          .SetShowCloseButton(true)
           .SetTitle(l10n_util::GetStringUTF16(IDS_UPGRADE_BUBBLE_TITLE))
           .AddOkButton(
               base::BindOnce(&OnDialogAccepted, navigator, auto_update_enabled),

chrome/browser/ui/views/session_crashed_bubble_view.cc

@@ -226,7 +226,7 @@ views::BubbleDialogDelegateView* SessionCrashedBubbleView::ShowBubble(
   SessionCrashedBubbleDelegate* bubble_delegate = bubble_delegate_unique.get();
 
   ui::DialogModel::Builder dialog_builder(std::move(bubble_delegate_unique));
-  dialog_builder.SetShowCloseButton(true)
+  dialog_builder
       .SetTitle(l10n_util::GetStringUTF16(IDS_SESSION_CRASHED_BUBBLE_TITLE))
       .DisableCloseOnDeactivate()
       .SetIsAlertDialog()

ui/base/models/dialog_model.h

@@ -111,8 +111,13 @@ class COMPONENT_EXPORT(UI_BASE) DialogModel final {
     // with the DialogModel so uses of it will not result in use-after-frees.
     DialogModel* model() { return model_.get(); }
 
-    Builder& SetShowCloseButton(bool show_close_button) {
-      model_->show_close_button_ = show_close_button;
+    // Overrides the close-x use for the dialog. Should be avoided as the
+    // close-x is generally derived from dialog modality. Kept to allow
+    // conversion of dialogs that currently do not allow style.
+    // TODO(pbos): Propose UX updates to existing dialogs that require this,
+    // then remove OverrideShowCloseButton().
+    Builder& OverrideShowCloseButton(bool show_close_button) {
+      model_->override_show_close_button_ = show_close_button;
       return *this;
     }
 
@@ -267,8 +272,9 @@ class COMPONENT_EXPORT(UI_BASE) DialogModel final {
     host_ = host;
   }
 
-  bool show_close_button(util::PassKey<DialogModelHost>) const {
-    return show_close_button_;
+  const base::Optional<bool>& override_show_close_button(
+      util::PassKey<DialogModelHost>) const {
+    return override_show_close_button_;
   }
 
   const base::string16& title(util::PassKey<DialogModelHost>) const {
@@ -318,7 +324,7 @@ class COMPONENT_EXPORT(UI_BASE) DialogModel final {
   std::unique_ptr<DialogModelDelegate> delegate_;
   DialogModelHost* host_ = nullptr;
 
-  bool show_close_button_ = false;
+  base::Optional<bool> override_show_close_button_;
   bool close_on_deactivate_ = true;
   base::string16 title_;
 

ui/views/bubble/bubble_dialog_model_host.cc

@@ -134,7 +134,11 @@ BubbleDialogModelHost::BubbleDialogModelHost(
   SetButtons(button_mask);
 
   SetTitle(model_->title(GetPassKey()));
-  SetShowCloseButton(model_->show_close_button(GetPassKey()));
+  if (model_->override_show_close_button(GetPassKey())) {
+    SetShowCloseButton(*model_->override_show_close_button(GetPassKey()));
+  } else {
+    SetShowCloseButton(!IsModalDialog());
+  }
   if (model_->is_alert_dialog(GetPassKey()))
     SetAccessibleRole(ax::mojom::Role::kAlertDialog);
 
@@ -484,6 +488,10 @@ std::unique_ptr<View> BubbleDialogModelHost::CreateViewForLabel(
   return text_label;
 }
 
+bool BubbleDialogModelHost::IsModalDialog() const {
+  return GetModalType() != ui::MODAL_TYPE_NONE;
+}
+
 BEGIN_METADATA(BubbleDialogModelHost, BubbleDialogDelegateView)
 END_METADATA
 

ui/views/bubble/bubble_dialog_model_host.h

@@ -74,6 +74,8 @@ class VIEWS_EXPORT BubbleDialogModelHost : public BubbleDialogDelegateView,
 
   View* FieldToView(ui::DialogModelField* field);
 
+  bool IsModalDialog() const;
+
   std::unique_ptr<ui::DialogModel> model_;
   base::flat_map<ui::DialogModelField*, View*> field_to_view_;
   std::vector<PropertyChangedSubscription> property_changed_subscriptions_;