Material Page Info (Views, 3/3): Update site settings section.

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 54 matching lines @@
 namespace {
 
 // NOTE(jdonnelly): This use of this process-wide variable assumes that there's
 // never more than one website settings popup shown and that it's associated
 // with the current window. If this assumption fails in the future, we'll need
 // to return a weak pointer from ShowPopup so callers can associate it with the
 // current window (or other context) and check if the popup they care about is
 // showing.
 bool is_popup_showing = false;
 
-// Left icon margin.
-const int kIconMarginLeft = 6;
+// General constants -----------------------------------------------------------
+
+// Popup width constraints.
+const int kMinPopupWidth = 320;
+const int kMaxPopupWidth = 1000;
+
+// Margin and padding values shared by all sections.
+const int kSectionPaddingHorizontal = 16;
+
+// Margin and padding values shared by all sections.

[msw, 2016/10/08 00:21:57]

nit: omit comment and line above (line 81 comment can be shared)

[lgarron, 2016/10/08 03:50:04]

Comment updated instead.

+const int kPopupMarginBottom = 16;
+
+// Security Section (PopupHeaderView) ------------------------------------------
 
 // Margin and padding values for the |PopupHeaderView|.
 const int kHeaderMarginBottom = 10;
 const int kHeaderPaddingBottom = 16;
-const int kHeaderPaddingLeft = 18;
-const int kHeaderPaddingRightForText = kHeaderPaddingLeft;
 const int kHeaderPaddingTop = 16;
 const int kHeaderPaddingForCloseButton = 8;
 
 // Spacing between labels in the header.
 const int kHeaderLabelSpacing = 4;
 
-// The max possible width of the popup.
-const int kMaxPopupWidth = 1000;
+// Site Settings Section -------------------------------------------------------
 
-// The margins between the popup border and the popup content.
-const int kPopupMarginBottom = 14;
+// Spacing before and after the cookies view.

[msw, 2016/10/08 00:21:57]

nit: s/before and after/above and below/

[lgarron, 2016/10/08 03:50:04]

Done.

+const int kCookiesViewVerticalPadding = 6;
 
-// Padding values for sections on the site settings view.
-const int kSiteSettingsViewContentMinWidth = 300;
-const int kSiteSettingsViewPaddingBottom = 6;
-const int kSiteSettingsViewPaddingLeft = 18;
-const int kSiteSettingsViewPaddingRight = 18;
-const int kSiteSettingsViewPaddingTop = 4;
+// Square size of the permission images.
+const int kPermissionImageSize = 16;

[msw, 2016/10/08 00:21:57]

nit: any chance we could replace/merge this with kPermissionIconColumnWidth?

[lgarron, 2016/10/08 03:50:04]

Replaced.

 
-// Space between the headline and the content of a section.
-const int kSiteSettingsViewHeadlineMarginBottom = 10;
-// Spacing between rows in the "Permissions" and "Cookies and Site Data"
-// sections.
-const int kContentRowSpacing = 2;
+// Spacing between a permission image and the text.
+const int kPermissionImageSpacing = 6;
 
-const int kSiteDataIconColumnWidth = 20;
+// Spacing between rows in the site settings section
+const int kPermissionsVerticalSpacing = 12;
+
+// Button IDs ------------------------------------------------------------------
 
 const int BUTTON_RESET_CERTIFICATE_DECISIONS = 1337;
+const int BUTTON_SITE_SETTINGS = 1338;
 
 }  // namespace
 
 // |PopupHeaderView| is the UI element (view) that represents the header of the
 // |WebsiteSettingsPopupView|. The header shows the status of the site's
 // identity check and the name of the site's identity.
 class PopupHeaderView : public views::View {
  public:
   explicit PopupHeaderView(views::ButtonListener* button_listener,
                            views::StyledLabelListener* styled_label_listener);
   ~PopupHeaderView() override;
 
   // Sets the security summary for the current page.
   void SetSummary(const base::string16& summary_text);
 
   // Sets the security details for the current page.
   void SetDetails(const base::string16& details_text,
                   bool include_details_link);
 
-  int GetPreferredNameWidth() const;
-
   void AddResetDecisionsLabel();
 
  private:
   // The listener for the styled labels in this view.
   views::StyledLabelListener* styled_label_listener_;
 
   // The label that displays security summary for the current page.
   views::Label* summary_label_;
 
   // The label that displays the status of the identity check for this site.
@@ skipping 43 matching lines @@
     : styled_label_listener_(styled_label_listener),
       summary_label_(nullptr),
       details_label_(nullptr),
       reset_decisions_label_container_(nullptr),
       reset_decisions_label_(nullptr) {
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
 
   const int label_column = 0;
   views::ColumnSet* column_set = layout->AddColumnSet(label_column);
-  column_set->AddPaddingColumn(0, kHeaderPaddingLeft);
+  column_set->AddPaddingColumn(0, kSectionPaddingHorizontal);
   column_set->AddColumn(views::GridLayout::FILL,
                         views::GridLayout::FILL,
                         1,
                         views::GridLayout::USE_PREF,
                         0,
                         0);
   column_set->AddPaddingColumn(1, 0);
   column_set->AddColumn(views::GridLayout::FILL,
                         views::GridLayout::FILL,
                         1,
                         views::GridLayout::USE_PREF,
                         0,
                         0);
   column_set->AddPaddingColumn(0, kHeaderPaddingForCloseButton);
 
   // First we add the padding needed for the close button.
   // In order to move down the summary, we simulate additional padding by giving
   // it an empty border on top later on.
   layout->AddPaddingRow(0, kHeaderPaddingForCloseButton);
 
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
 
   layout->StartRow(0, label_column);
   const gfx::FontList& font_list = rb.GetFontListWithDelta(1);
   summary_label_ = new views::Label(base::string16(), font_list);
+  summary_label_->SetMultiLine(true);
   summary_label_->SetBorder(views::Border::CreateEmptyBorder(
       kHeaderPaddingTop - kHeaderPaddingForCloseButton, 0, 0, 0));
   layout->AddView(summary_label_, 1, 1, views::GridLayout::LEADING,
                   views::GridLayout::TRAILING);
   views::ImageButton* close_button = new views::ImageButton(button_listener);
   close_button->SetImage(views::CustomButton::STATE_NORMAL,
                          rb.GetImageNamed(IDR_CLOSE_2).ToImageSkia());
   close_button->SetImage(views::CustomButton::STATE_HOVERED,
                          rb.GetImageNamed(IDR_CLOSE_2_H).ToImageSkia());
   close_button->SetImage(views::CustomButton::STATE_PRESSED,
                          rb.GetImageNamed(IDR_CLOSE_2_P).ToImageSkia());
   layout->AddView(close_button, 1, 1, views::GridLayout::TRAILING,
                   views::GridLayout::LEADING);
 
   layout->AddPaddingRow(0, kHeaderLabelSpacing);
 
   const int label_column_status = 1;
   views::ColumnSet* column_set_status =
       layout->AddColumnSet(label_column_status);
-  column_set_status->AddPaddingColumn(0, kHeaderPaddingLeft);
+  column_set_status->AddPaddingColumn(0, kSectionPaddingHorizontal);
   column_set_status->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
                                1, views::GridLayout::USE_PREF, 0, 0);
-  column_set_status->AddPaddingColumn(0, kHeaderPaddingRightForText);
+  column_set_status->AddPaddingColumn(0, kSectionPaddingHorizontal);
 
   layout->AddPaddingRow(0, kHeaderLabelSpacing);
 
   layout->StartRow(0, label_column_status);
   details_label_ =
       new views::StyledLabel(base::string16(), styled_label_listener);
   layout->AddView(details_label_, 1, 1, views::GridLayout::FILL,
                   views::GridLayout::LEADING);
 
   layout->StartRow(0, label_column_status);
   reset_decisions_label_container_ = new views::View();
   reset_decisions_label_container_->SetLayoutManager(
       new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0));
   layout->AddView(reset_decisions_label_container_, 1, 1,
                   views::GridLayout::FILL, views::GridLayout::LEADING);
 
   layout->AddPaddingRow(1, kHeaderPaddingBottom);
 }
 
 PopupHeaderView::~PopupHeaderView() {}
 
