Part 2 of the SSL InfoBubble. 2nd try, this time with test fixed.

- Added Certificate Information link.

- Changed iconography to be:
 Identity  : Green check mark vs. Red skull and bones
 Connection: Green padlock vs. Orange exclamation point.
 (This used to be Green checkmark and Orange Exclamation point for both sections.

- Removed the icon for the First visit section. Need to see if we want an icon for when you haven't visited the site before. Mocks show no icons when you have visited before.

BUG=52916
TEST=Open the SSL InfoBubble and notice the new iconography. Click the link and the Certificate dialog should show.

Review URL: http://codereview.chromium.org/3187019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57498 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 123 insertions, 54 deletions

diff --git a/chrome/app/theme/pageinfo_bad.png b/chrome/app/theme/pageinfo_bad.png
index f8be672..08ee500 100644
--- a/chrome/app/theme/pageinfo_bad.png
+++ b/chrome/app/theme/pageinfo_bad.png
diff --git a/chrome/app/theme/pageinfo_good.png b/chrome/app/theme/pageinfo_good.png
index 9c8311e..dcdc611 100644
--- a/chrome/app/theme/pageinfo_good.png
+++ b/chrome/app/theme/pageinfo_good.png
diff --git a/chrome/browser/cocoa/page_info_window_mac_unittest.mm b/chrome/browser/cocoa/page_info_window_mac_unittest.mm
index 4091b44..9454206 100644
--- a/chrome/browser/cocoa/page_info_window_mac_unittest.mm
+++ b/chrome/browser/cocoa/page_info_window_mac_unittest.mm
@@ -21,12 +21,14 @@ class FakeModel : public PageInfoModel {
  public:
   void AddSection(bool state,
                   const string16& title,
-                  const string16& description) {
+                  const string16& description,
+                  SectionInfoType type) {
     sections_.push_back(SectionInfo(
         state,
         title,
         string16(),
-        description));
+        description,
+        type));
   }
 };
 
@@ -113,12 +115,14 @@ TEST_F(PageInfoWindowMacTest, NoHistoryNoSecurity) {
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY,
-          ASCIIToUTF16("google.com")));
+          ASCIIToUTF16("google.com")),
+      PageInfoModel::SECTION_INFO_IDENTITY);
   model_->AddSection(false,
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
-          ASCIIToUTF16("google.com")));
+          ASCIIToUTF16("google.com")),
+      PageInfoModel::SECTION_INFO_CONNECTION);
 
   bridge_->ModelChanged();
 
@@ -131,12 +135,14 @@ TEST_F(PageInfoWindowMacTest, HistoryNoSecurity) {
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY,
-          ASCIIToUTF16("google.com")));
+          ASCIIToUTF16("google.com")),
+      PageInfoModel::SECTION_INFO_IDENTITY);
   model_->AddSection(false,
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
-          ASCIIToUTF16("google.com")));
+          ASCIIToUTF16("google.com")),
+      PageInfoModel::SECTION_INFO_CONNECTION);
 
   // In practice, the history information comes later because it's queried
   // asynchronously, so replicate the double-build here.
@@ -146,7 +152,8 @@ TEST_F(PageInfoWindowMacTest, HistoryNoSecurity) {
       l10n_util::GetStringUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_PERSONAL_HISTORY_TITLE),
       l10n_util::GetStringUTF16(
-          IDS_PAGE_INFO_SECURITY_TAB_FIRST_VISITED_TODAY));
+          IDS_PAGE_INFO_SECURITY_TAB_FIRST_VISITED_TODAY),
+      PageInfoModel::SECTION_INFO_FIRST_VISIT);
 
   bridge_->ModelChanged();
 
@@ -159,7 +166,8 @@ TEST_F(PageInfoWindowMacTest, NoHistoryMixedSecurity) {
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_SECURE_IDENTITY,
-          ASCIIToUTF16("Goat Security Systems")));
+          ASCIIToUTF16("Goat Security Systems")),
+      PageInfoModel::SECTION_INFO_IDENTITY);
 
   // This string is super long and the text should overflow the default clip
   // region (kImageSize).
