Add a Certificate Viewer link to the Page Info dropdown

chrome/browser/ui/views/page_info/page_info_bubble_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/page_info/page_info_bubble_view.h"
 
 #include <stddef.h>
 
 #include <algorithm>
-#include <vector>
+#include <utility>
 
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/certificate_viewer.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 61 matching lines @@
 
 // Margin and padding values for the |BubbleHeaderView|.
 const int kHeaderMarginBottom = 10;
 const int kHeaderPaddingBottom = 13;
 
 // Spacing between labels in the header.
 const int kHeaderLabelSpacing = 4;
 
 // Site Settings Section -------------------------------------------------------
 
-// Spacing above and below the cookies view.
-const int kCookiesViewVerticalPadding = 6;
+// Spacing above and below the cookies and certificate views.
+const int kSubViewsVerticalPadding = 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;
+const int LINK_CERTIFICATE_VIEWER = 1342;
 
 // The default, ui::kTitleFontSizeDelta, is too large for the page info
 // bubble (e.g. +3). Use +1 to obtain a smaller font.
 constexpr int kSummaryFontSizeDelta = 1;
 
 // Adds a ColumnSet on |layout| with a single View column and padding columns
 // on either side of it with |margin| width.
 void AddColumnWithSideMargin(views::GridLayout* layout, int margin, int id) {
   views::ColumnSet* column_set = layout->AddColumnSet(id);
   column_set->AddPaddingColumn(0, margin);
@@ skipping 33 matching lines @@
 
   // A container for the styled label with a link for resetting cert decisions.
   // 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(BubbleHeaderView);
 };
 
+// |InspectLinkView| is a UI element (view) that shows information and
+// a link to more details. It is used for the Certificate and Cookie
+// subviews.
+class InspectLinkView : public views::View {
+ public:
+  InspectLinkView(const gfx::Image& image_icon,
+                  const base::string16& title,
+                  const base::string16& text_link,
+                  views::LinkListener* link_listener,
+                  const int link_id);
+  ~InspectLinkView() override;
+
+  // Update the text of the link.
+  void SetLinkText(const base::string16& text_link);
+
+  // Update the tooltip of the link.
+  void SetLinkTooltip(const base::string16& text_tooltip);
+
+ private:
+  views::Link* link_inspect_;
+
+  DISALLOW_COPY_AND_ASSIGN(InspectLinkView);
+};
+
 // The regular PageInfoBubbleView is not supported for internal Chrome pages and
 // extension pages. Instead of the |PageInfoBubbleView|, the
 // |InternalPageInfoBubbleView| is displayed.
 class InternalPageInfoBubbleView : 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.
   InternalPageInfoBubbleView(views::View* anchor_view,
                              gfx::NativeView parent_window,
                              const GURL& url);
@@ skipping 98 matching lines @@
   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::CreateEmptyBorder(8, 0, 0, 0));
 
   InvalidateLayout();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// InspectLinkView
+////////////////////////////////////////////////////////////////////////////////
+
+// |InspectLinkView| is a UI element (view) that shows information and

[msw, 2017/04/27 19:47:55]

nit: remove copied comment, the class comment above is sufficient

[elawrence, 2017/04/27 21:44:25]

Done.