-int PopupHeaderView::GetPreferredNameWidth() const {
-  return summary_label_->GetPreferredSize().width();
-}
-
 void PopupHeaderView::SetSummary(const base::string16& summary_text) {
   summary_label_->SetText(summary_text);
 }
 
 void PopupHeaderView::SetDetails(const base::string16& details_text,
                                  bool include_details_label_link) {
   if (include_details_label_link) {
     base::string16 details_link_text =
         l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_DETAILS_LINK);
 
@@ skipping 161 matching lines @@
     gfx::NativeView parent_window,
     Profile* profile,
     content::WebContents* web_contents,
     const GURL& url,
     const security_state::SecurityStateModel::SecurityInfo& security_info)
     : content::WebContentsObserver(web_contents),
       BubbleDialogDelegateView(anchor_view, views::BubbleBorder::TOP_LEFT),
       header_(nullptr),
       separator_(nullptr),
       site_settings_view_(nullptr),
-      site_data_content_(nullptr),
+      cookies_view_(nullptr),
       cookie_dialog_link_(nullptr),
-      permissions_content_(nullptr),
-      site_settings_link_(nullptr),
+      permissions_view_(nullptr),
       weak_factory_(this) {
   set_parent_window(parent_window);
 
   is_devtools_disabled_ =
       profile->GetPrefs()->GetBoolean(prefs::kDevToolsDisabled);
 
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(
       GetLayoutConstant(LOCATION_BAR_BUBBLE_ANCHOR_VERTICAL_INSET), 0));
 
