Harmony - convert hung renderer dialog.

ui/views/window/dialog_client_view.cc

Index: ui/views/window/dialog_client_view.cc
diff --git a/ui/views/window/dialog_client_view.cc b/ui/views/window/dialog_client_view.cc
index ee88f8654e741d8370cbb236fb3f1f24aeef8efb..5fd2009f28d8da6e4a54f38a2155a948bd167ce2 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -42,9 +42,7 @@ bool ShouldShow(View* view) {
 }
 
 // Do the layout for a button.
-void LayoutButton(LabelButton* button,
-                  gfx::Rect* row_bounds,
-                  int button_height) {
+void LayoutButton(View* button, gfx::Rect* row_bounds, int button_height) {
   if (!button)
     return;
 
@@ -81,9 +79,6 @@ DialogClientView::DialogClientView(Widget* owner, View* contents_view)
   // Doing this now ensures this accelerator will have lower priority than
   // one set by the contents view.
   AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
-
-  if (ViewsDelegate::GetInstance())
-    button_row_insets_ = ViewsDelegate::GetInstance()->GetDialogButtonInsets();

[Peter Kasting, 2017/02/06 21:44:51]

It's not obvious to me why removing this is correct.

[Peter Kasting, 2017/02/07 02:37:15]

Oh nm, I see it's duplicating stuff just above.

Scott says we are clear to remove ViewsDelegate null-checks, so I'd rip out both
this block and the one above and then just init this directly in the initializer
list.

[Bret, 2017/02/09 00:49:49]

Thank god. Removed everywhere in this class.

 }
 
 DialogClientView::~DialogClientView() {
@@ -225,9 +220,13 @@ void DialogClientView::Layout() {
         custom_padding -= GetButtonsAndExtraViewRowTopPadding();
         row_bounds.set_width(row_bounds.width() - custom_padding);
       }
-      row_bounds.set_width(std::min(row_bounds.width(),
-          extra_view_->GetPreferredSize().width()));
-      extra_view_->SetBoundsRect(row_bounds);
+      if (GetDialogDelegate()->GroupExtraViewWithButtons()) {
+        LayoutButton(extra_view_, &row_bounds, button_height);
+      } else {
+        row_bounds.set_width(std::min(row_bounds.width(),
+                                      extra_view_->GetPreferredSize().width()));
+        extra_view_->SetBoundsRect(row_bounds);
+      }
     }
 
     if (height > 0)
@@ -346,8 +345,9 @@ LabelButton* DialogClientView::CreateDialogButton(ui::DialogButton type) {
     button = MdTextButton::CreateSecondaryUiButton(this, title);
   }
 
-  const int kDialogMinButtonWidth = 75;
-  button->SetMinSize(gfx::Size(kDialogMinButtonWidth, 0));
+  // In harmony the minimum size is handled by MdTextButton.
+  if (!ui::MaterialDesignController::IsSecondaryUiMaterial())
+    button->SetMinSize(gfx::Size(kDialogMinimumButtonWidth, 0));

[Peter Kasting, 2017/02/06 21:44:51]

I think this should be plumbed off the ViewsDelegate/LayoutDelegate (and should
return 0 in Harmony, in which case this code should just not call SetMinSize()),
rather than be explicitly conditional on IsSecondaryUiMaterial().

[Bret, 2017/02/09 00:49:49]

Done. I also put in a TODO to delete this along with pre-harmony, because
otherwise it's not clear it's redundant.

   button->SetGroup(kButtonGroup);
   return button;
 }
@@ -372,6 +372,10 @@ gfx::Insets DialogClientView::GetButtonRowInsets() const {
 }
 
 int DialogClientView::GetButtonsAndExtraViewRowTopPadding() const {
+  // Harmony does not use the extra padding.
+  if (ui::MaterialDesignController::IsSecondaryUiMaterial())
+    return 0;

[Peter Kasting, 2017/02/06 21:44:51]

Similarly, maybe this should be plumbed through ViewsDelegate/LayoutDelegate as
something like GetDefaultDialogButtonRowTopPadding().

[Bret, 2017/02/09 00:49:49]

This function didn't really make sense to me so I just rolled the special logic
into GetButtonRowInsets above and then deleted it. I think this might change
some pre-harmony layouts slightly, but we seem less concerned with that lately;
I suppose it's all going away soon anyway.

+
   int spacing = button_row_insets_.top();
   // Some subclasses of DialogClientView, in order to do their own layout, set
   // button_row_insets_ to gfx::Insets(). To avoid breaking behavior of those