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

ui/views/window/dialog_client_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/window/dialog_client_view.h"
 
 #include <algorithm>
 
 #include "build/build_config.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/blue_button.h"
 #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"
 #include "ui/views/window/dialog_delegate.h"
 
 namespace views {
 
 namespace {
 
 // The group used by the buttons.  This name is chosen voluntarily big not to
@@ skipping 19 matching lines @@
   if (!button)
     return;
 
   const gfx::Size size = button->GetPreferredSize();
   row_bounds->set_width(row_bounds->width() - size.width());
   DCHECK_LE(button_height, row_bounds->height());
   button->SetBounds(
       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);
 }
 
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
 // DialogClientView, public:
 
 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));
 }
 
 DialogClientView::~DialogClientView() {
 }
 
 void DialogClientView::AcceptWindow() {
   // Only notify the delegate once. See |delegate_allowed_close_|'s comment.
@@ skipping 33 matching lines @@
 
 const DialogClientView* DialogClientView::AsDialogClientView() const {
   return this;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // DialogClientView, View overrides:
 
 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) +
           GetExtraViewSpacing(),
       0);
 
   int buttons_height = GetButtonsAndExtraViewRowHeight();
   if (buttons_height != 0) {
     size.Enlarge(0, buttons_height);
     // Inset the buttons and extra view.
@@ skipping 36 matching lines @@
     } else {
       LayoutButton(ok_button_, &row_bounds, button_height);
       LayoutButton(cancel_button_, &row_bounds, button_height);
     }
     if (extra_view_) {
       int custom_padding = 0;
       if (has_dialog_buttons() &&
           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(),
                                     extra_view_->GetPreferredSize().width()));
       extra_view_->SetBoundsRect(row_bounds);
     }
 
     if (height > 0) {
       // Inset to the top of the buttons, plus their top padding, in order to
       // exclude that area from the content view's bounds.
@@ skipping 91 matching lines @@
        PlatformStyle::kDialogDefaultButtonCanBeCancel);
 
   // The default button is always blue in Harmony.
   if (is_default && (ui::MaterialDesignController::IsSecondaryUiMaterial() ||
                      GetDialogDelegate()->ShouldDefaultButtonBeBlue())) {
     button = MdTextButton::CreateSecondaryUiBlueButton(this, title);
   } else {
     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);
   return button;
 }
 
 int DialogClientView::GetButtonHeight() const {
   return std::max(
       ok_button_ ? ok_button_->GetPreferredSize().height() : 0,
       cancel_button_ ? cancel_button_->GetPreferredSize().height() : 0);
 }
 
 int DialogClientView::GetExtraViewHeight() const {
   return ShouldShow(extra_view_) ? extra_view_->GetPreferredSize().height() : 0;
 }
 
 int DialogClientView::GetButtonsAndExtraViewRowHeight() const {
   return std::max(GetExtraViewHeight(), GetButtonHeight());
 }
 
 gfx::Insets DialogClientView::GetButtonRowInsets() const {
   if (GetButtonsAndExtraViewRowHeight() == 0)
     return gfx::Insets();
 
   // 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());
 }
 
 void DialogClientView::SetupFocusChain() {
   // Create a vector of child views in the order of intended focus.
   std::vector<View*> child_views;
   child_views.push_back(contents_view());
   child_views.push_back(extra_view_);
@@ skipping 17 matching lines @@
 }
 
 int DialogClientView::GetExtraViewSpacing() const {
   if (!ShouldShow(extra_view_) || !has_dialog_buttons())
     return 0;
 
   int extra_view_padding = 0;
   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() {
   const int buttons = GetDialogDelegate()->GetDialogButtons();
 
   if (buttons & ui::DIALOG_BUTTON_OK) {
     if (!ok_button_) {
       ok_button_ = CreateDialogButton(ui::DIALOG_BUTTON_OK);
       AddChildView(ok_button_);
     }
@@ skipping 20 matching lines @@
     return;
 
   extra_view_ = GetDialogDelegate()->CreateExtraView();
   if (extra_view_) {
     extra_view_->SetGroup(kButtonGroup);
     AddChildView(extra_view_);
   }
 }
 
 }  // namespace views