Material Page Info (Views, 2/3): Update security 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 29 matching lines @@
 #include "extensions/common/constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/image/image.h"
 #include "ui/resources/grit/ui_resources.h"
+#include "ui/views/border.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/md_text_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/styled_label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/layout/layout_manager.h"
@@ skipping 13 matching lines @@
 
 // Left icon margin.
 const int kIconMarginLeft = 6;
 
 // Margin and padding values for the |PopupHeaderView|.
 const int kHeaderMarginBottom = 10;
 const int kHeaderPaddingBottom = 16;
 const int kHeaderPaddingLeft = 18;
 const int kHeaderPaddingRightForCloseButton = 8;
 const int kHeaderPaddingRightForText = kHeaderPaddingLeft;
-const int kHeaderPaddingTop = 12;
+const int kHeaderPaddingTop = 16;
 
-// Spacing between the site identity label and the site identity status text in
-// the popup header.
-const int kHeaderRowSpacing = 4;
+// Spacing between labels in the header.
+const int kHeaderLabelSpacing = 4;
 
 // The max possible width of the popup.
 const int kMaxPopupWidth = 1000;
 
 // The margins between the popup border and the popup content.
-const int kPopupMarginTop = 4;
-const int kPopupMarginLeft = 0;
 const int kPopupMarginBottom = 14;
-const int kPopupMarginRight = 0;
 
 // 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;
 
 // 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;
 
 const int kSiteDataIconColumnWidth = 20;
 
 const int BUTTON_RESET_CERTIFICATE_DECISIONS = 1337;
 
 }  // 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 name of the site's identity.
-  void SetIdentityName(const base::string16& name);
+  // Sets the security summary for the current page.
+  void SetSummary(const base::string16& summary_text);
 
-  // Sets the security summary text for the current page.
-  void SetSecuritySummary(const base::string16& security_summary_text,
-                          bool include_details_link);
+  // Sets the security details for the current page.
+  void SetDetails(const base::string16& details_text,
+                  bool include_details_link);
 
   int GetPreferredNameWidth() const;
 
-  void AddResetDecisionsButton();
+  void AddResetDecisionsLabel();
 
  private:
-  // The label that displays the name of the site's identity.
-  views::Label* name_;
+  // 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.
   // Includes a link to open the DevTools Security panel.
-  views::StyledLabel* status_;
+  views::StyledLabel* details_label_;
 
-  // The button listener attached to the buttons in this view.
-  views::ButtonListener* button_listener_;
-
-  // A container for the button for resetting cert decisions. The button is only
-  // shown sometimes, so we use a container to keep track of where to place it
-  // (if needed).
-  views::View* reset_decisions_button_container_;
+  // A container for the styled label with link for resetting cert decisions.

[msw, 2016/09/10 01:51:26]

nit: "a link"

[lgarron, 2016/09/10 02:08:10]

Done.

