Delete panel metrics and define insets in terms of distance metrics.

chrome/browser/ui/views/cookie_info_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 "chrome/browser/ui/views/cookie_info_view.h"
 
 #include <algorithm>
 
 #include "base/i18n/time_formatting.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 131 matching lines @@
   created_label_ = new views::Label(
       l10n_util::GetStringUTF16(IDS_COOKIES_COOKIE_CREATED_LABEL));
   created_value_field_ = new views::Textfield;
   expires_label_ = new views::Label(
       l10n_util::GetStringUTF16(IDS_COOKIES_COOKIE_EXPIRES_LABEL));
   expires_value_field_ = new views::Textfield;
 
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
   ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
-  int dialog_button_margin =
-      provider->GetDistanceMetric(DISTANCE_DIALOG_BUTTON_MARGIN);
-  SetBorder(views::CreateEmptyBorder(0, dialog_button_margin, 0,
-                                     dialog_button_margin));
+  const gfx::Insets button_row_insets =
+      provider->GetInsetsMetric(views::INSETS_DIALOG_BUTTON_ROW);

[Peter Kasting, 2017/05/18 00:00:06]

I was pondering how here you use the button row insets but elsewhere you use
DISTANCE_DIALOG_CONTENTS_HORIZONTAL_MARGIN.  It seems like it's inconsistent,
and they have different consequences in terms of whether they assume the two
side values are symmetric.

Here's a thought for a path forward:
(1) Provide INSETS_DIALOG, which is the dialog's outer insets. 
INSETS_DIALOG_XXX for the sub-pieces can stick around for convenience if we
want, and are computed by taking INSETS_DIALOG and then setting the top or
bottom or whatever to zero.  (To do this, it would be convenient if Insets had
setters fro individual sides.  I've wanted those a lot in the past weeks.)
(2) Make BoxLayout take an Insets arg, instead of the horizontal and vertical
constants it has now.  This is more powerful and simpler to specify, and we can
replace callsites like the ones in this CL with just passing INSETS_DIALOG.
(3) Once this is done, look to eliminate direct usage of
DISTANCE_DIALOG_CONTENTS_XXX_MARGIN in favor of having people use the insets
metrics above.  This aims to force callers not to assume that the left and right
sides match, for example.
(4) Once that's done, make DISTANCE_DIALOG_CONTENTS_XXX_MARGIN be protected in
layout_provider.h instead of public, so they become "implementation detail"
metrics.

WDYT?  In my head, this makes the system clearer and simpler, yet also slightly
more powerful than before.

This is probably all post-this-CL, but I wanted your thoughts while you're
adding this stuff.

[Bret, 2017/05/18 00:50:05]

I tried to only used the HORIZONTAL/VERTICAL metrics when I had to (i.e. for
BoxLayout). I think your way forward is solid. I've also run into several places
where an Insets#set_left or whatever would have been really nice, so I might add
that.

[Peter Kasting, 2017/05/18 00:56:28]

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=723924 on this
stuff.  You're welcome to take it if it would make your life easier.  It seems
like a nice-to-have rather than a must-have for Harmony phase 1, so I'm not
assigning it.

[Bret, 2017/05/18 01:20:30]

I'll probably do it. I'll assign it to myself if I do.

+  SetBorder(views::CreateEmptyBorder(0, button_row_insets.left(), 0,
+                                     button_row_insets.right()));
 
   int three_column_layout_id = 0;
   views::ColumnSet* column_set = layout->AddColumnSet(three_column_layout_id);
   column_set->AddColumn(provider->GetControlLabelGridAlignment(),
                         views::GridLayout::CENTER, 0,
                         views::GridLayout::USE_PREF, 0, 0);
   column_set->AddPaddingColumn(
       0,
       provider->GetDistanceMetric(views::DISTANCE_RELATED_CONTROL_HORIZONTAL));
   column_set->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
                         0, views::GridLayout::USE_PREF, 0, 0);
   column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::CENTER,
                         1, views::GridLayout::USE_PREF, 0, 0);
 
   AddLabelRow(three_column_layout_id, layout, name_label_, name_value_field_);
   AddLabelRow(three_column_layout_id, layout, content_label_,
               content_value_field_);
   AddLabelRow(three_column_layout_id, layout, domain_label_,
               domain_value_field_);
   AddLabelRow(three_column_layout_id, layout, path_label_, path_value_field_);
   AddLabelRow(three_column_layout_id, layout, send_for_label_,
               send_for_value_field_);
   AddLabelRow(three_column_layout_id, layout, created_label_,
               created_value_field_);
   AddLabelRow(three_column_layout_id, layout, expires_label_,
               expires_value_field_);
 }