@@ skipping 38 matching lines @@
     GetWidget()->Close();
 }
 
 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.
+  SizeToContents();

[lgarron, 2016/10/07 00:54:36]

Is it okay to call this in the observer callback, or should I rely on the Views
hierarchy (e.g. implement WebsiteSettingsPopupView::ChildPreferredSizeChanged()
and call SizeToContents() in it)?

[msw, 2016/10/07 23:56:23]

Making a ChildPreferredSizeChanged override that calls SizeToContents seems
preferable to me, it's already done for a couple other bubbles and seems more
direct and complete. (for example, that should handle chosen object changes too,
I'm not sure exactly how that's working otherwise at the moment)

[lgarron, 2016/10/08 03:50:04]

Hmm, I can't figure out the right combination of
ChildPreferredSizeChanged/SizeToPreferredSize/SizeToContents to implement/call.
Will have to pick it up another day. :-(

[msw, 2016/10/08 16:15:25]

Acknowledged.

 }
 
 void WebsiteSettingsPopupView::OnChosenObjectDeleted(
     const WebsiteSettingsUI::ChosenObjectInfo& info) {
   presenter_->OnSiteChosenObjectDeleted(info.ui_info, *info.object);
 }
 
 void WebsiteSettingsPopupView::OnWidgetDestroying(views::Widget* widget) {
   is_popup_showing = false;
   presenter_->OnUIClosing();
 }
 
 int WebsiteSettingsPopupView::GetDialogButtons() const {
   return ui::DIALOG_BUTTON_NONE;
 }
 
 void WebsiteSettingsPopupView::ButtonPressed(views::Button* button,
                                              const ui::Event& event) {
-  if (button->id() == BUTTON_RESET_CERTIFICATE_DECISIONS)
-    presenter_->OnRevokeSSLErrorBypassButtonPressed();
-  GetWidget()->Close();
+  switch (button->id()) {
+    case BUTTON_RESET_CERTIFICATE_DECISIONS:
+      presenter_->OnRevokeSSLErrorBypassButtonPressed();
+      GetWidget()->Close();
+      break;
+    case BUTTON_SITE_SETTINGS:
+      // TODO(palmer): This opens the general Content Settings pane, which is OK
+      // for now. But on Android, it opens a page specific to a given origin
+      // that shows all of the settings for that origin. If/when that's
+      // available on desktop we should link to that here, too.
+      web_contents()->OpenURL(content::OpenURLParams(
+          GURL(chrome::kChromeUIContentSettingsURL), content::Referrer(),
+          WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK,
+          false));
+      presenter_->RecordWebsiteSettingsAction(
+          WebsiteSettings::WEBSITE_SETTINGS_SITE_SETTINGS_OPENED);
+      break;
+    default:
+      NOTREACHED();
+  }
 }
 
 void WebsiteSettingsPopupView::LinkClicked(views::Link* source,
                                            int event_flags) {
   // The popup closes automatically when the collected cookies dialog or the
   // certificate viewer opens. So delay handling of the link clicked to avoid
   // a crash in the base class which needs to complete the mouse event handling.
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&WebsiteSettingsPopupView::HandleLinkClickedAsync,
                  weak_factory_.GetWeakPtr(), source));
 }
 
 gfx::Size WebsiteSettingsPopupView::GetPreferredSize() const {
   if (header_ == nullptr && site_settings_view_ == nullptr)
     return views::View::GetPreferredSize();
 
   int height = 0;
   if (header_)
     height += header_->GetPreferredSize().height() + kHeaderMarginBottom;
   if (separator_)
     height += separator_->GetPreferredSize().height();
 
   if (site_settings_view_)
     height += site_settings_view_->GetPreferredSize().height();
 
-  int width = kSiteSettingsViewContentMinWidth;
-  if (site_data_content_)
-    width = std::max(width, site_data_content_->GetPreferredSize().width());
-  if (permissions_content_)
-    width = std::max(width, permissions_content_->GetPreferredSize().width());
-  if (header_)
-    width = std::max(width, header_->GetPreferredNameWidth());
-  width += kSiteSettingsViewPaddingLeft + kSiteSettingsViewPaddingRight;
+  int width = kMinPopupWidth;

[msw, 2016/10/08 00:21:57]

q: just checking; should this consider the |cookies_view_|, |permissions_view_|,
or |header_| preferred widths?

[lgarron, 2016/10/08 03:50:04]

cookies_view and permissions_view_: handled through site_settings_view_
header_: Not taken into account for preferred width (see
https://codereview.chromium.org/2400883002/ to make sure it reflows when the
bubble changes size).

[msw, 2016/10/08 16:15:25]

Acknowledged.

+  if (site_settings_view_)
+    width = std::max(width, site_settings_view_->GetPreferredSize().width());
   width = std::min(width, kMaxPopupWidth);
   return gfx::Size(width, height);
 }
 
 void WebsiteSettingsPopupView::SetCookieInfo(
     const CookieInfoList& cookie_info_list) {
   // |cookie_info_list| should only ever have 2 items: first- and third-party
   // cookies.
   DCHECK_EQ(cookie_info_list.size(), 2u);
-  base::string16 first_party_label_text;
-  base::string16 third_party_label_text;
-  for (const auto& i : cookie_info_list) {
-    if (i.is_first_party) {
-      first_party_label_text = l10n_util::GetPluralStringFUTF16(
-          IDS_WEBSITE_SETTINGS_FIRST_PARTY_SITE_DATA, i.allowed);
-    } else {
-      third_party_label_text = l10n_util::GetPluralStringFUTF16(
-          IDS_WEBSITE_SETTINGS_THIRD_PARTY_SITE_DATA, i.allowed);
-    }
-  }
+  int total_allowed = 0;
+  for (const auto& i : cookie_info_list)
+    total_allowed += i.allowed;
+  base::string16 label_text = l10n_util::GetPluralStringFUTF16(
+      IDS_WEBSITE_SETTINGS_NUM_COOKIES, total_allowed);
 
   if (!cookie_dialog_link_) {
-    cookie_dialog_link_ = new views::Link(first_party_label_text);
+    cookie_dialog_link_ = new views::Link(label_text);
     cookie_dialog_link_->set_listener(this);
   } else {
-    cookie_dialog_link_->SetText(first_party_label_text);
+    cookie_dialog_link_->SetText(label_text);
   }
 
   views::GridLayout* layout =
-      static_cast<views::GridLayout*>(site_data_content_->GetLayoutManager());
+      static_cast<views::GridLayout*>(cookies_view_->GetLayoutManager());
   if (!layout) {
-    layout = new views::GridLayout(site_data_content_);
-    site_data_content_->SetLayoutManager(layout);
+    layout = new views::GridLayout(cookies_view_);
+    cookies_view_->SetLayoutManager(layout);
 
-    const int site_data_content_column = 0;
-    views::ColumnSet* column_set =
-        layout->AddColumnSet(site_data_content_column);
-    column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
-                          views::GridLayout::FIXED, kSiteDataIconColumnWidth,
-                          0);
-    column_set->AddPaddingColumn(0, kIconMarginLeft);
-    column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
-                          views::GridLayout::USE_PREF, 0, 0);
-    // No padding. This third column is for |third_party_label_text| (see
-    // below),
-    // and the text needs to flow naturally from the |first_party_label_text|
-    // link.
-    column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
-                          views::GridLayout::USE_PREF, 0, 0);
+    const int cookies_view_column = 0;
+    views::ColumnSet* column_set = layout->AddColumnSet(cookies_view_column);
+    column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 0,
+                          views::GridLayout::FIXED, kPermissionImageSize, 0);
+    column_set->AddPaddingColumn(0, kPermissionImageSpacing);
+    column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL,
+                          0, views::GridLayout::USE_PREF, 0, 0);
 
