Use an InsetMetric instead of a constant for vector button padding.

chrome/browser/ui/views/harmony/harmony_layout_delegate.cc

Index: chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
diff --git a/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc b/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
index 57f90992c019c21ba18590ebbbad9fd623c760fc..44c5a75039a839ebe148e52dbc02dc54e939e60a 100644
--- a/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
+++ b/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
@@ -31,9 +31,10 @@ int HarmonyLayoutDelegate::GetMetric(Metric metric) const {
     case Metric::DIALOG_BUTTON_TOP_SPACING:
       return kHarmonyLayoutUnit;
     case Metric::DIALOG_CLOSE_BUTTON_MARGIN:
-      // TODO(pkasting): The "- 4" here is a hack that matches the extra padding
-      // in vector_icon_button.cc and should be removed when that padding is.
-      return (kHarmonyLayoutUnit / 2) - 4;
+      // The amount of padding doesn't affect the button's position, so subtract
+      // it out here.

[Peter Kasting, 2017/03/29 22:39:42]

Nit: Maybe clearer:

      constexpr int kVisibleMargin = kHarmonyLayoutUnit / 2;
      // The visible margin is based on the unpadded size, but the actual margin
      // needs to account for the padding.
      return kVisibleMargin - GetMetric(Metric::VECTOR_IMAGE_BUTTON_PADDING);

[Bret, 2017/03/29 23:06:11]

Done.

+      return (kHarmonyLayoutUnit / 2) -
+             GetMetric(Metric::VECTOR_IMAGE_BUTTON_PADDING);
     case Metric::PANEL_CONTENT_MARGIN:
       return kHarmonyLayoutUnit;
     case Metric::RELATED_BUTTON_HORIZONTAL_SPACING:
@@ -58,6 +59,8 @@ int HarmonyLayoutDelegate::GetMetric(Metric metric) const {
       return kHarmonyLayoutUnit;
     case Metric::UNRELATED_CONTROL_VERTICAL_SPACING_LARGE:
       return kHarmonyLayoutUnit;
+    case Metric::VECTOR_IMAGE_BUTTON_PADDING:
+      return 4;

[Peter Kasting, 2017/03/29 22:39:42]

Nit: It seems like if we scaled up the layout unit we'd probably scale this
value up too, so maybe use kHarmonyLayoutUnit / 4?

[Bret, 2017/03/29 23:06:11]

I went back and forth on this. It's not really a layout unit value, since it
results in a button size of 24 dips, it's just that making the button any
smaller is problematic. I think if the layout unit was 24 for example we'd be
happy keeping it as 4, and if we changed the vector asset size we'd want to
fiddle with it. So I'm going to leave it as just "4".

   }
   NOTREACHED();
   return 0;