Broke out layout metric information from ViewsDelegate to LayoutProvider

chrome/browser/chromeos/options/vpn_config_view.cc

Index: chrome/browser/chromeos/options/vpn_config_view.cc
diff --git a/chrome/browser/chromeos/options/vpn_config_view.cc b/chrome/browser/chromeos/options/vpn_config_view.cc
index ee6204c59782243f14b738c541962790fb398eb7..c789e105611f512529c35940411cb2f61ccd2b4b 100644
--- a/chrome/browser/chromeos/options/vpn_config_view.cc
+++ b/chrome/browser/chromeos/options/vpn_config_view.cc
@@ -14,7 +14,7 @@
 #include "chrome/browser/chromeos/enrollment_dialog_view.h"
 #include "chrome/browser/chromeos/net/shill_error.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/ui/views/harmony/layout_delegate.h"
+#include "chrome/browser/ui/views/harmony/chrome_views_layout_delegate.h"
 #include "chrome/browser/ui/views/layout_utils.h"
 #include "chrome/common/net/x509_certificate_model.h"
 #include "chrome/grit/generated_resources.h"
@@ -505,7 +505,7 @@ void VPNConfigView::Init() {
     DCHECK(vpn && vpn->type() == shill::kTypeVPN);
   }
   layout_ = layout_utils::CreatePanelLayout(this);
-  LayoutDelegate* delegate = LayoutDelegate::Get();
+  ChromeViewsLayoutDelegate* delegate = ChromeViewsLayoutDelegate::Get();

[Peter Kasting, 2017/03/30 00:49:04]

Nit: For this and other files in this directory, use the base (views) version of
the delegate and constants, then remove chrome_views_layout_delegate.h from the
DEPS for this directory.

[kylix_rd, 2017/03/30 19:34:07]

Duh... I kind of went on auto-pilot and didn't look close enough that the
ViewsLayoutDelegate was sufficient here. Thanks.

Done.

[Peter Kasting, 2017/03/30 19:40:02]

Do you think it makes sense to suggest as a rule of thumb "use the core class
everywhere you don't actually need the Chrome-specific side"?  That might make
future componentization easier (if any occurs).

If this makes sense, it might be a minor argument for the "simpler enum model"
approach, because there will be no enum overlap -- so whether code really needs
to depend on Chrome-side values will become more obvious.

[kylix_rd, 2017/03/31 21:12:40]

Yes, that makes sense. We just need to ensure that only those things that are
truly "core" use the core class. We don't want folks pushing things into the
core which rightly belong in the Chrome-specific side. It should be easy to
determine, but I've found that it pays to be very specific. It also doesn't make
sense to mix core and Chrome stuff in the same file. In general, things in
c/b/ui/views should use the ChromeViewsLayoutDelegate, and other stuff should
favor ViewsLayoutDelegate when it makes sense.
 
Hmmm... that is a reasonable argument for the simpler enum approach. I've not
zeroed in on another, more palatable approach yet but I'm determined.

[Peter Kasting, 2017/03/31 22:14:29]

I think we're probably on the same page in general, but I would be more open to
people using different types in the same file if some of the uses can get away
with only using the core class.  But there's little difference to me either way
on that one since the pros and cons of the two routes are so minor.

I definitely would not want people to end up pushing things down into ui/views
that could have stayed in chrome/ just because they didn't realize they could
switch delegate types.  I don't know how significant that risk is (my guess is
not very).

 
   // Observer any changes to the certificate list.
   CertLibrary::Get()->AddObserver(this);
@@ -515,15 +515,15 @@ void VPNConfigView::Init() {
   column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 1,
                         views::GridLayout::USE_PREF, 0, 0);
   column_set->AddPaddingColumn(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_HORIZONTAL_SPACING));
+      0, delegate->GetDistanceMetric(
+             ChromeDistanceMetric::RELATED_CONTROL_HORIZONTAL));
   // Textfield, combobox.
   column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
                         views::GridLayout::USE_PREF, 0,
                         ChildNetworkConfigView::kInputFieldMinWidth);
   column_set->AddPaddingColumn(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_HORIZONTAL_SPACING));