-    layout->AddPaddingRow(1, 5);
+    layout->AddPaddingRow(0, kCookiesViewVerticalPadding);
 
-    layout->StartRow(1, site_data_content_column);
+    layout->StartRow(1, cookies_view_column);
     WebsiteSettingsUI::PermissionInfo info;
     info.type = CONTENT_SETTINGS_TYPE_COOKIES;
     info.setting = CONTENT_SETTING_ALLOW;
     info.is_incognito =
         Profile::FromBrowserContext(web_contents()->GetBrowserContext())
             ->IsOffTheRecord();
     views::ImageView* icon = new views::ImageView();
     const gfx::Image& image = WebsiteSettingsUI::GetPermissionIcon(info);
     icon->SetImage(image.ToImageSkia());
-    layout->AddView(icon, 1, 1, views::GridLayout::CENTER,
-                    views::GridLayout::CENTER);
-    layout->AddView(cookie_dialog_link_, 1, 1, views::GridLayout::CENTER,
-                    views::GridLayout::CENTER);
-    base::string16 comma = base::ASCIIToUTF16(", ");
+    layout->AddView(
+        icon, 1, 2, views::GridLayout::FILL,
+        // TODO: The vertical alignment may change to CENTER once Harmony is
+        // implemented. See https://crbug.com/512442#c48
+        views::GridLayout::LEADING);
 
