Use DialogModel for ExtensionUninstallDialogView

Adds the following to DialogModel:
* Support for dialog icons through DialogModel::Builder::SetIcon.
* Support for base::string16 (not message_id) in DialogModelLabel.
* Support for character break in DialogModelLabel.

Then refactors ExtensionUninstallDialogView to use DialogModel, which
required adding the above items to DialogModel.

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

chrome/browser/extensions/extension_uninstall_dialog.cc

@@ -73,6 +73,7 @@ ExtensionUninstallDialog::ExtensionUninstallDialog(
     gfx::NativeWindow parent,
     ExtensionUninstallDialog::Delegate* delegate)
     : profile_(profile), parent_(parent), delegate_(delegate) {
+  DCHECK(delegate_);
   if (parent)
     parent_window_tracker_ = NativeWindowTracker::Create(parent);
 }

chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc

@@ -135,7 +135,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
   EXPECT_TRUE(delegate.canceled());
 }
 
-#if defined(OS_CHROMEOS)
 // Test that we don't crash when uninstalling an extension from a web app
 // window in Ash. Context: crbug.com/825554
 IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
@@ -155,12 +154,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
   Browser* app_browser =
       web_app::LaunchWebAppBrowser(browser()->profile(), app_id);
 
+  TestExtensionUninstallDialogDelegate delegate{base::DoNothing()};
   std::unique_ptr<extensions::ExtensionUninstallDialog> dialog;
   {
     base::RunLoop run_loop;
     dialog = extensions::ExtensionUninstallDialog::Create(
         app_browser->profile(), app_browser->window()->GetNativeWindow(),
-        nullptr);
+        &delegate);
     run_loop.RunUntilIdle();
   }
 
@@ -172,7 +172,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
     run_loop.RunUntilIdle();
   }
 }
-#endif  // defined(OS_CHROMEOS)
 
 class ParameterizedExtensionUninstallDialogViewBrowserTest
     : public InProcessBrowserTest,

chrome/browser/ui/views/extensions/extensions_menu_view_browsertest.cc

@@ -37,6 +37,7 @@
 #include "extensions/test/test_extension_dir.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/views/animation/ink_drop.h"
+#include "ui/views/bubble/bubble_dialog_model_host.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/layout/animating_layout_manager.h"
 #include "ui/views/layout/animating_layout_manager_test_util.h"
