Refactor ViewsDelegate and MD-ify the icon-to-text spacing for checkbox and radiobutton

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 f646639183f3608a53116767b366f3b288869699..f221b77a5bcf326175a24fdeae1929945314eb14 100644
--- a/ui/views/window/dialog_client_view.cc
+++ b/ui/views/window/dialog_client_view.cc
@@ -15,6 +15,7 @@
 #include "ui/views/controls/button/custom_button.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/layout/layout_constants.h"
 #include "ui/views/style/platform_style.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
@@ -54,8 +55,10 @@ void LayoutButton(LabelButton* button,
       row_bounds->right(),
       row_bounds->y() + (row_bounds->height() - button_height) / 2,
       size.width(), button_height);
-  const int spacing =
-      ViewsDelegate::GetInstance()->GetDialogRelatedButtonHorizontalSpacing();
+  int spacing = ViewsDelegate::GetInstance()

[Peter Kasting, 2017/03/01 06:32:37]

Doesn't seem like you need to add null-checks.  I suspect this is due to not
rebasing properly, since we used to have things like these, but they got
removed.  (4 places)

+                    ? ViewsDelegate::GetInstance()->GetSpacingMetric(
+                          SpacingMetric::RELATED_HORIZONTAL_BUTTON)
+                    : kRelatedButtonHSpacing;
   row_bounds->set_width(row_bounds->width() - spacing);
 }
 
@@ -66,8 +69,13 @@ void LayoutButton(LabelButton* button,
 
 DialogClientView::DialogClientView(Widget* owner, View* contents_view)
     : ClientView(owner, contents_view),
-      button_row_insets_(
-          ViewsDelegate::GetInstance()->GetDialogButtonInsets()) {
+      button_row_insets_(ViewsDelegate::GetInstance()
+                             ? ViewsDelegate::GetInstance()->GetInsetsMetric(
+                                   InsetsMetric::DIALOG_BUTTON)
+                             : gfx::Insets(0,
+                                           kButtonHEdgeMarginNew,
+                                           kButtonVEdgeMarginNew,
+                                           kButtonHEdgeMarginNew)) {
   // 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));
@@ -121,12 +129,20 @@ const DialogClientView* DialogClientView::AsDialogClientView() const {
 
 gfx::Size DialogClientView::GetPreferredSize() const {
   // Initialize the size to fit the buttons and extra view row.
+  int extra_view_padding = 0;
+  if (!GetDialogDelegate()->GetExtraViewPadding(&extra_view_padding))
+    extra_view_padding = ViewsDelegate::GetInstance()
+                             ? ViewsDelegate::GetInstance()->GetSpacingMetric(
+                                   SpacingMetric::RELATED_HORIZONTAL_BUTTON)
+                             : kRelatedButtonHSpacing;

[Peter Kasting, 2017/03/01 06:32:36]

More bad merging?

   gfx::Size size(
       (ok_button_ ? ok_button_->GetPreferredSize().width() : 0) +
           (cancel_button_ ? cancel_button_->GetPreferredSize().width() : 0) +
           (cancel_button_ && ok_button_
-               ? ViewsDelegate::GetInstance()
-                     ->GetDialogRelatedButtonHorizontalSpacing()
+               ? (ViewsDelegate::GetInstance()
+                      ? ViewsDelegate::GetInstance()->GetSpacingMetric(
+                            SpacingMetric::RELATED_HORIZONTAL_BUTTON)
+                      : kRelatedButtonHSpacing)
                : 0) +
           (ShouldShow(extra_view_) ? extra_view_->GetPreferredSize().width()
                                    : 0) +
@@ -183,8 +199,8 @@ void DialogClientView::Layout() {
           GetDialogDelegate()->GetExtraViewPadding(&custom_padding)) {
         // The padding between buttons applied in LayoutButton() will already
         // have accounted for some of the distance here.
-        custom_padding -= ViewsDelegate::GetInstance()
-                              ->GetDialogRelatedButtonHorizontalSpacing();
+        custom_padding -= ViewsDelegate::GetInstance()->GetSpacingMetric(
+            SpacingMetric::RELATED_HORIZONTAL_BUTTON);
         row_bounds.set_width(row_bounds.width() - custom_padding);
       }
       row_bounds.set_width(std::min(row_bounds.width(),
@@ -296,8 +312,10 @@ LabelButton* DialogClientView::CreateDialogButton(ui::DialogButton type) {
     button = MdTextButton::CreateSecondaryUiButton(this, title);
   }
 
-  const int minimum_width =
-      ViewsDelegate::GetInstance()->GetDialogButtonMinimumWidth();
+  // TODO(bsep): Setting the minimum size is redundant with MdTextButton, so
+  // this can be deleted when harmony is always on.

[Peter Kasting, 2017/03/01 06:32:37]

Another bad merge, I think

+  const int minimum_width = ViewsDelegate::GetInstance()->GetSpacingMetric(
+      SpacingMetric::DIALOG_BUTTON_MINIMUM_WIDTH);
   button->SetMinSize(gfx::Size(minimum_width, 0));
 
   button->SetGroup(kButtonGroup);
@@ -325,11 +343,11 @@ gfx::Insets DialogClientView::GetButtonRowInsets() const {
   // Some subclasses of DialogClientView, in order to do their own layout, set
   // button_row_insets_ to gfx::Insets(). To avoid breaking behavior of those
   // dialogs, supplying 0 for the top inset of the row falls back to
-  // ViewsDelegate::GetRelatedControlVerticalSpacing.
+  // ViewsDelegate::GetSpacingMetric(SpacingMetric::RELATED_VERTICAL_CONTROL).
   // TODO(bsep): The top inset should never be 0 when harmony is enabled.
   const int top = button_row_insets_.top() == 0
-                      ? ViewsDelegate::GetInstance()
-                            ->GetDialogRelatedControlVerticalSpacing()
+                      ? ViewsDelegate::GetInstance()->GetSpacingMetric(
+                            SpacingMetric::RELATED_VERTICAL_CONTROL)
                       : button_row_insets_.top();
   return gfx::Insets(top, button_row_insets_.left(),
                      button_row_insets_.bottom(), button_row_insets_.right());
@@ -367,8 +385,8 @@ int DialogClientView::GetExtraViewSpacing() const {
   if (GetDialogDelegate()->GetExtraViewPadding(&extra_view_padding))
     return extra_view_padding;
 
-  return ViewsDelegate::GetInstance()
-      ->GetDialogRelatedButtonHorizontalSpacing();
+  return ViewsDelegate::GetInstance()->GetSpacingMetric(
+      views::SpacingMetric::RELATED_HORIZONTAL_BUTTON);
 }
 
 void DialogClientView::SetupViews() {