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

chrome/browser/ui/views/harmony/layout_delegate.h

 // Copyright 2016 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.
 
 #ifndef CHROME_BROWSER_UI_VIEWS_HARMONY_LAYOUT_DELEGATE_H_
 #define CHROME_BROWSER_UI_VIEWS_HARMONY_LAYOUT_DELEGATE_H_
 
 #include "ui/views/layout/grid_layout.h"
 
 class LayoutDelegate {
@@ skipping 30 matching lines @@
     // checkboxes below explanatory text/headings.
     SUBSECTION_HORIZONTAL_INDENT,
     // Horizontal spacing between controls that are logically unrelated.
     UNRELATED_CONTROL_HORIZONTAL_SPACING,
     // Larger horizontal spacing between unrelated controls.
     UNRELATED_CONTROL_HORIZONTAL_SPACING_LARGE,
     // Vertical spacing between controls that are logically unrelated.
     UNRELATED_CONTROL_VERTICAL_SPACING,
     // Larger vertical spacing between unrelated controls.
     UNRELATED_CONTROL_VERTICAL_SPACING_LARGE,
+    // Horizontal spacing between the image/icon on a label button and its
+    // associated text. This includes checkboxes and radio buttons.

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

I'm confused.  How is this different than RELATED_LABEL_HORIZONTAL_SPACING,
covers "an item such as an icon or checkbox and a label related to it"?

It seems like we shouldn't have both of these.  Unless they really mean
completely different things and should be clarified, let's collapse, even if
that causes a behavior change.

Note: If this stays, alphabetize.

[kylix_rd, 2017/03/01 17:33:27]

The original constant, private to the label_button.cc module, was kSpacing with
a value of 5. RELATED_LABEL_HORIZONTAL_SPACING would eventually resolve to
kItemLabelSpacing which has a value of 10. I added a new constant since it was
different from kItemLabelSpacing.

kItemLabelSpacing is clearly confusing and is used in a different context. A
quick search of the source seems to show that its use isn't what it appears to
say it's for.

[Peter Kasting, 2017/03/01 23:32:23]

Right.  My hope was to either decide we could change the existing behavior and
unify both as either 5 or 10, or else to understand the necessary difference
well enough to make the names and comments really clear so that it would be
obvious what these would actually be used for.  I'm OK with either route, the
first is preferable :)

[Peter Kasting, 2017/03/03 00:05:55]

After looking at the use of RELATED_LABEL_HORIZONTAL_SPACING, I think we should:
* Change its value to 5 in pre-Harmony
* Not add ICON_TEXT_SPACING
* Use RELATED_LABEL_HORIZONTAL_SPACING for cases that would have used
ICON_TEXT_SPACING
* Change website_settings_popup_view.cc:kPermissionImageSpacing to use this
constant as well

+    ICON_TEXT_SPACING,
   };
 
   enum class DialogWidth {
     SMALL,
     MEDIUM,
     LARGE,
   };
 
   LayoutDelegate() {}
   virtual ~LayoutDelegate() {}
@@ skipping 29 matching lines @@
 
   // Returns the preferred width in DIPs for a dialog of the specified |width|.
   // May return 0 if the dialog has no preferred width.
   virtual int GetDialogPreferredWidth(DialogWidth width) const;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(LayoutDelegate);
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_HARMONY_LAYOUT_DELEGATE_H_