@@ -88,7 +89,7 @@ class ExtensionsMenuViewBrowserTest : public ExtensionsToolbarBrowserTest {
       // Trigger uninstall dialog.
       views::NamedWidgetShownWaiter waiter(
           views::test::AnyWidgetTestPasskey{},
-          "ExtensionUninstallDialogDelegateView");
+          views::BubbleDialogModelHost::kViewClassName);
       extensions::ExtensionContextMenuModel menu_model(
           extensions()[0].get(), browser(),
           extensions::ExtensionContextMenuModel::PINNED, nullptr,

ui/base/models/dialog_model.h

@@ -14,6 +14,7 @@
 #include "base/util/type_safety/pass_key.h"
 #include "ui/base/models/dialog_model_field.h"
 #include "ui/base/models/dialog_model_host.h"
+#include "ui/base/models/image_model.h"
 #include "ui/base/ui_base_types.h"
 
 namespace ui {
@@ -126,6 +127,11 @@ class COMPONENT_EXPORT(UI_BASE) DialogModel final {
       return *this;
     }
 
+    Builder& SetIcon(ImageModel icon) {
+      model_->icon_ = std::move(icon);
+      return *this;
+    }
+
     // Make screen readers announce the contents of the dialog as it appears.
     // See |ax::mojom::Role::kAlertDialog|.
     Builder& SetIsAlertDialog() {
@@ -281,6 +287,8 @@ class COMPONENT_EXPORT(UI_BASE) DialogModel final {
     return title_;
   }
 
+  const ImageModel& icon(util::PassKey<DialogModelHost>) const { return icon_; }
+
   base::Optional<int> initially_focused_field(
       util::PassKey<DialogModelHost>) const {
     return initially_focused_field_;
@@ -327,8 +335,8 @@ class COMPONENT_EXPORT(UI_BASE) DialogModel final {
   base::Optional<bool> override_show_close_button_;
   bool close_on_deactivate_ = true;
   base::string16 title_;
+  ImageModel icon_;
 
-  static constexpr int kExtraButtonId = DIALOG_BUTTON_LAST + 1;
   std::vector<std::unique_ptr<DialogModelField>> fields_;
   base::Optional<int> initially_focused_field_;
   bool is_alert_dialog_ = false;

ui/base/models/dialog_model_field.cc

@@ -3,7 +3,9 @@
 // found in the LICENSE file.
 
 #include "ui/base/models/dialog_model_field.h"
+
 #include "base/bind.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/dialog_model.h"
 
 namespace ui {
@@ -18,9 +20,23 @@ DialogModelLabel::Link::Link(int message_id, base::RepeatingClosure closure)
 DialogModelLabel::Link::Link(const Link&) = default;
 DialogModelLabel::Link::~Link() = default;
 
-DialogModelLabel::DialogModelLabel(int message_id) : message_id_(message_id) {}
+DialogModelLabel::DialogModelLabel(int message_id)
+    : message_id_(message_id),
+      string_(l10n_util::GetStringUTF16(message_id_)) {}
 DialogModelLabel::DialogModelLabel(int message_id, std::vector<Link> links)
-    : message_id_(message_id), links_(std::move(links)) {}
+    : message_id_(message_id), links_(std::move(links)) {
+  // Note that this constructor does not set |string_| which is invalid for
+  // labels with links.
+}
+
+DialogModelLabel::DialogModelLabel(base::string16 fixed_string)
+    : message_id_(-1), string_(std::move(fixed_string)) {}
+
+const base::string16& DialogModelLabel::GetString(
+    util::PassKey<DialogModelHost>) const {
+  DCHECK(links_.empty());
+  return string_;
+}
 
 DialogModelLabel::DialogModelLabel(const DialogModelLabel&) = default;
 

ui/base/models/dialog_model_field.h

@@ -45,6 +45,7 @@ class COMPONENT_EXPORT(UI_BASE) DialogModelLabel {
   };
 
   explicit DialogModelLabel(int message_id);
+  explicit DialogModelLabel(base::string16 fixed_string);
   DialogModelLabel(const DialogModelLabel&);
   DialogModelLabel& operator=(const DialogModelLabel&) = delete;
   ~DialogModelLabel();
@@ -54,11 +55,22 @@ class COMPONENT_EXPORT(UI_BASE) DialogModelLabel {
   static DialogModelLabel CreateWithLinks(int message_id,
                                           std::vector<Link> links);
 
+  // Gets the string. Not for use with links, in which case the caller must use
+  // links() and message_id() to construct the final label. This is required to
+  // style the final label appropriately and support link callbacks. The caller
+  // is responsible for checking links().empty() before calling this.
+  const base::string16& GetString(util::PassKey<DialogModelHost>) const;
+
   DialogModelLabel& set_is_secondary() {
     is_secondary_ = true;
     return *this;
   }
 
+  DialogModelLabel& set_allow_character_break() {
+    allow_character_break_ = true;
+    return *this;
+  }
+
   int message_id(util::PassKey<DialogModelHost>) const { return message_id_; }
   const std::vector<Link> links(util::PassKey<DialogModelHost>) const {
     return links_;
@@ -66,13 +78,18 @@ class COMPONENT_EXPORT(UI_BASE) DialogModelLabel {
   bool is_secondary(util::PassKey<DialogModelHost>) const {
     return is_secondary_;
   }
+  bool allow_character_break(util::PassKey<DialogModelHost>) const {
+    return allow_character_break_;
+  }
 
  private:
   explicit DialogModelLabel(int message_id, std::vector<Link> links);
 
   const int message_id_;
+  const base::string16 string_;
   const std::vector<Link> links_;
   bool is_secondary_ = false;
+  bool allow_character_break_ = false;
 };
 
 // These "field" classes represent entries in a DialogModel. They are owned

ui/views/bubble/bubble_dialog_model_host.cc

@@ -134,11 +134,19 @@ BubbleDialogModelHost::BubbleDialogModelHost(
   SetButtons(button_mask);
 
   SetTitle(model_->title(GetPassKey()));
+
   if (model_->override_show_close_button(GetPassKey())) {
     SetShowCloseButton(*model_->override_show_close_button(GetPassKey()));
   } else {
     SetShowCloseButton(!IsModalDialog());
   }
+
+  if (!model_->icon(GetPassKey()).IsEmpty()) {
+    // TODO(pbos): Consider adding ImageModel support to SetIcon().
+    SetIcon(model_->icon(GetPassKey()).GetImage().AsImageSkia());
+    SetShowIcon(true);
+  }
+
   if (model_->is_alert_dialog(GetPassKey()))
     SetAccessibleRole(ax::mojom::Role::kAlertDialog);
 
@@ -290,8 +298,20 @@ void BubbleDialogModelHost::AddInitialFields() {
     first_row = false;
   }
 
-  set_margins(LayoutProvider::Get()->GetDialogInsetsForContentType(
-      first_field_content_type, last_field_content_type));
+  gfx::Insets margins = LayoutProvider::Get()->GetDialogInsetsForContentType(
+      first_field_content_type, last_field_content_type);
+  if (!model_->icon(GetPassKey()).IsEmpty()) {
+    // If we have a window icon, inset margins additionally to align with
+    // title label.
+    // TODO(pbos): Reconsider this. Aligning with title gives a massive gap on
+    // the left side of the dialog. This style is from
+    // ExtensionUninstallDialogView as part of refactoring it to use
+    // DialogModel.
+    margins.set_left(
+        margins.left() + model_->icon(GetPassKey()).Size().width() +
+        LayoutProvider::Get()->GetInsetsMetric(INSETS_DIALOG_TITLE).left());
+  }
+  set_margins(margins);
 }
 
 void BubbleDialogModelHost::OnWindowClosing() {
@@ -479,8 +499,7 @@ std::unique_ptr<View> BubbleDialogModelHost::CreateViewForLabel(
   }
 
   auto text_label = std::make_unique<Label>(
-      l10n_util::GetStringUTF16(dialog_label.message_id(GetPassKey())),
-      style::CONTEXT_DIALOG_BODY_TEXT,
+      dialog_label.GetString(GetPassKey()), style::CONTEXT_DIALOG_BODY_TEXT,
       dialog_label.is_secondary(GetPassKey()) ? style::STYLE_SECONDARY
                                               : style::STYLE_PRIMARY);
   text_label->SetMultiLine(true);