views: align columns in site settings dialog

chrome/browser/ui/views/website_settings/website_settings_popup_view.cc

Index: chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
diff --git a/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc b/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
index afd6fe99263041f45d549314517808a09f20c987..bf3d2ea9f09d456e30c4ccfc1459d485d14dbbc9 100644
--- a/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
+++ b/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
@@ -97,6 +97,9 @@ const int kPermissionImageSpacing = 6;
 // Spacing between rows in the site settings section
 const int kPermissionsVerticalSpacing = 12;
 
+// Spacing between the label and the menu.
+const int kPermissionMenuSpacing = 16;
+
 // Button/styled label/link IDs ------------------------------------------------
 const int BUTTON_CLOSE = 1337;
 const int STYLED_LABEL_SECURITY_DETAILS = 1338;
@@ -462,7 +465,8 @@ void WebsiteSettingsPopupView::OnPermissionChanged(
     const WebsiteSettingsUI::PermissionInfo& permission) {
   presenter_->OnSitePermissionChanged(permission.type, permission.setting);
   // The menu buttons for the permissions might have longer strings now, so we
-  // need to size the whole bubble.
+  // need to layout and size the whole bubble.
+  Layout();

[sky, 2017/03/07 22:03:40]

How come you Layout() here?

[Elly Fong-Jones, 2017/03/08 18:41:09]

If we don't, the sizes of the individual permission selectors might not change
after their values have changed. If we did the layout in the individual
selectors instead (where the InvalidateLayout is), the WebsiteSettingsPopupView
won't resize to be wide enough to contain the selectors, so we do the Layout
here instead. I tried to explain this in the comment above but it seems I failed
:(

[sky, 2017/03/08 18:50:35]

Are you sure you don't want the Layout() after the SizeToContents()?

   SizeToContents();
 }
 
@@ -625,16 +629,29 @@ void WebsiteSettingsPopupView::SetPermissionInfo(
                         views::GridLayout::USE_PREF,
                         0,
                         0);
+  const int permissions_column = 1;
+  views::ColumnSet* permissions_set = layout->AddColumnSet(permissions_column);
+  permissions_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
+                             0, views::GridLayout::FIXED,
+                             kPermissionIconColumnWidth, 0);
+  permissions_set->AddPaddingColumn(0, kPermissionIconMarginLeft);
+  permissions_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
+                             0, views::GridLayout::USE_PREF, 0, 0);
+  permissions_set->AddPaddingColumn(1, kPermissionMenuSpacing);
+  permissions_set->AddColumn(views::GridLayout::TRAILING,
+                             views::GridLayout::FILL, 0,
+                             views::GridLayout::USE_PREF, 0, 0);
   for (const auto& permission : permission_info_list) {
-    layout->StartRow(1, content_column);
-    PermissionSelectorRow* selector = new PermissionSelectorRow(
-        profile_,
-        web_contents() ? web_contents()->GetVisibleURL() : GURL::EmptyGURL(),
-        permission);
+    layout->StartRow(1, permissions_column);
+    std::unique_ptr<PermissionSelectorRow> selector =
+        base::MakeUnique<PermissionSelectorRow>(
+            profile_,
+            web_contents() ? web_contents()->GetVisibleURL()
+                           : GURL::EmptyGURL(),
+            permission, layout);
     selector->AddObserver(this);
-    layout->AddView(selector, 1, 1, views::GridLayout::FILL,
-                    views::GridLayout::CENTER);
     layout->AddPaddingRow(1, kPermissionsVerticalSpacing);
+    selector_rows_.push_back(std::move(selector));
   }
 
   for (auto& object : chosen_object_info_list) {