views: align columns in site settings dialog

chrome/browser/ui/views/website_settings/website_settings_popup_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/website_settings/website_settings_popup_view.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <vector>
@@ skipping 79 matching lines @@
 
 // Spacing above and below the cookies view.
 const int kCookiesViewVerticalPadding = 6;
 
 // Spacing between a permission image and the text.
 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;
 const int STYLED_LABEL_RESET_CERTIFICATE_DECISIONS = 1339;
 const int LINK_COOKIE_DIALOG = 1340;
 const int LINK_SITE_SETTINGS = 1341;
 
 // The default, ui::kTitleFontSizeDelta, is too large for the website settings
 // bubble (e.g. +3). Use +1 to obtain a smaller font.
 constexpr int kSummaryFontSizeDelta = 1;
@@ skipping 345 matching lines @@
 }
 
 void WebsiteSettingsPopupView::WebContentsDestroyed() {
   weak_factory_.InvalidateWeakPtrs();
 }
 
 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();
 }
 
 void WebsiteSettingsPopupView::OnChosenObjectDeleted(
     const WebsiteSettingsUI::ChosenObjectInfo& info) {
   presenter_->OnSiteChosenObjectDeleted(info.ui_info, *info.object);
 }
 
 base::string16 WebsiteSettingsPopupView::GetWindowTitle() const {
   return summary_text_;
@@ skipping 142 matching lines @@
   site_settings_view_->AddChildView(permissions_view_);
 
   const int content_column = 0;
   views::ColumnSet* column_set = layout->AddColumnSet(content_column);
   column_set->AddColumn(views::GridLayout::FILL,
                         views::GridLayout::FILL,
                         1,
                         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) {
     layout->StartRow(1, content_column);
     // The view takes ownership of the object info.
     auto* object_view = new ChosenObjectRow(std::move(object));
     object_view->AddObserver(this);
     layout->AddView(object_view, 1, 1, views::GridLayout::LEADING,
                     views::GridLayout::CENTER);
     layout->AddPaddingRow(1, kPermissionsVerticalSpacing);
@@ skipping 93 matching lines @@
           WebsiteSettings::WEBSITE_SETTINGS_CONNECTION_HELP_OPENED);
       break;
     case STYLED_LABEL_RESET_CERTIFICATE_DECISIONS:
       presenter_->OnRevokeSSLErrorBypassButtonPressed();
       GetWidget()->Close();
       break;
     default:
       NOTREACHED();
   }
 }