Views/Harmony: Replace layout constants in chrome/browser/ui/views/extensions.

chrome/browser/ui/views/extensions/extension_install_dialog_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/extensions/extension_install_dialog_view.h"
 
 #include <stddef.h>
 #include <algorithm>
 #include <string>
 #include <utility>
@@ skipping 31 matching lines @@
 #include "ui/native_theme/common_theme.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
-#include "ui/views/layout/layout_constants.h"
 #include "ui/views/widget/widget.h"
 
 using content::OpenURLParams;
 using content::Referrer;
 using extensions::ExperienceSamplingEvent;
 
 namespace {
 
 // Width of the bullet column in BulletedView.
 const int kBulletWidth = 20;
 
 // Size of extension icon in top left of dialog.
 const int kIconSize = 64;
 
 // The maximum height of the scroll view before it will show a scrollbar.
 const int kScrollViewMaxHeight = 250;
 
 // Width of the left column of the dialog when the extension requests
 // permissions.
 const int kPermissionsLeftColumnWidth = 250;
 
 // Width of the left column of the dialog when the extension requests no
 // permissions.
 const int kNoPermissionsLeftColumnWidth = 200;
 
 // Width of the left column for external install prompts. The text is long in
 // this case, so make it wider than normal.
 const int kExternalInstallLeftColumnWidth = 350;
 
+// Get the appropriate indentation for an item if its parent is using bullet
+// points. If the parent is using bullets for its items, then a padding of one
+// unit will make the child item (which has no bullet) look like a sibling of
+// its parent. Therefore increase the indentation by one more unit to show that
+// it is in fact a child item (with no missing bullet) and not a sibling.
+int GetLeftPaddingForBulletedItems(const bool parent_bulleted) {

[tapted, 2017/03/22 03:05:59]

nit: no need for `const`

[Patti Lor, 2017/03/22 05:57:40]

Done.

+  return LayoutDelegate::Get()->GetMetric(
+             LayoutDelegate::Metric::RELATED_BUTTON_HORIZONTAL_SPACING) *
+         (parent_bulleted ? 2 : 1);
+}
+
 void AddResourceIcon(const gfx::ImageSkia* skia_image, void* data) {
   views::View* parent = static_cast<views::View*>(data);
   views::ImageView* image_view = new views::ImageView();
   image_view->SetImage(*skia_image);
   parent->AddChildView(image_view);
 }
 
 // Creates a string for displaying |message| to the user. If it has to look
 // like a entry in a bullet point list, one is added.
 base::string16 PrepareForDisplay(const base::string16& message,
@@ skipping 148 matching lines @@
     rating->AddChildView(rating_count);
 
     layout->StartRow(0, column_set_id);
     views::Label* user_count =
         new views::Label(prompt_->GetUserCount(), small_font_list);
     user_count->SetAutoColorReadabilityEnabled(false);
     user_count->SetEnabledColor(SK_ColorGRAY);
     layout->AddView(user_count);
   }
 
+  LayoutDelegate* layout_delegate = LayoutDelegate::Get();
+  const int vertical_padding = layout_delegate->GetMetric(
+      LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING);
   if (prompt_->ShouldShowPermissions()) {
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+    layout->AddPaddingRow(0, vertical_padding);
     layout->StartRow(0, column_set_id);
     layout->AddView(new views::Separator(), 3, 1, views::GridLayout::FILL,
                     views::GridLayout::FILL);
   }
 
-  const int content_width = left_column_width +
-                            LayoutDelegate::Get()->GetMetric(
-                                LayoutDelegate::Metric::PANEL_CONTENT_MARGIN) +
-                            kIconSize;
+  const int content_width =
+      left_column_width +
+      layout_delegate->GetMetric(LayoutDelegate::Metric::PANEL_CONTENT_MARGIN) +
+      kIconSize;
 
   // Create the scrollable view which will contain the permissions and retained
   // files/devices. It will span the full content width.
   CustomScrollableView* scrollable = new CustomScrollableView();
   views::GridLayout* scroll_layout = new views::GridLayout(scrollable);
   scrollable->SetLayoutManager(scroll_layout);
 
   views::ColumnSet* scrollable_column_set =
       scroll_layout->AddColumnSet(column_set_id);
 
   scrollable_column_set->AddColumn(
       views::GridLayout::LEADING, views::GridLayout::LEADING,
       0,  // no resizing
       views::GridLayout::USE_PREF, content_width, content_width);
 
   // Pad to the very right of the dialog, so the scrollbar will be on the edge.
-  scrollable_column_set->AddPaddingColumn(0, views::kButtonHEdgeMarginNew);
+  const int button_margin =
+      layout_delegate->GetMetric(LayoutDelegate::Metric::DIALOG_BUTTON_MARGIN);
+  scrollable_column_set->AddPaddingColumn(0, button_margin);
 
   layout->StartRow(0, column_set_id);
   scroll_view_ = new views::ScrollView();
   scroll_view_->set_hide_horizontal_scrollbar(true);
   scroll_view_->SetContents(scrollable);
   layout->AddView(scroll_view_, 4, 1);
 
   if (prompt_->ShouldShowPermissions()) {
     bool has_permissions =
         prompt_->GetPermissionCount(
             ExtensionInstallPrompt::PermissionsType::ALL_PERMISSIONS) > 0;
     if (has_permissions) {
       AddPermissions(
           scroll_layout, rb, column_set_id, content_width,
           ExtensionInstallPrompt::PermissionsType::REGULAR_PERMISSIONS);
       AddPermissions(
           scroll_layout, rb, column_set_id, content_width,
           ExtensionInstallPrompt::PermissionsType::WITHHELD_PERMISSIONS);
     } else {
-      scroll_layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+      scroll_layout->AddPaddingRow(0, vertical_padding);
       scroll_layout->StartRow(0, column_set_id);
       views::Label* permission_label = new views::Label(
           l10n_util::GetStringUTF16(IDS_EXTENSION_NO_SPECIAL_PERMISSIONS));
       permission_label->SetMultiLine(true);
       permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
       permission_label->SizeToFit(content_width);
       scroll_layout->AddView(permission_label);
     }
   }
 
   if (prompt_->GetRetainedFileCount()) {
-    scroll_layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+    scroll_layout->AddPaddingRow(0, vertical_padding);
 
     scroll_layout->StartRow(0, column_set_id);
     views::Label* retained_files_header =
         new views::Label(prompt_->GetRetainedFilesHeading());
     retained_files_header->SetMultiLine(true);
     retained_files_header->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     retained_files_header->SizeToFit(content_width);
     scroll_layout->AddView(retained_files_header);
 
     scroll_layout->StartRow(0, column_set_id);
     PermissionDetails details;
     for (size_t i = 0; i < prompt_->GetRetainedFileCount(); ++i) {
       details.push_back(prompt_->GetRetainedFile(i));
     }
     ExpandableContainerView* issue_advice_view =
         new ExpandableContainerView(details, content_width, false);
     scroll_layout->AddView(issue_advice_view);
   }
 
   if (prompt_->GetRetainedDeviceCount()) {
-    scroll_layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+    scroll_layout->AddPaddingRow(0, vertical_padding);
 
     scroll_layout->StartRow(0, column_set_id);
     views::Label* retained_devices_header =
         new views::Label(prompt_->GetRetainedDevicesHeading());
     retained_devices_header->SetMultiLine(true);
     retained_devices_header->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     retained_devices_header->SizeToFit(content_width);
     scroll_layout->AddView(retained_devices_header);
 
     scroll_layout->StartRow(0, column_set_id);
     PermissionDetails details;
     for (size_t i = 0; i < prompt_->GetRetainedDeviceCount(); ++i) {
       details.push_back(prompt_->GetRetainedDeviceMessageString(i));
     }
     ExpandableContainerView* issue_advice_view =
         new ExpandableContainerView(details, content_width, false);
     scroll_layout->AddView(issue_advice_view);
   }
 
   DCHECK_GE(prompt_->type(), 0);
   UMA_HISTOGRAM_ENUMERATION("Extensions.InstallPrompt.Type",
                             prompt_->type(),
                             ExtensionInstallPrompt::NUM_PROMPT_TYPES);
 
   scroll_view_->ClipHeightTo(
       0,
       std::min(kScrollViewMaxHeight, scrollable->GetPreferredSize().height()));
 
-  dialog_size_ = gfx::Size(
-      content_width + 2 * views::kButtonHEdgeMarginNew,
-      container_->GetPreferredSize().height());
+  dialog_size_ = gfx::Size(content_width + 2 * button_margin,
+                           container_->GetPreferredSize().height());
 
   std::string event_name = ExperienceSamplingEvent::kExtensionInstallDialog;
   event_name.append(
       ExtensionInstallPrompt::PromptTypeToString(prompt_->type()));
   sampling_event_ = ExperienceSamplingEvent::Create(event_name);
 }
 
 bool ExtensionInstallDialogView::AddPermissions(
     views::GridLayout* layout,
     ui::ResourceBundle& rb,
     int column_set_id,
     int left_column_width,
     ExtensionInstallPrompt::PermissionsType perm_type) {
   if (prompt_->GetPermissionCount(perm_type) == 0)
     return false;
 
-  layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  const int vertical_padding = LayoutDelegate::Get()->GetMetric(
+      LayoutDelegate::Metric::RELATED_CONTROL_VERTICAL_SPACING);
+  layout->AddPaddingRow(0, vertical_padding);
 
   layout->StartRow(0, column_set_id);
   views::Label* permissions_header =
       new views::Label(prompt_->GetPermissionsHeading(perm_type));
   permissions_header->SetMultiLine(true);
   permissions_header->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   permissions_header->SizeToFit(left_column_width);
   layout->AddView(permissions_header);
 
   for (size_t i = 0; i < prompt_->GetPermissionCount(perm_type); ++i) {
-    layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+    layout->AddPaddingRow(0, vertical_padding);
     layout->StartRow(0, column_set_id);
     views::Label* permission_label =
         new views::Label(prompt_->GetPermission(i, perm_type));
 
     permission_label->SetMultiLine(true);
     permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     permission_label->SizeToFit(left_column_width - kBulletWidth);
     layout->AddView(new BulletedView(permission_label));
 
     // If we have more details to provide, show them in collapsed form.
     if (!prompt_->GetPermissionsDetails(i, perm_type).empty()) {
       layout->StartRow(0, column_set_id);
       PermissionDetails details;
       details.push_back(PrepareForDisplay(
           prompt_->GetPermissionsDetails(i, perm_type), false));
       ExpandableContainerView* details_container =
           new ExpandableContainerView(details, left_column_width, true);
       layout->AddView(details_container);
     }
   }
   return true;
 }
 
 views::GridLayout* ExtensionInstallDialogView::CreateLayout(
     int left_column_width,
     int column_set_id) {
   container_ = new views::View();
-  // This is basically views::GridLayout::CreatePanel, but without a top or
-  // right margin (we effectively get a top margin anyway from the empty dialog
-  // title, and we add an explicit padding column as a right margin below).
+  LayoutDelegate* layout_delegate = LayoutDelegate::Get();
+  const int horizontal_margin =
+      layout_delegate->GetMetric(LayoutDelegate::Metric::DIALOG_BUTTON_MARGIN);
+  const int bottom_margin =
+      layout_delegate->GetMetric(LayoutDelegate::Metric::PANEL_CONTENT_MARGIN);
+
+  // This is views::GridLayout::CreatePanel(), but without a top or right
+  // margin. The empty dialog title will then become the top margin, and a
+  // padding column will be manually added to handle a right margin. This is
+  // done so that the extension icon can be shown on the right of the dialog
+  // title, but on the same y-axis, and the scroll view used to contain other
+  // content can have its scrollbar aligned with the right edge of the dialog.
   views::GridLayout* layout = new views::GridLayout(container_);
-  layout->SetInsets(0, views::kButtonHEdgeMarginNew,
-                    LayoutDelegate::Get()->GetMetric(
-                        LayoutDelegate::Metric::PANEL_CONTENT_MARGIN),
-                    0);
+  layout->SetInsets(0, horizontal_margin, bottom_margin, 0);
   container_->SetLayoutManager(layout);
   AddChildView(container_);
 
   views::ColumnSet* column_set = layout->AddColumnSet(column_set_id);
   column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::LEADING,
                         0,  // no resizing
                         views::GridLayout::USE_PREF,
                         0,  // no fixed width
                         left_column_width);
   column_set->AddPaddingColumn(
-      0, LayoutDelegate::Get()->GetMetric(
-             LayoutDelegate::Metric::PANEL_CONTENT_MARGIN));
+      0, layout_delegate->GetMetric(
+             LayoutDelegate::Metric::UNRELATED_CONTROL_HORIZONTAL_SPACING));
   column_set->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
                         0,  // no resizing
                         views::GridLayout::USE_PREF,
                         0,  // no fixed width
                         kIconSize);
