Description: Replace layout constants in chrome/browser/extensions and chrome/browser/first_run

chrome/browser/first_run/try_chrome_dialog_view.cc

Index: chrome/browser/first_run/try_chrome_dialog_view.cc
diff --git a/chrome/browser/first_run/try_chrome_dialog_view.cc b/chrome/browser/first_run/try_chrome_dialog_view.cc
index 4696d13428be980b5e332e55fbadf00f9c344dd4..054e40c0977dafec190c7bf1fb735329167480ed 100644
--- a/chrome/browser/first_run/try_chrome_dialog_view.cc
+++ b/chrome/browser/first_run/try_chrome_dialog_view.cc
@@ -12,6 +12,7 @@
 #include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/process_singleton.h"
+#include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
@@ -33,7 +34,6 @@
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/grid_layout.h"
-#include "ui/views/layout/layout_constants.h"
 #include "ui/views/layout/layout_provider.h"
 #include "ui/views/widget/widget.h"
 
@@ -103,29 +103,37 @@ TryChromeDialogView::Result TryChromeDialogView::ShowModal(
   views::GridLayout* layout = views::GridLayout::CreatePanel(root_view);
   views::ColumnSet* columns;
 
-  // First row: [icon][pad][text][pad][button].
-  columns = layout->AddColumnSet(0);
+  ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
+  const int related_spacing =
+      provider->GetDistanceMetric(views::DISTANCE_RELATED_CONTROL_HORIZONTAL);
+  const int unrelated_spacing =
+      provider->GetDistanceMetric(DISTANCE_UNRELATED_CONTROL_HORIZONTAL);
+  const int button_spacing =
+      provider->GetDistanceMetric(views::DISTANCE_RELATED_BUTTON_HORIZONTAL);
+
+      // First row: [icon][pad][text][pad][button].
+      columns = layout->AddColumnSet(0);
   columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::LEADING, 0,
                      views::GridLayout::FIXED, icon_size.width(),
                      icon_size.height());
-  columns->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
+  columns->AddPaddingColumn(0, related_spacing);

[Peter Kasting, 2017/04/17 19:36:18]

Seems like this should be using RELATED_LABEL, given the comment above.

[ananta, 2017/04/26 02:37:29]

Done.

   columns->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
                      views::GridLayout::USE_PREF, 0, 0);
-  columns->AddPaddingColumn(0, views::kUnrelatedControlHorizontalSpacing);
+  columns->AddPaddingColumn(0, unrelated_spacing);
   columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL, 1,
                      views::GridLayout::USE_PREF, 0, 0);
 
   // Optional second row: [pad][pad][radio 1].
   columns = layout->AddColumnSet(1);
   columns->AddPaddingColumn(0, icon_size.width());
-  columns->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
+  columns->AddPaddingColumn(0, related_spacing);
   columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 1,
                      views::GridLayout::USE_PREF, 0, 0);
 
   // Third row: [pad][pad][radio 2].
   columns = layout->AddColumnSet(2);
   columns->AddPaddingColumn(0, icon_size.width());
-  columns->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
+  columns->AddPaddingColumn(0, related_spacing);
   columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 1,
                      views::GridLayout::USE_PREF, 0, 0);
 
@@ -134,15 +142,14 @@ TryChromeDialogView::Result TryChromeDialogView::ShowModal(
   columns->AddPaddingColumn(0, icon_size.width());

[Peter Kasting, 2017/04/17 19:36:18]

It looks to me as if this was supposed to have another padding column in here,
and is missing it.  What does this dialog look like?

This suggests that instead of adding two padding columns, we should have
computed one "icon + padding" width above and then used it for all these padding
places below.

[ananta, 2017/04/26 02:37:29]

Thanks done.

   columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 0,
                      views::GridLayout::USE_PREF, 0, 0);
-  columns->AddPaddingColumn(0, views::LayoutProvider::Get()->GetDistanceMetric(
-                                   views::DISTANCE_RELATED_BUTTON_HORIZONTAL));
+  columns->AddPaddingColumn(0, button_spacing);
   columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 0,
                      views::GridLayout::USE_PREF, 0, 0);
 
   // Fifth row: [pad][pad][link].
   columns = layout->AddColumnSet(4);
   columns->AddPaddingColumn(0, icon_size.width());
-  columns->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
+  columns->AddPaddingColumn(0, related_spacing);
   columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 1,
                      views::GridLayout::USE_PREF, 0, 0);
 
@@ -160,7 +167,7 @@ TryChromeDialogView::Result TryChromeDialogView::ShowModal(
   columns = layout->AddColumnSet(7);
   columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 0,
                      views::GridLayout::USE_PREF, 0, 0);
-  columns->AddPaddingColumn(0, views::kUnrelatedControlHorizontalSpacing);
+  columns->AddPaddingColumn(0, unrelated_spacing);
   columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL, 1,
                      views::GridLayout::USE_PREF, 0, 0);
 
@@ -232,11 +239,11 @@ TryChromeDialogView::Result TryChromeDialogView::ShowModal(
   if (experiment.flags & installer::kToastUiMakeDefault) {
     // In this flavor we have some veritical space, then a separator line

[Peter Kasting, 2017/04/17 19:36:18]

Nit: While here: correct "veritical" misspelling

[ananta, 2017/04/26 02:37:29]

Done.

     // and the 'make default' checkbox and the OK button on the same row.
-    layout->AddPaddingRow(0, views::kUnrelatedControlVerticalSpacing);
+    layout->AddPaddingRow(0, unrelated_spacing);

[Peter Kasting, 2017/04/17 19:36:18]

This is a behavior change; you went from a vertical constant to a horizontal
one, and the two didn't have the same values.  (2 places)

[ananta, 2017/04/26 02:37:29]

Done.

     layout->StartRow(0, 6);
     separator = new views::Separator();
     layout->AddView(separator);
-    layout->AddPaddingRow(0, views::kUnrelatedControlVerticalSpacing);
+    layout->AddPaddingRow(0, unrelated_spacing);
 
     layout->StartRow(0, 7);
     make_default_ = new views::Checkbox(