Add a Certificate Viewer link to the Page Info dropdown

chrome/browser/ui/views/page_info/page_info_bubble_view.cc

Index: chrome/browser/ui/views/page_info/page_info_bubble_view.cc
diff --git a/chrome/browser/ui/views/page_info/page_info_bubble_view.cc b/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
index 12777bad3ac4687db742565dd5eea693085828a5..6cad7272b5d369b7d344462e9f1f41009051ebe1 100644
--- a/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
+++ b/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
@@ -7,7 +7,7 @@
 #include <stddef.h>
 
 #include <algorithm>
-#include <vector>
+#include <utility>
 
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
@@ -89,8 +89,8 @@ 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;
@@ -107,6 +107,7 @@ 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.
@@ -160,6 +161,30 @@ class BubbleHeaderView : public views::View {
   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.
@@ -278,6 +303,60 @@ void BubbleHeaderView::AddResetDecisionsLabel() {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// InspectLinkView
+////////////////////////////////////////////////////////////////////////////////
+
+InspectLinkView::InspectLinkView(const gfx::Image& image_icon,

[Evan Stade, 2017/04/27 21:58:04]

I would suggest not creating this new class. It doesn't (nor does it need to)
override any methods of View. Instead most of it can be accomplished via a
factory function. You can get rid of several parameters and both methods by
creating, storing a reference to, and passing in the link separately.

[elawrence, 2017/04/27 22:12:49]

I may not understand exactly what sort of factory function you have in mind.
Would it just return a regular View which doesn't contain a link as a member
field? And then the link would be manipulated separately?

The new subview was requested by lgarron@ in his original review as a means of
cleaning up the code and providing some level of organization. As someone not
very familiar with the Views architecture, the result of its cleanup seemed
pretty good to me.

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

Removed in the latest revision.

+                                 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);
+}
+
+void InspectLinkView::SetLinkTooltip(const base::string16& text_tooltip) {
+  link_inspect_->SetTooltipText(text_tooltip);
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // InternalPageInfoBubbleView
 ////////////////////////////////////////////////////////////////////////////////
 
@@ -389,8 +468,8 @@ PageInfoBubbleView::PageInfoBubbleView(
       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;
@@ -541,60 +620,7 @@ void PageInfoBubbleView::SetCookieInfo(const CookieInfoList& cookie_info_list) {
   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();
 }
 
@@ -683,6 +709,31 @@ void PageInfoBubbleView::SetIdentityInfo(const IdentityInfo& identity_info) {
 
     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);
+    }
   }
 
   header_->SetDetails(security_description->details);
@@ -700,14 +751,27 @@ views::View* PageInfoBubbleView::CreateSiteSettingsView(int side_margin) {
       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()) {
@@ -729,6 +793,15 @@ void PageInfoBubbleView::HandleLinkClickedAsync(views::Link* source) {
           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;
+    }
     default:
       NOTREACHED();
   }