-  column_set->AddPaddingColumn(0, views::kButtonHEdgeMarginNew);
+  column_set->AddPaddingColumn(0, horizontal_margin);
 
   layout->StartRow(0, column_set_id);
   views::Label* title =
       new views::Label(prompt_->GetDialogTitle(),
                        ui::ResourceBundle::GetSharedInstance().GetFontList(
                            ui::ResourceBundle::MediumFont));
   title->SetMultiLine(true);
   title->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   title->SizeToFit(left_column_width);
 
@@ skipping 122 matching lines @@
 }
 
 // ExpandableContainerView::DetailsView ----------------------------------------
 
 ExpandableContainerView::DetailsView::DetailsView(int horizontal_space,
                                                   bool parent_bulleted)
     : layout_(new views::GridLayout(this)),
       state_(0) {
   SetLayoutManager(layout_);
   views::ColumnSet* column_set = layout_->AddColumnSet(0);
-  // If the parent is using bullets for its items, then a padding of one unit
-  // will make the child item (which has no bullet) look like a sibling of its
-  // parent. Therefore increase the indentation by one more unit to show that it
-  // is in fact a child item (with no missing bullet) and not a sibling.
-  int padding =
-      views::kRelatedControlHorizontalSpacing * (parent_bulleted ? 2 : 1);
+  int padding = GetLeftPaddingForBulletedItems(parent_bulleted);

[tapted, 2017/03/22 03:05:59]

nit: inline below (`padding` temporary no required)

[Patti Lor, 2017/03/22 05:57:41]

Done - replaced in 2 places.

   column_set->AddPaddingColumn(0, padding);
   column_set->AddColumn(views::GridLayout::LEADING,
                         views::GridLayout::LEADING,
                         0,
                         views::GridLayout::FIXED,
                         horizontal_space - padding,
                         0);
 }
 
 void ExpandableContainerView::DetailsView::AddDetail(
     const base::string16& detail) {
-  layout_->StartRowWithPadding(0, 0,
-                               0, views::kRelatedControlSmallVerticalSpacing);
+  layout_->StartRowWithPadding(
+      0, 0, 0,
+      LayoutDelegate::Get()->GetMetric(
+          LayoutDelegate::Metric::RELATED_CONTROL_HORIZONTAL_SPACING));

[tapted, 2017/03/22 03:05:59]

For this, I suggest

constexpr int kPaddingAboveDetailsLink = 4;

There is RELATED_CONTROL_VERTICAL_SPACING. But
 - that is 8px rather than 4px.
 - this isn't a control -- it's just text

[Patti Lor, 2017/03/22 05:57:41]

Done. I also updated all the constants in the anon namespace above to use
constexpr.

   views::Label* detail_label =
       new views::Label(PrepareForDisplay(detail, false));
   detail_label->SetMultiLine(true);
   detail_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   layout_->AddView(detail_label);
 }
 
 gfx::Size ExpandableContainerView::DetailsView::GetPreferredSize() const {
   gfx::Size size = views::View::GetPreferredSize();
   return gfx::Size(size.width(), size.height() * state_);
@@ skipping 28 matching lines @@
 
   details_view_ = new DetailsView(horizontal_space, parent_bulleted);
 
   layout->StartRow(0, column_set_id);
   layout->AddView(details_view_);
 
   for (size_t i = 0; i < details.size(); ++i)
     details_view_->AddDetail(details[i]);
 
   // Make sure the link width column is as wide as needed for both Show and
-  // Hide details, so that the arrow doesn't shift horizontally when we
-  // toggle.
+  // Hide details, so that the arrow doesn't shift horizontally when we toggle.
   views::Link* link = new views::Link(
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_DETAILS));
   int link_col_width = link->GetPreferredSize().width();
   link->SetText(l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS));
   link_col_width = std::max(link_col_width, link->GetPreferredSize().width());
 
   column_set = layout->AddColumnSet(++column_set_id);