+// a link to more details. It is used for the Certificate and Cookie
+// information.
+InspectLinkView::InspectLinkView(const gfx::Image& image_icon,
+                                 const base::string16& title,
+                                 const base::string16& text_link,
+                                 views::LinkListener* link_listener,
+                                 const int link_id) {
+  views::GridLayout* layout = new views::GridLayout(this);
+  SetLayoutManager(layout);
+
+  const int column = 0;
+  views::ColumnSet* column_set = layout->AddColumnSet(column);
+  column_set->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 0,
+                        views::GridLayout::FIXED, kPermissionIconColumnWidth,
+                        0);
+  column_set->AddPaddingColumn(0, kPermissionImageSpacing);
+  column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL, 0,
+                        views::GridLayout::USE_PREF, 0, 0);
+
+  layout->AddPaddingRow(0, kSubViewsVerticalPadding);
+
+  layout->StartRow(1, column);
+
+  views::ImageView* icon = new NonAccessibleImageView();
+  icon->SetImage(image_icon.ToImageSkia());
+  layout->AddView(
+      icon, 1, 2, views::GridLayout::FILL,
+      // TODO(lgarron): The vertical alignment may change to CENTER once
+      // Harmony is implemented. See https://crbug.com/512442#c48
+      views::GridLayout::LEADING);
+
+  views::Label* title_label = new views::Label(title, CONTEXT_BODY_TEXT_LARGE);
+  layout->AddView(title_label);
+  layout->StartRow(1, column);
+  layout->SkipColumns(1);
+
+  link_inspect_ = new views::Link(text_link);
+  link_inspect_->set_id(link_id);
+  link_inspect_->set_listener(link_listener);
+  layout->AddView(link_inspect_);
+}
+
+InspectLinkView::~InspectLinkView() {}
+
+void InspectLinkView::SetLinkText(const base::string16& text_link) {
+  link_inspect_->SetText(text_link);
+  InvalidateLayout();

[msw, 2017/04/27 19:47:55]

q: Is this needed? Perhaps GridLayout just does the right thing without this?

[elawrence, 2017/04/27 21:44:25]

Apparently it is not needed.

[elawrence, 2017/04/28 17:00:48]

Upon retesting on Windows Views, we actually do need to trigger a layout here.
Without one, the Cookies section shows "44 in ..." because the label isn't made
wider as the number increments. 

Calling Layout() before SizeToContents() is the pattern used elsewhere, so we'll
use that.

+}
+
+void InspectLinkView::SetLinkTooltip(const base::string16& text_tooltip) {
+  link_inspect_->SetTooltipText(text_tooltip);
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // InternalPageInfoBubbleView
 ////////////////////////////////////////////////////////////////////////////////
 
 InternalPageInfoBubbleView::InternalPageInfoBubbleView(
     views::View* anchor_view,
     gfx::NativeView parent_window,
     const GURL& url)
     : BubbleDialogDelegateView(anchor_view, views::BubbleBorder::TOP_LEFT) {
   g_shown_bubble_type = PageInfoBubbleView::BUBBLE_INTERNAL_PAGE;
   set_parent_window(parent_window);
@@ skipping 91 matching lines @@
     Profile* profile,
     content::WebContents* web_contents,
     const GURL& url,
     const security_state::SecurityInfo& security_info)
     : content::WebContentsObserver(web_contents),
       BubbleDialogDelegateView(anchor_view, views::BubbleBorder::TOP_LEFT),
       profile_(profile),
       header_(nullptr),
       separator_(nullptr),
       site_settings_view_(nullptr),
+      certificate_view_(nullptr),
       cookies_view_(nullptr),
-      cookie_dialog_link_(nullptr),
       permissions_view_(nullptr),
       weak_factory_(this) {
   g_shown_bubble_type = BUBBLE_PAGE_INFO;
   set_parent_window(parent_window);
 
   // 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));
 
   // Capture the default bubble margin, and move it to the Layout classes. This
@@ skipping 130 matching lines @@
 void PageInfoBubbleView::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);
   int total_allowed = 0;
   for (const auto& i : cookie_info_list)
     total_allowed += i.allowed;
   base::string16 label_text = l10n_util::GetPluralStringFUTF16(
       IDS_PAGE_INFO_NUM_COOKIES, total_allowed);
 
-  if (!cookie_dialog_link_) {
-    cookie_dialog_link_ = new views::Link(label_text);
-    cookie_dialog_link_->set_id(LINK_COOKIE_DIALOG);
-    cookie_dialog_link_->set_listener(this);
-  } else {
-    cookie_dialog_link_->SetText(label_text);
-  }
-
-  views::GridLayout* layout =
-      static_cast<views::GridLayout*>(cookies_view_->GetLayoutManager());
-  if (!layout) {
-    layout = new views::GridLayout(cookies_view_);
-    cookies_view_->SetLayoutManager(layout);
-
-    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, kPermissionIconColumnWidth,
-                          0);
-    column_set->AddPaddingColumn(0, kPermissionImageSpacing);
-    column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL,
-                          0, views::GridLayout::USE_PREF, 0, 0);
-
-    layout->AddPaddingRow(0, kCookiesViewVerticalPadding);
-
-    layout->StartRow(1, cookies_view_column);
-    PageInfoUI::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 NonAccessibleImageView();
-    const gfx::Image& image = PageInfoUI::GetPermissionIcon(info);
-    icon->SetImage(image.ToImageSkia());
-    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);
-
-    views::Label* cookies_label = new views::Label(
-        l10n_util::GetStringUTF16(IDS_PAGE_INFO_TITLE_SITE_DATA),
-        CONTEXT_BODY_TEXT_LARGE);
-    layout->AddView(cookies_label);
-    layout->StartRow(1, cookies_view_column);
-    layout->SkipColumns(1);
-
-    layout->AddView(cookie_dialog_link_);
-
-    layout->AddPaddingRow(0, kCookiesViewVerticalPadding);
-  }
-
-  layout->Layout(cookies_view_);
+  cookies_view_->SetLinkText(label_text);
   SizeToContents();
 }
 
 void PageInfoBubbleView::SetPermissionInfo(
     const PermissionInfoList& permission_info_list,
     ChosenObjectInfoList chosen_object_info_list) {
   // When a permission is changed, PageInfo::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, PageInfo
@@ skipping 68 matching lines @@
       identity_info.GetSecurityDescription();
 
   summary_text_ = security_description->summary;
   GetWidget()->UpdateWindowTitle();
 
   if (identity_info.certificate) {
     certificate_ = identity_info.certificate;
 
     if (identity_info.show_ssl_decision_revoke_button)
       header_->AddResetDecisionsLabel();
+
+    if (PageInfoUI::ShouldShowCertificateLink()) {
+      // The text of link to the Certificate Viewer varies depending on the
+      // validity of the Certificate.
+      const base::string16 link_title =
+          l10n_util::GetStringUTF16((identity_info.identity_status ==
+                                     PageInfo::SITE_IDENTITY_STATUS_ERROR)
+                                        ? IDS_PAGE_INFO_CERTIFICATE_INVALID_LINK
+                                        : IDS_PAGE_INFO_CERTIFICATE_VALID_LINK);
+
+      certificate_view_ = new InspectLinkView(
+          PageInfoUI::GetCertificateIcon(),
+          l10n_util::GetStringUTF16(IDS_PAGE_INFO_CERTIFICATE), link_title,
+          this, LINK_CERTIFICATE_VIEWER);
+
+      if (identity_info.identity_status !=
+          PageInfo::SITE_IDENTITY_STATUS_ERROR) {
+        base::string16 issuer = l10n_util::GetStringFUTF16(
+            IDS_PAGE_INFO_CERTIFICATE_VALID_LINK_TOOLTIP,
+            base::UTF8ToUTF16(certificate_->issuer().GetDisplayName()));
+        certificate_view_->SetLinkTooltip(issuer);
+      }
+
+      site_settings_view_->AddChildViewAt(certificate_view_, 0);

[msw, 2017/04/27 19:47:55]

Is SetIdentityInfo potentially called more than once for an open bubble? If so
you'll need to remove and delete the old |certificate_view_| before creating and
adding a new one (or just update the old one?) I also wonder if you should
re-use the prior index in that case (are you sure that |certificate_view_| was
really at index 0?) Or perhaps this should just be done in
CreateSiteSettingsView?

[elawrence, 2017/04/27 21:44:25]

As far as I can tell, no, SetIdentityInfo() is only ever called once per Page
Info bubble. lgarron@ may know something I don't?

Unlike the Cookie subview created in CreateSiteSettingsView, we don't show the
certificate subview on all pages, only those that have a certificate.

+    }
   }
 
   header_->SetDetails(security_description->details);
 
   Layout();
   SizeToContents();
 }
 
 views::View* PageInfoBubbleView::CreateSiteSettingsView(int side_margin) {
   views::View* site_settings_view = new views::View();
   views::BoxLayout* box_layout =
       new views::BoxLayout(views::BoxLayout::kVertical, side_margin, 0, 0);
   site_settings_view->SetLayoutManager(box_layout);
   box_layout->set_cross_axis_alignment(
       views::BoxLayout::CROSS_AXIS_ALIGNMENT_STRETCH);
 
   // Add cookies view.
-  cookies_view_ = new views::View();
+  PageInfoUI::PermissionInfo info;
+  info.type = CONTENT_SETTINGS_TYPE_COOKIES;
+  info.setting = CONTENT_SETTING_ALLOW;
+  info.is_incognito =
+      Profile::FromBrowserContext(web_contents()->GetBrowserContext())
+          ->IsOffTheRecord();
+
+  const base::string16 link_title =
+      l10n_util::GetPluralStringFUTF16(IDS_PAGE_INFO_NUM_COOKIES, 0);
+
+  cookies_view_ = new InspectLinkView(
+      PageInfoUI::GetPermissionIcon(info),
+      l10n_util::GetStringUTF16(IDS_PAGE_INFO_TITLE_SITE_DATA), link_title,
+      this, LINK_COOKIE_DIALOG);
   site_settings_view->AddChildView(cookies_view_);
 
   return site_settings_view;
 }
 
 void PageInfoBubbleView::HandleLinkClickedAsync(views::Link* source) {
-  // Both switch cases require accessing web_contents(), so we check it here.
+  // All switch cases require accessing web_contents(), so we check it here.
   if (web_contents() == nullptr || web_contents()->IsBeingDestroyed())
     return;
   switch (source->id()) {
     case LINK_SITE_SETTINGS:
       // TODO(crbug.com/655876): 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_->RecordPageInfoAction(
           PageInfo::PAGE_INFO_SITE_SETTINGS_OPENED);
       break;
     case LINK_COOKIE_DIALOG:
       // Count how often the Collected Cookies dialog is opened.
       presenter_->RecordPageInfoAction(
           PageInfo::PAGE_INFO_COOKIES_DIALOG_OPENED);
       new CollectedCookiesViews(web_contents());
       break;
+    case LINK_CERTIFICATE_VIEWER: {
+      gfx::NativeWindow top_window = web_contents()->GetTopLevelNativeWindow();
+      if (certificate_ && top_window) {
+        presenter_->RecordPageInfoAction(
+            PageInfo::PAGE_INFO_CERTIFICATE_DIALOG_OPENED);
+        ShowCertificateViewer(web_contents(), top_window, certificate_.get());
+      }
+    } break;

[msw, 2017/04/27 19:47:55]

nit: break on next line (git cl format?)

[elawrence, 2017/04/27 21:44:25]

Yes, running "git cl format" moves the break to this point. Apparently, our
coding style is to put the break inside the brace
(https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements),
so I've made that change.

     default:
       NOTREACHED();
   }
 }
 
 void PageInfoBubbleView::StyledLabelLinkClicked(views::StyledLabel* label,
                                                 const gfx::Range& range,
                                                 int event_flags) {
   switch (label->id()) {
     case STYLED_LABEL_SECURITY_DETAILS:
       web_contents()->OpenURL(content::OpenURLParams(
           GURL(chrome::kPageInfoHelpCenterURL), content::Referrer(),
           WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK,
           false));
       presenter_->RecordPageInfoAction(
           PageInfo::PAGE_INFO_CONNECTION_HELP_OPENED);
       break;
     case STYLED_LABEL_RESET_CERTIFICATE_DECISIONS:
       presenter_->OnRevokeSSLErrorBypassButtonPressed();
       GetWidget()->Close();
       break;
     default:
       NOTREACHED();
   }
 }