-    layout->AddView(new views::Label(comma + third_party_label_text), 1, 1,
-                    views::GridLayout::LEADING, views::GridLayout::CENTER);
+    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+    const gfx::FontList& font_list = rb.GetFontListWithDelta(1);
+    views::Label* cookies_label = new views::Label(
+        l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_TITLE_SITE_DATA),
+        font_list);
+    layout->AddView(cookies_label);
+    layout->StartRow(1, cookies_view_column);
+    layout->SkipColumns(1);
 
-    layout->AddPaddingRow(1, 6);
+    layout->AddView(cookie_dialog_link_);
+
+    layout->AddPaddingRow(0, kCookiesViewVerticalPadding);
   }
 
-  layout->Layout(site_data_content_);
+  layout->Layout(cookies_view_);
   SizeToContents();
 }
 
 void WebsiteSettingsPopupView::SetPermissionInfo(
     const PermissionInfoList& permission_info_list,
     const ChosenObjectInfoList& chosen_object_info_list) {
   // When a permission is changed, WebsiteSettings::OnSitePermissionChanged()
   // calls this method with updated permissions. However, PermissionSelectorRow
   // will have already updated its state, so it's already reflected in the UI.
   // In addition, if a permission is set to the default setting, WebsiteSettings
   // removes it from |permission_info_list|, but the button should remain.
-  if (permissions_content_) {
+  if (permissions_view_) {
     base::STLDeleteContainerPointers(chosen_object_info_list.begin(),
                                      chosen_object_info_list.end());
     return;
   }
 
-  permissions_content_ = new views::View();
-  views::GridLayout* layout = new views::GridLayout(permissions_content_);
-  permissions_content_->SetLayoutManager(layout);
+  permissions_view_ = new views::View();
+  views::GridLayout* layout = new views::GridLayout(permissions_view_);
+  permissions_view_->SetLayoutManager(layout);
 
-  base::string16 headline =
-      permission_info_list.empty()
-          ? base::string16()
-          : l10n_util::GetStringUTF16(
-                IDS_WEBSITE_SETTINGS_TITLE_SITE_PERMISSIONS);
-  views::View* permissions_section =
-      CreateSection(headline, permissions_content_, nullptr);
-  site_settings_view_->AddChildView(permissions_section);
+  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);
   for (const auto& permission : permission_info_list) {
     layout->StartRow(1, content_column);
     PermissionSelectorRow* selector = new PermissionSelectorRow(
         web_contents() ? web_contents()->GetVisibleURL() : GURL::EmptyGURL(),
         permission);
     selector->AddObserver(this);
-    layout->AddView(selector,
-                    1,
-                    1,
-                    views::GridLayout::LEADING,
+    layout->AddView(selector, 1, 1, views::GridLayout::FILL,
                     views::GridLayout::CENTER);
-    layout->AddPaddingRow(1, kContentRowSpacing);
+    layout->AddPaddingRow(1, kPermissionsVerticalSpacing);
   }
 
   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(base::WrapUnique(object));
     object_view->AddObserver(this);
     layout->AddView(object_view, 1, 1, views::GridLayout::LEADING,
                     views::GridLayout::CENTER);
-    layout->AddPaddingRow(1, kContentRowSpacing);
+    layout->AddPaddingRow(1, kPermissionsVerticalSpacing);
   }
 
-  layout->Layout(permissions_content_);
+  layout->Layout(permissions_view_);
 
   // Add site settings link.
-  site_settings_link_ = new views::Link(
+  views::Link* site_settings_link = new views::Link(
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SITE_SETTINGS_LINK));
-  site_settings_link_->set_listener(this);
+  site_settings_link->set_listener(this);
   views::View* link_section = new views::View();
   const int kLinkMarginTop = 4;
-  link_section->SetLayoutManager(
-      new views::BoxLayout(views::BoxLayout::kHorizontal,
-                           kSiteSettingsViewPaddingLeft, kLinkMarginTop, 0));
-  link_section->AddChildView(site_settings_link_);
+  link_section->SetLayoutManager(new views::BoxLayout(
+      views::BoxLayout::kHorizontal, 0, kLinkMarginTop, 0));
+  link_section->AddChildView(site_settings_link);
   site_settings_view_->AddChildView(link_section);
 
   SizeToContents();
 }
 
 void WebsiteSettingsPopupView::SetIdentityInfo(
     const IdentityInfo& identity_info) {
   std::unique_ptr<WebsiteSettingsUI::SecurityDescription> security_description =
       identity_info.GetSecurityDescription();
 
@@ skipping 12 matching lines @@
 
   Layout();
   SizeToContents();
 }
 
 void WebsiteSettingsPopupView::SetSelectedTab(TabId tab_id) {
   // TODO(lgarron): Remove this method. (https://crbug.com/571533)
 }
 
 views::View* WebsiteSettingsPopupView::CreateSiteSettingsView() {
-  views::View* pane = new views::View();
-  pane->SetLayoutManager(
-      new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 1));
+  views::View* site_settings_view = new views::View();