@@ -174,7 +182,8 @@ TEST_F(PageInfoWindowMacTest, NoHistoryMixedSecurity) {
               ASCIIToUTF16("chrome.google.com"),
               base::IntToString16(1024)),
           l10n_util::GetStringUTF16(
-              IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_INSECURE_CONTENT_WARNING)));
+              IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_INSECURE_CONTENT_WARNING)),
+      PageInfoModel::SECTION_INFO_CONNECTION);
 
   bridge_->ModelChanged();
 
diff --git a/chrome/browser/gtk/page_info_window_gtk.cc b/chrome/browser/gtk/page_info_window_gtk.cc
index 0274b38..87671ae 100644
--- a/chrome/browser/gtk/page_info_window_gtk.cc
+++ b/chrome/browser/gtk/page_info_window_gtk.cc
@@ -157,8 +157,8 @@ GtkWidget* PageInfoWindowGtk::CreateSection(
   gtk_misc_set_alignment(GTK_MISC(image), 0, 0);
 
   GtkWidget* text_box = gtk_vbox_new(FALSE, gtk_util::kControlSpacing);
-  if (!section.head_line.empty()) {
-    label = gtk_label_new(UTF16ToUTF8(section.head_line).c_str());
+  if (!section.headline.empty()) {
+    label = gtk_label_new(UTF16ToUTF8(section.headline).c_str());
     gtk_misc_set_alignment(GTK_MISC(label), 0, 0);
     gtk_box_pack_start(GTK_BOX(text_box), label, FALSE, FALSE, 0);
   }
diff --git a/chrome/browser/page_info_model.cc b/chrome/browser/page_info_model.cc
index cf649d4..ec818fe 100644
--- a/chrome/browser/page_info_model.cc
+++ b/chrome/browser/page_info_model.cc
@@ -99,7 +99,8 @@ PageInfoModel::PageInfoModel(Profile* profile,
       state,
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
       head_line,
-      description));
+      description,
+      SECTION_INFO_IDENTITY));
 
   // Connection section.
   // We consider anything less than 80 bits encryption to be weak encryption.
@@ -139,8 +140,9 @@ PageInfoModel::PageInfoModel(Profile* profile,
   if (ssl.security_bits() > 0 && cipher_suite) {
     bool did_fallback = (ssl.connection_status() &
                          net::SSL_CONNECTION_SSL3_FALLBACK) != 0;
-    bool no_renegotiation = (ssl.connection_status() &
-                             net::SSL_CONNECTION_NO_RENEGOTIATION_EXTENSION) != 0;
+    bool no_renegotiation =
+        (ssl.connection_status() &
+        net::SSL_CONNECTION_NO_RENEGOTIATION_EXTENSION) != 0;
     const char *key_exchange, *cipher, *mac;
     net::SSLCipherSuiteToStrings(&key_exchange, &cipher, &mac, cipher_suite);
 
@@ -181,7 +183,8 @@ PageInfoModel::PageInfoModel(Profile* profile,
       state,
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
       head_line,
-      description));
+      description,
+      SECTION_INFO_CONNECTION));
 
   // Request the number of visits.
   HistoryService* history = profile->GetHistoryService(
@@ -226,7 +229,8 @@ void PageInfoModel::OnGotVisitCountToHost(HistoryService::Handle handle,
             IDS_PAGE_INFO_SECURITY_TAB_PERSONAL_HISTORY_TITLE),
         string16(),
         l10n_util::GetStringUTF16(
-            IDS_PAGE_INFO_SECURITY_TAB_FIRST_VISITED_TODAY)));
+            IDS_PAGE_INFO_SECURITY_TAB_FIRST_VISITED_TODAY),
+        SECTION_INFO_FIRST_VISIT));
   } else {
     sections_.push_back(SectionInfo(
         true,
@@ -235,7 +239,8 @@ void PageInfoModel::OnGotVisitCountToHost(HistoryService::Handle handle,
         string16(),
         l10n_util::GetStringFUTF16(
             IDS_PAGE_INFO_SECURITY_TAB_VISITED_BEFORE_TODAY,
-            WideToUTF16(base::TimeFormatShortDate(first_visit)))));
+            WideToUTF16(base::TimeFormatShortDate(first_visit))),
+        SECTION_INFO_FIRST_VISIT));
   }
   observer_->ModelChanged();
 }