-  // Padding to the left of the More Details column. If the parent is using
-  // bullets for its items, then a padding of one unit will make the child
-  // item (which has no bullet) look like a sibling of its parent. Therefore
-  // increase the indentation by one more unit to show that it is in fact a
-  // child item (with no missing bullet) and not a sibling.
-  column_set->AddPaddingColumn(
-      0, views::kRelatedControlHorizontalSpacing * (parent_bulleted ? 2 : 1));
+  // Padding to the left of the More Details column.
+  column_set->AddPaddingColumn(0,
+                               GetLeftPaddingForBulletedItems(parent_bulleted));
   // The More Details column.
   column_set->AddColumn(views::GridLayout::LEADING,
                         views::GridLayout::LEADING,
                         0,
                         views::GridLayout::FIXED,
                         link_col_width,
                         link_col_width);
   // The Up/Down arrow column.
   column_set->AddColumn(views::GridLayout::LEADING,
                         views::GridLayout::TRAILING,
@@ skipping 62 matching lines @@
   gfx::ImageSkia icon = gfx::CreateVectorIcon(
       expanded ? kCaretUpIcon : kCaretDownIcon, gfx::kChromeIconGrey);
   arrow_toggle_->SetImage(views::Button::STATE_NORMAL, &icon);
 }
 
 // static
 ExtensionInstallPrompt::ShowDialogCallback
 ExtensionInstallPrompt::GetViewsShowDialogCallback() {
   return base::Bind(&ShowExtensionInstallDialogImpl);
 }