[msw, 2016/10/08 00:21:57]

aside: it'd be nice to avoid this container (but maybe that makes layout tough),
and it'd be nice to refactor some of the |cookies_view_| nit, but don't go out
of your way any further in this CL, I've already caused you too much trouble.
Anyway, just something to consider for followup.

[lgarron, 2016/10/08 03:50:04]

Ack.

+  views::BoxLayout* box_layout =
+      new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0);
+  site_settings_view->SetLayoutManager(box_layout);
+  box_layout->set_cross_axis_alignment(
+      views::BoxLayout::CROSS_AXIS_ALIGNMENT_STRETCH);
+  box_layout->set_inside_border_insets(
+      gfx::Insets(0, kSectionPaddingHorizontal));
 
-  // Add cookies and site data section.
-  site_data_content_ = new views::View();
-  views::View* site_data_section = CreateSection(
-      l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_TITLE_SITE_DATA),
-      site_data_content_, nullptr);
-  pane->AddChildView(site_data_section);
+  // Add cookies view.
+  cookies_view_ = new views::View();
+  site_settings_view->AddChildView(cookies_view_);
 
-  return pane;
+  return site_settings_view;
 }
-views::View* WebsiteSettingsPopupView::CreateSection(
-    const base::string16& headline_text,
-    views::View* content,
-    views::Link* link) {
-  views::View* container = new views::View();
-  views::GridLayout* layout = new views::GridLayout(container);
-  container->SetLayoutManager(layout);
-  const int content_column = 0;
-  views::ColumnSet* column_set = layout->AddColumnSet(content_column);
-  column_set->AddPaddingColumn(0, kSiteSettingsViewPaddingLeft);
-  column_set->AddColumn(views::GridLayout::FILL,
-                        views::GridLayout::FILL,
-                        1,
-                        views::GridLayout::USE_PREF,
-                        0,
-                        0);
-
-  if (headline_text.length() > 0) {
-    layout->AddPaddingRow(1, kSiteSettingsViewPaddingTop);
-    layout->StartRow(1, content_column);
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    views::Label* headline = new views::Label(
-        headline_text, rb.GetFontList(ui::ResourceBundle::BoldFont));
-    layout->AddView(headline, 1, 1, views::GridLayout::LEADING,
-                    views::GridLayout::CENTER);
-  }
-
-  layout->AddPaddingRow(1, kSiteSettingsViewHeadlineMarginBottom);
-  layout->StartRow(1, content_column);
-  layout->AddView(content, 1, 1, views::GridLayout::LEADING,
-                  views::GridLayout::CENTER);
-
-  if (link) {
-    layout->AddPaddingRow(1, 4);
-    layout->StartRow(1, content_column);
-    layout->AddView(link, 1, 1, views::GridLayout::LEADING,
-                    views::GridLayout::CENTER);
-  }
-
-  layout->AddPaddingRow(1, kSiteSettingsViewPaddingBottom);
-  return container;
-}
-
 
 void WebsiteSettingsPopupView::HandleLinkClickedAsync(views::Link* source) {
   if (web_contents() == nullptr || web_contents()->IsBeingDestroyed())
     return;
-
-  if (source == cookie_dialog_link_) {
-    // Count how often the Collected Cookies dialog is opened.
-    presenter_->RecordWebsiteSettingsAction(
-        WebsiteSettings::WEBSITE_SETTINGS_COOKIES_DIALOG_OPENED);
-
-    new CollectedCookiesViews(web_contents());
-  } else if (source == site_settings_link_) {
-    // TODO(palmer): This opens the general Content Settings pane, which is OK
-    // for now. But on Android, it opens a page specific to a given origin that
-    // shows all of the settings for that origin. If/when that's available on
-    // desktop we should link to that here, too.
-    web_contents()->OpenURL(content::OpenURLParams(
-        GURL(chrome::kChromeUIContentSettingsURL), content::Referrer(),
-        WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK,
-        false));
-    presenter_->RecordWebsiteSettingsAction(
-        WebsiteSettings::WEBSITE_SETTINGS_SITE_SETTINGS_OPENED);
-  } else {
-    NOTREACHED();
-  }
+  DCHECK_EQ(source, cookie_dialog_link_);
+  // Count how often the Collected Cookies dialog is opened.
+  presenter_->RecordWebsiteSettingsAction(
+      WebsiteSettings::WEBSITE_SETTINGS_COOKIES_DIALOG_OPENED);
+  new CollectedCookiesViews(web_contents());
 }
 
 void WebsiteSettingsPopupView::StyledLabelLinkClicked(views::StyledLabel* label,
                                                       const gfx::Range& range,
                                                       int event_flags) {
   presenter_->RecordWebsiteSettingsAction(
       WebsiteSettings::WEBSITE_SETTINGS_SECURITY_DETAILS_OPENED);
 
   if (is_devtools_disabled_) {
     DCHECK(certificate_);
     gfx::NativeWindow parent =
         anchor_widget() ? anchor_widget()->GetNativeWindow() : nullptr;
     presenter_->RecordWebsiteSettingsAction(
         WebsiteSettings::WEBSITE_SETTINGS_CERTIFICATE_DIALOG_OPENED);
     ShowCertificateViewer(web_contents(), parent, certificate_.get());
   } else {
     DevToolsWindow::OpenDevToolsWindow(
         web_contents(), DevToolsToggleAction::ShowSecurityPanel());
   }
 }