+  // This is only shown sometimes, so we use a container to keep track of
+  // where to place it (if needed).
+  views::View* reset_decisions_label_container_;
+  views::StyledLabel* reset_decisions_label_;
 
   DISALLOW_COPY_AND_ASSIGN(PopupHeaderView);
 };
 
 // Website Settings are not supported for internal Chrome pages and extension
 // pages. Instead of the |WebsiteSettingsPopupView|, the
 // |InternalPageInfoPopupView| is displayed.
 class InternalPageInfoPopupView : public views::BubbleDialogDelegateView {
  public:
   // If |anchor_view| is nullptr, or has no Widget, |parent_window| may be
   // provided to ensure this bubble is closed when the parent closes.
   InternalPageInfoPopupView(views::View* anchor_view,
                             gfx::NativeView parent_window,
                             const GURL& url);
   ~InternalPageInfoPopupView() override;
 
   // views::BubbleDialogDelegateView:
   views::NonClientFrameView* CreateNonClientFrameView(
       views::Widget* widget) override;
   void OnWidgetDestroying(views::Widget* widget) override;
-  int GetDialogButtons() const override;
 
  private:
   friend class WebsiteSettingsPopupView;
 
   DISALLOW_COPY_AND_ASSIGN(InternalPageInfoPopupView);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // Popup Header
 ////////////////////////////////////////////////////////////////////////////////
 
 PopupHeaderView::PopupHeaderView(
     views::ButtonListener* button_listener,
     views::StyledLabelListener* styled_label_listener)
-    : name_(nullptr),
-      status_(nullptr),
-      button_listener_(button_listener),
-      reset_decisions_button_container_(nullptr) {
+    : 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->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, kHeaderPaddingRightForCloseButton);
 
-  layout->AddPaddingRow(0, kHeaderPaddingTop);
+  // 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, kHeaderPaddingRightForCloseButton);

[msw, 2016/09/10 01:51:26]

nit: If the value is a height, using kHeaderPaddingRight* doesn't seem correct.

[lgarron, 2016/09/10 02:08:10]

Changed to kHeaderPaddingForCloseButton.

+
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
 
   layout->StartRow(0, label_column);
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  name_ = new views::Label(
-      base::string16(), rb.GetFontList(ui::ResourceBundle::BoldFont));
-  layout->AddView(name_, 1, 1, views::GridLayout::LEADING,
+  const gfx::FontList& font_list = rb.GetFontListWithDelta(1);

[msw, 2016/09/10 01:51:26]

nit: I still think it's odd to increase this font size by just 1px. I wonder if
the spec simply cited a random size that happened to be 1px off from the base
font size, instead of explicitly declaring that this label should be 1px taller
than the base size that's used nearly everywhere else? Can you point me to that
spec/decision?

[lgarron, 2016/09/10 02:08:10]

This is based off these mocks [1], this comment [2], and an okay from Max Walker
(the current designer for the project) by saying "Thanks" and picking other nits
at [3].


[1] https://drive.google.com/corp/drive/folders/0B1kg1oK-WfnuU1NJV2xrRFYzRFE
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=512442#c48
[3] https://bugs.chromium.org/p/chromium/issues/detail?id=512442#c55

+  summary_label_ = new views::Label(base::string16(), font_list);
+  summary_label_->SetBorder(views::Border::CreateEmptyBorder(
+      kHeaderPaddingTop - kHeaderPaddingRightForCloseButton, 0, 0, 0));

[msw, 2016/09/10 01:51:26]

ditto nit: it seems weird to subtract 'Top' and 'Right' padding values.

[lgarron, 2016/09/10 02:08:10]

[see above]

+  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, kHeaderRowSpacing);
+  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->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
                                1, views::GridLayout::USE_PREF, 0, 0);
   column_set_status->AddPaddingColumn(0, kHeaderPaddingRightForText);
 
+  layout->AddPaddingRow(0, kHeaderLabelSpacing);
+
   layout->StartRow(0, label_column_status);