diff --git a/chrome/browser/page_info_model.h b/chrome/browser/page_info_model.h
index 6bf0389..60e3883 100644
--- a/chrome/browser/page_info_model.h
+++ b/chrome/browser/page_info_model.h
@@ -18,7 +18,7 @@ class PrefService;
 class Profile;
 
 // The model that provides the information that should be displayed in the page
-// info dialog.
+// info dialog/bubble.
 class PageInfoModel {
  public:
   class PageInfoModelObserver {
@@ -29,15 +29,23 @@ class PageInfoModel {
     virtual ~PageInfoModelObserver() {}
   };
 
+  enum SectionInfoType {
+    SECTION_INFO_IDENTITY = 0,
+    SECTION_INFO_CONNECTION,
+    SECTION_INFO_FIRST_VISIT,
+  };
+
   struct SectionInfo {
     SectionInfo(bool state,
                 const string16& title,
-                const string16& head_line,
-                const string16& description)
+                const string16& headline,
+                const string16& description,
+                SectionInfoType type)
         : state(state),
           title(title),
-          head_line(head_line),
-          description(description) {
+          headline(headline),
+          description(description),
+          type(type) {
     }
 
     bool state;  // True if state is OK, false otherwise (ex of bad states:
@@ -47,10 +55,14 @@ class PageInfoModel {
     string16 title;
 
     // A single line describing the section, optional.
-    string16 head_line;
+    string16 headline;
 
     // The full description of what this section is.
     string16 description;
+
+    // The type of SectionInfo we are dealing with, for example: Identity,
+    // Connection, First Visit.
+    SectionInfoType type;
   };
 
   PageInfoModel(Profile* profile,
diff --git a/chrome/browser/views/page_info_bubble_view.cc b/chrome/browser/views/page_info_bubble_view.cc
index ac94233c..2273402 100644
--- a/chrome/browser/views/page_info_bubble_view.cc
+++ b/chrome/browser/views/page_info_bubble_view.cc
@@ -4,20 +4,26 @@
 
 #include "chrome/browser/views/page_info_bubble_view.h"
 
+#include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/certificate_viewer.h"
 #include "chrome/browser/views/frame/browser_view.h"
 #include "chrome/browser/views/info_bubble.h"
 #include "chrome/browser/views/toolbar_view.h"
+#include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "grit/theme_resources.h"
 #include "views/controls/image_view.h"
 #include "views/controls/label.h"
+#include "views/controls/link.h"
 #include "views/controls/separator.h"
 #include "views/grid_layout.h"
 #include "views/widget/widget.h"
 #include "views/window/window.h"
 
+namespace {
+
 // Layout constants.
 const int kHGapToBorder = 11;
 const int kVGapToImage = 10;
@@ -30,28 +36,29 @@ const int kIconOffset = 28;
 
 // A section contains an image that shows a status (good or bad), a title, an
 // optional head-line (in bold) and a description.
-class Section : public views::View {
+class Section : public views::View,
+                public views::LinkController {
  public:
-  Section(bool state,
-          const string16& headline,
-          const string16& description);
+  Section(PageInfoBubbleView* owner,
+          const PageInfoModel::SectionInfo& section_info);
   virtual ~Section();
 
+  // views::View methods:
   virtual int GetHeightForWidth(int w);
   virtual void Layout();
 
+  // views::LinkController methods:
+  virtual void LinkActivated(views::Link* source, int event_flags);
+
  private:
   // Calculate the layout if |compute_bounds_only|, otherwise does Layout also.
   gfx::Size LayoutItems(bool compute_bounds_only, int width);
 
-  // Whether to show the good/bad icon.
-  bool state_;
+  // The view that owns this Section object.
+  PageInfoBubbleView* owner_;
 
-  // The first line of the description, show in bold.
-  string16 headline_;
-
-  // The description, displayed below the head line.
-  string16 description_;
+  // The information this view represents.
+  PageInfoModel::SectionInfo info_;
 
   static SkBitmap* good_state_icon_;
   static SkBitmap* bad_state_icon_;
@@ -59,6 +66,7 @@ class Section : public views::View {
   views::ImageView* status_image_;
   views::Label* headline_label_;
   views::Label* description_label_;
+  views::Link* link_;
 
   DISALLOW_COPY_AND_ASSIGN(Section);
 };
@@ -67,15 +75,19 @@ class Section : public views::View {
 SkBitmap* Section::good_state_icon_ = NULL;
 SkBitmap* Section::bad_state_icon_ = NULL;
 
+}  // namespace
+
 ////////////////////////////////////////////////////////////////////////////////
 // PageInfoBubbleView
 
-PageInfoBubbleView::PageInfoBubbleView(Profile* profile,
+PageInfoBubbleView::PageInfoBubbleView(gfx::NativeWindow parent_window,
+                                       Profile* profile,
                                        const GURL& url,
                                        const NavigationEntry::SSLStatus& ssl,
                                        bool show_history)
     : ALLOW_THIS_IN_INITIALIZER_LIST(model_(profile, url, ssl,
                                             show_history, this)),
+      parent_window_(parent_window),
       cert_id_(ssl.cert_id()),
       info_bubble_(NULL) {
   LayoutSections();
@@ -84,6 +96,10 @@ PageInfoBubbleView::PageInfoBubbleView(Profile* profile,
 PageInfoBubbleView::~PageInfoBubbleView() {
 }
 
+void PageInfoBubbleView::ShowCertDialog() {
+  ShowCertificateViewerByID(parent_window_, cert_id_);
+}
+
 void PageInfoBubbleView::LayoutSections() {
   // Remove all the existing sections.
   RemoveAllChildViews(true);
@@ -104,7 +120,7 @@ void PageInfoBubbleView::LayoutSections() {
     layout->StartRow(0, 0);
     // TODO(finnur): Remove title from the info struct, since it is
     //               not used anymore.
-    layout->AddView(new Section(info.state, info.head_line, info.description));
+    layout->AddView(new Section(this, info));
 
     // Add separator after all sections except the last.
     if (i < count - 1) {
@@ -124,7 +140,7 @@ gfx::Size PageInfoBubbleView::GetPreferredSize() {
   int count = model_.GetSectionCount();
   for (int i = 0; i < count; ++i) {
     PageInfoModel::SectionInfo info = model_.GetSectionInfo(i);
-    Section section(info.state, info.head_line, info.description);
+    Section section(this, info);
     size.Enlarge(0, section.GetHeightForWidth(size.width()));
   }
 
@@ -145,35 +161,43 @@ void PageInfoBubbleView::ModelChanged() {
 ////////////////////////////////////////////////////////////////////////////////
 // Section
 
-Section::Section(bool state,
-                 const string16& headline,
-                 const string16& description)
-    : state_(state),
-      headline_(headline),
-      description_(description) {
+Section::Section(PageInfoBubbleView* owner,
+                 const PageInfoModel::SectionInfo& section_info)
+    : owner_(owner),
+      info_(section_info) {
   if (!good_state_icon_) {
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
     good_state_icon_ = rb.GetBitmapNamed(IDR_PAGEINFO_GOOD);
     bad_state_icon_ = rb.GetBitmapNamed(IDR_PAGEINFO_BAD);
   }
 
-  status_image_ = new views::ImageView();
-  status_image_->SetImage(state ? good_state_icon_ : bad_state_icon_);
-  AddChildView(status_image_);
+  if (info_.type == PageInfoModel::SECTION_INFO_IDENTITY ||
+      info_.type == PageInfoModel::SECTION_INFO_CONNECTION) {
+    status_image_ = new views::ImageView();
+    status_image_->SetImage(info_.state ? good_state_icon_ : bad_state_icon_);
+    AddChildView(status_image_);
+  }
 
-  headline_label_ = new views::Label(UTF16ToWideHack(headline));
+  headline_label_ = new views::Label(UTF16ToWideHack(info_.headline));
   headline_label_->SetFont(
       headline_label_->font().DeriveFont(0, gfx::Font::BOLD));
   headline_label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   AddChildView(headline_label_);
 
-  description_label_ = new views::Label(UTF16ToWideHack(description));
+  description_label_ = new views::Label(UTF16ToWideHack(info_.description));
   description_label_->SetMultiLine(true);
   description_label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   // Allow linebreaking in the middle of words if necessary, so that extremely
   // long hostnames (longer than one line) will still be completely shown.
   description_label_->SetAllowCharacterBreak(true);
   AddChildView(description_label_);
+
+  if (info_.type == PageInfoModel::SECTION_INFO_IDENTITY) {
+    link_ = new views::Link(
+        l10n_util::GetStringUTF16(IDS_PAGEINFO_CERT_INFO_BUTTON));
+    link_->SetController(this);
+    AddChildView(link_);
+  }
 }
 
 Section::~Section() {
@@ -187,14 +211,22 @@ void Section::Layout() {
   LayoutItems(false, width());
 }
 
+void Section::LinkActivated(views::Link* source, int event_flags) {
+  owner_->ShowCertDialog();
+}
+
 gfx::Size Section::LayoutItems(bool compute_bounds_only, int width) {
   int x = kHGapToBorder;
   int y = kVGapToImage;
 
   // Layout the image, head-line and description.
-  gfx::Size size = status_image_->GetPreferredSize();
-  if (!compute_bounds_only)
-    status_image_->SetBounds(x, y, size.width(), size.height());
+  gfx::Size size;
+  if (info_.type == PageInfoModel::SECTION_INFO_IDENTITY ||
+    info_.type == PageInfoModel::SECTION_INFO_CONNECTION) {
+      size = status_image_->GetPreferredSize();
+    if (!compute_bounds_only)
+      status_image_->SetBounds(x, y, size.width(), size.height());
+  }
   int image_height = y + size.height();
   x += size.width() + kHGapImageToDescription;
   int w = width - x - kTextPaddingRight;
@@ -217,6 +249,11 @@ gfx::Size Section::LayoutItems(bool compute_bounds_only, int width) {
     if (!compute_bounds_only)
       description_label_->SetBounds(x, y, 0, 0);
   }
+  if (info_.type == PageInfoModel::SECTION_INFO_IDENTITY) {
+    size = link_->GetPreferredSize();
+    link_->SetBounds(x, y, size.width(), size.height());
+    y += size.height();
+  }
 
   // Make sure the image is not truncated if the text doesn't contain much.
   y = std::max(y, image_height);
@@ -242,7 +279,7 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
 
   // Show the bubble.
   PageInfoBubbleView* page_info_bubble =
-      new PageInfoBubbleView(profile, url, ssl, show_history);
+      new PageInfoBubbleView(parent, profile, url, ssl, show_history);
   InfoBubble* info_bubble =
       InfoBubble::Show(browser_view->GetWidget(), bounds,
                        BubbleBorder::TOP_LEFT,
diff --git a/chrome/browser/views/page_info_bubble_view.h b/chrome/browser/views/page_info_bubble_view.h
index ede280b..88aac05 100644
--- a/chrome/browser/views/page_info_bubble_view.h
+++ b/chrome/browser/views/page_info_bubble_view.h
@@ -18,12 +18,15 @@ class PageInfoBubbleView : public views::View,
                            public PageInfoModel::PageInfoModelObserver,
                            public InfoBubbleDelegate {
  public:
-  PageInfoBubbleView(Profile* profile,
+  PageInfoBubbleView(gfx::NativeWindow parent_window,
+                     Profile* profile,
                      const GURL& url,
                      const NavigationEntry::SSLStatus& ssl,
                      bool show_history);
   virtual ~PageInfoBubbleView();
 
+  // Show the certificate dialog.
+  void ShowCertDialog();
 
   void set_info_bubble(InfoBubble* info_bubble) { info_bubble_ = info_bubble; }
 
@@ -44,6 +47,9 @@ class PageInfoBubbleView : public views::View,
   // Layout the sections within the bubble.
   void LayoutSections();
 
+  // The parent window of the InfoBubble showing this view.
+  gfx::NativeWindow parent_window_;
+
   // The model providing the various section info.
   PageInfoModel model_;
 
diff --git a/chrome/browser/views/page_info_window_view.cc b/chrome/browser/views/page_info_window_view.cc
index b962acf..2432a93 100644
--- a/chrome/browser/views/page_info_window_view.cc
+++ b/chrome/browser/views/page_info_window_view.cc
@@ -217,7 +217,7 @@ void PageInfoWindowView::LayoutSections() {
   for (int i = 0; i < model_.GetSectionCount(); ++i) {
     PageInfoModel::SectionInfo info = model_.GetSectionInfo(i);
     layout->StartRow(0, 0);
-    layout->AddView(new Section(info.title, info.state, info.head_line,
+    layout->AddView(new Section(info.title, info.state, info.headline,
                                 info.description));
     layout->AddPaddingRow(0, kVerticalPadding);
   }