+      0, delegate->GetDistanceMetric(
+             ChromeDistanceMetric::RELATED_CONTROL_HORIZONTAL));
   // Policy indicator.
   column_set->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, 0,
                         views::GridLayout::USE_PREF, 0, 0);
@@ -548,9 +548,9 @@ void VPNConfigView::Init() {
   server_textfield_ = new views::Textfield();
   server_textfield_->set_controller(this);
   layout_->AddView(server_textfield_);
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
   if (!service_path_.empty()) {
     server_label->SetEnabled(false);
     server_textfield_->SetEnabled(false);
@@ -571,9 +571,9 @@ void VPNConfigView::Init() {
     layout_->AddView(service_text_);
     service_textfield_ = NULL;
   }
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // Provider type label and select.
   layout_->StartRow(0, 0);
@@ -593,9 +593,9 @@ void VPNConfigView::Init() {
     layout_->AddView(provider_type_text_label_);
     provider_type_combobox_ = NULL;
   }
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // PSK passphrase label, input and visible button.
   layout_->StartRow(0, 0);
@@ -607,9 +607,9 @@ void VPNConfigView::Init() {
   layout_->AddView(psk_passphrase_textfield_);
   layout_->AddView(
       new ControlledSettingIndicatorView(psk_passphrase_ui_data_));
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // Server CA certificate
   if (service_path_.empty()) {
@@ -623,9 +623,9 @@ void VPNConfigView::Init() {
         server_ca_cert_combobox_model_.get());
     layout_->AddView(server_ca_cert_combobox_);
     layout_->AddView(new ControlledSettingIndicatorView(ca_cert_ui_data_));
-    layout_->AddPaddingRow(
-        0, delegate->GetMetric(
-               LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+    layout_->AddPaddingRow(0,
+                           delegate->GetDistanceMetric(
+                               ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
   } else {
     server_ca_cert_label_ = NULL;
     server_ca_cert_combobox_ = NULL;
@@ -642,9 +642,9 @@ void VPNConfigView::Init() {
   user_cert_combobox_->set_listener(this);
   layout_->AddView(user_cert_combobox_);
   layout_->AddView(new ControlledSettingIndicatorView(user_cert_ui_data_));
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // Username label and input.
   layout_->StartRow(0, 0);
@@ -655,9 +655,9 @@ void VPNConfigView::Init() {
   username_textfield_->SetEnabled(username_ui_data_.IsEditable());
   layout_->AddView(username_textfield_);
   layout_->AddView(new ControlledSettingIndicatorView(username_ui_data_));
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // User passphrase label, input and visble button.
   layout_->StartRow(0, 0);
@@ -669,9 +669,9 @@ void VPNConfigView::Init() {
   layout_->AddView(user_passphrase_textfield_);
   layout_->AddView(
       new ControlledSettingIndicatorView(user_passphrase_ui_data_));
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // OTP label and input.
   layout_->StartRow(0, 0);
@@ -681,9 +681,9 @@ void VPNConfigView::Init() {
   otp_textfield_ = new views::Textfield();
   otp_textfield_->set_controller(this);
   layout_->AddView(otp_textfield_);
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // Group Name label and input.
   layout_->StartRow(0, 0);
@@ -695,9 +695,9 @@ void VPNConfigView::Init() {
   group_name_textfield_->set_controller(this);
   layout_->AddView(group_name_textfield_);
   layout_->AddView(new ControlledSettingIndicatorView(group_name_ui_data_));
-  layout_->AddPaddingRow(
-      0, delegate->GetMetric(
-             LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING));
+  layout_->AddPaddingRow(0,
+                         delegate->GetDistanceMetric(
+                             ChromeDistanceMetric::RELATED_CONTROL_VERTICAL));
 
   // Save credentials
   layout_->StartRow(0, 0);