-  status_ = new views::StyledLabel(base::string16(), styled_label_listener);
-  layout->AddView(status_,
-                  1,
-                  1,
-                  views::GridLayout::LEADING,
+  details_label_ =
+      new views::StyledLabel(base::string16(), styled_label_listener);
+  layout->AddView(details_label_, 1, 1, views::GridLayout::LEADING,
                   views::GridLayout::LEADING);
 
   layout->StartRow(0, label_column_status);
-  reset_decisions_button_container_ = new views::View();
-  reset_decisions_button_container_->SetLayoutManager(
+  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_button_container_, 1, 1,
+  layout->AddView(reset_decisions_label_container_, 1, 1,
                   views::GridLayout::LEADING, views::GridLayout::LEADING);
 
   layout->AddPaddingRow(1, kHeaderPaddingBottom);
 }
 
 PopupHeaderView::~PopupHeaderView() {}
 
 int PopupHeaderView::GetPreferredNameWidth() const {
-  return name_->GetPreferredSize().width();
+  return summary_label_->GetPreferredSize().width();
 }
 
-void PopupHeaderView::SetIdentityName(const base::string16& name) {
-  name_->SetText(name);
+void PopupHeaderView::SetSummary(const base::string16& summary_text) {
+  summary_label_->SetText(summary_text);
 }
 
-void PopupHeaderView::SetSecuritySummary(
-    const base::string16& security_summary_text,
-    bool include_details_link) {
-  if (include_details_link) {
-    base::string16 details_string =
+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);
 
     std::vector<base::string16> subst;
-    subst.push_back(security_summary_text);
-    subst.push_back(details_string);
+    subst.push_back(details_text);
+    subst.push_back(details_link_text);
 
     std::vector<size_t> offsets;
 
     base::string16 text = base::ReplaceStringPlaceholders(
         base::ASCIIToUTF16("$1 $2"), subst, &offsets);
-    status_->SetText(text);
+    details_label_->SetText(text);
     gfx::Range details_range(offsets[1], text.length());
 
     views::StyledLabel::RangeStyleInfo link_style =
         views::StyledLabel::RangeStyleInfo::CreateForLink();
     if (!ui::MaterialDesignController::IsSecondaryUiMaterial())
       link_style.font_style |= gfx::Font::FontStyle::UNDERLINE;
     link_style.disable_line_wrapping = false;
 
-    status_->AddStyleRange(details_range, link_style);
+    details_label_->AddStyleRange(details_range, link_style);
   } else {
-    status_->SetText(security_summary_text);
+    details_label_->SetText(details_text);
   }
 
   // Fit the styled label to occupy available width.
-  status_->SizeToFit(0);
+  details_label_->SizeToFit(0);
 }
 
-void PopupHeaderView::AddResetDecisionsButton() {
-  // TODO(estade): this looks pretty crazy as an MD button because the button
-  // text is very long. See crbug.com/512442
-  views::LabelButton* reset_decisions_button =
-      views::MdTextButton::CreateSecondaryUiButton(
-          button_listener_,
-          l10n_util::GetStringUTF16(
-              IDS_PAGEINFO_RESET_INVALID_CERTIFICATE_DECISIONS_BUTTON));
-  reset_decisions_button->set_id(BUTTON_RESET_CERTIFICATE_DECISIONS);
+void PopupHeaderView::AddResetDecisionsLabel() {
+  std::vector<base::string16> subst;
+  subst.push_back(
+      l10n_util::GetStringUTF16(IDS_PAGEINFO_INVALID_CERTIFICATE_DESCRIPTION));
+  subst.push_back(l10n_util::GetStringUTF16(
+      IDS_PAGEINFO_RESET_INVALID_CERTIFICATE_DECISIONS_BUTTON));
 
-  reset_decisions_button_container_->AddChildView(reset_decisions_button);
+  std::vector<size_t> offsets;
 
-  // Now that it contains a button, the container needs padding at the top.
-  reset_decisions_button_container_->SetBorder(
+  base::string16 text = base::ReplaceStringPlaceholders(
+      base::ASCIIToUTF16("$1 $2"), subst, &offsets);
+  reset_decisions_label_ = new views::StyledLabel(text, styled_label_listener_);
+  gfx::Range link_range(offsets[1], text.length());
+
+  views::StyledLabel::RangeStyleInfo link_style =
+      views::StyledLabel::RangeStyleInfo::CreateForLink();
+  if (!ui::MaterialDesignController::IsSecondaryUiMaterial())
+    link_style.font_style |= gfx::Font::FontStyle::UNDERLINE;
+  link_style.disable_line_wrapping = false;
+
+  reset_decisions_label_->AddStyleRange(link_range, link_style);
+  // Fit the styled label to occupy available width.
+  reset_decisions_label_->SizeToFit(0);
+  reset_decisions_label_container_->AddChildView(reset_decisions_label_);
+
+  // Now that it contains a label, the container needs padding at the top.
+  reset_decisions_label_container_->SetBorder(
       views::Border::CreateEmptyBorder(8, 0, 0, 0));
 
   InvalidateLayout();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // InternalPageInfoPopupView
 ////////////////////////////////////////////////////////////////////////////////
 
 InternalPageInfoPopupView::InternalPageInfoPopupView(
@@ skipping 48 matching lines @@
   // 16px padding + half of icon width comes out to 24px.
   frame->bubble_border()->set_arrow_offset(
       24 + frame->bubble_border()->GetBorderThickness());
   return frame;
 }
 
 void InternalPageInfoPopupView::OnWidgetDestroying(views::Widget* widget) {
   is_popup_showing = false;
 }
 
-int InternalPageInfoPopupView::GetDialogButtons() const {
-  return ui::DIALOG_BUTTON_NONE;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // WebsiteSettingsPopupView
 ////////////////////////////////////////////////////////////////////////////////
 
 WebsiteSettingsPopupView::~WebsiteSettingsPopupView() {
 }
 
 // static
 void WebsiteSettingsPopupView::ShowPopup(
     views::View* anchor_view,
@@ skipping 72 matching lines @@
   layout->StartRow(0, content_column);
   separator_ = new views::Separator(views::Separator::HORIZONTAL);
   layout->AddView(separator_);
 
   layout->AddPaddingRow(1, kHeaderMarginBottom);
   layout->StartRow(1, content_column);
 
   site_settings_view_ = CreateSiteSettingsView();
   layout->AddView(site_settings_view_);
 
-  set_margins(gfx::Insets(kPopupMarginTop, kPopupMarginLeft,
-                          kPopupMarginBottom, kPopupMarginRight));
+  // Each section handles its own padding.
+  set_margins(gfx::Insets(0, 0, kPopupMarginBottom, 0));
 
   views::BubbleDialogDelegateView::CreateBubble(this);
 
   presenter_.reset(new WebsiteSettings(
       this, profile, TabSpecificContentSettings::FromWebContents(web_contents),
       web_contents, url, security_info));
 }
 
 void WebsiteSettingsPopupView::RenderFrameDeleted(
     content::RenderFrameHost* render_frame_host) {
@@ skipping 212 matching lines @@
       new views::BoxLayout(views::BoxLayout::kHorizontal,
                            kSiteSettingsViewPaddingLeft, kLinkMarginTop, 0));
   link_section->AddChildView(site_settings_link_);
   site_settings_view_->AddChildView(link_section);
 
   SizeToContents();
 }
 
 void WebsiteSettingsPopupView::SetIdentityInfo(
     const IdentityInfo& identity_info) {
-  base::string16 security_summary_text = identity_info.GetSecuritySummary();
-  header_->SetIdentityName(base::UTF8ToUTF16(identity_info.site_identity));
+  std::unique_ptr<WebsiteSettingsUI::SecurityDescription> security_description =
+      identity_info.GetSecurityDescription();
+
+  header_->SetSummary(security_description->summary);
 
   if (identity_info.certificate) {
     certificate_ = identity_info.certificate;
 
     if (identity_info.show_ssl_decision_revoke_button)
-      header_->AddResetDecisionsButton();
+      header_->AddResetDecisionsLabel();
   }
 
   bool include_details_link = !is_devtools_disabled_ || certificate_;
 
-  header_->SetSecuritySummary(security_summary_text, include_details_link);
+  header_->SetDetails(security_description->details, include_details_link);
 
   Layout();
   SizeToContents();
 }
 
 void WebsiteSettingsPopupView::SetSelectedTab(TabId tab_id) {
   // TODO(lgarron): Remove this method. (https://crbug.com/571533)
 }
 
 views::View* WebsiteSettingsPopupView::CreateSiteSettingsView() {
@@ skipping 91 matching lines @@
     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());
   }
 }