[Mac] Do not use the |title| field of the Page Info section, but the optional |headline|.

* As a consequence of this, the view into which we draw |-isFlipped==YES| so
  layout now happens right-side up.
* Do not show the cert button unless it would be enabled.

BUG=59611
TEST=No more headings for "Identity" or "Connection".

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

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

2 files changed, 104 insertions, 70 deletions

diff --git a/chrome/browser/cocoa/page_info_bubble_controller.mm b/chrome/browser/cocoa/page_info_bubble_controller.mm
index a2cc34f..b10c1cf 100644
--- a/chrome/browser/cocoa/page_info_bubble_controller.mm
+++ b/chrome/browser/cocoa/page_info_bubble_controller.mm
@@ -26,9 +26,9 @@
 - (PageInfoModel*)model;
 - (NSButton*)certificateButtonWithFrame:(NSRect)frame;
 - (void)configureTextFieldAsLabel:(NSTextField*)textField;
-- (CGFloat)addTitleViewForInfo:(const PageInfoModel::SectionInfo&)info
-                    toSubviews:(NSMutableArray*)subviews
-                       atPoint:(NSPoint)point;
+- (CGFloat)addHeadlineViewForInfo:(const PageInfoModel::SectionInfo&)info
+                       toSubviews:(NSMutableArray*)subviews
+                          atPoint:(NSPoint)point;
 - (CGFloat)addDescriptionViewForInfo:(const PageInfoModel::SectionInfo&)info
                           toSubviews:(NSMutableArray*)subviews
                              atPoint:(NSPoint)point;
@@ -45,6 +45,19 @@
                              parentWindow:(NSWindow*)parent;
 @end
 
+// This simple NSView subclass is used as the single subview of the page info
+// bubble's window's contentView. Drawing is flipped so that layout of the
+// sections is easier. Apple recommends flipping the coordinate origin when
+// doing a lot of text layout because it's more natural.
+@interface PageInfoContentView : NSView {
+}
+@end
+@implementation PageInfoContentView
+- (BOOL)isFlipped {
+  return YES;
+}
+@end
+
 namespace {
 
 // The width of the window, in view coordinates. The height will be determined
@@ -57,8 +70,8 @@ const NSInteger kVerticalSpacing = 10;
 // Padding along on the X-axis between the window frame and content.
 const NSInteger kFramePadding = 20;
 
-// Spacing between the title and description text views.
-const NSInteger kTitleSpacing = 2;
+// Spacing between the optional headline and description text views.
+const NSInteger kHeadlineSpacing = 2;
 
 // Spacing between the image and the text.
 const NSInteger kImageSpacing = 10;
@@ -173,28 +186,33 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
 // not using HTTPS.
 - (void)performLayout {
   // |offset| is the Y position that should be drawn at next.
-  CGFloat offset = kVerticalSpacing;
+  CGFloat offset = kFramePadding;
 
   // Keep the new subviews in an array that gets replaced at the end.
   NSMutableArray* subviews = [NSMutableArray array];
 
-  // First item, drawn at the bottom of the window, is the help center link.
-  offset += [self addHelpButtonToSubviews:subviews atOffset:offset];
-  offset += kVerticalSpacing;
-  offset += [self addSeparatorToSubviews:subviews atOffset:offset];
-
-  // Build the window from bottom-up because Cocoa's coordinate origin is the
-  // lower left.
-  for (int i = model_->GetSectionCount() - 1; i >= 0; --i) {
+  // The subviews will be attached to the PageInfoContentView, which has a
+  // flipped origin. This allows the code to build top-to-bottom.
+  const int sectionCount = model_->GetSectionCount();
+  for (int i = 0; i < sectionCount; ++i) {
     PageInfoModel::SectionInfo info = model_->GetSectionInfo(i);
 
     // Only certain sections have images. This affects the X position.
     BOOL hasImage = model_->GetIconImage(info.icon_id) != nil;
     CGFloat xPosition = (hasImage ? kTextXPosition : kTextXPositionNoImage);
 
-    if (info.type == PageInfoModel::SECTION_INFO_IDENTITY) {
-      offset += [self addCertificateButtonToSubviews:subviews
-          atOffset:offset];
+    // Insert the image subview for sections that are appropriate.
+    CGFloat imageBaseline = offset + kImageSize;
+    if (hasImage) {
+      [self addImageViewForInfo:info toSubviews:subviews atOffset:offset];
+    }
+
+    // Add the title.
+    if (!info.headline.empty()) {
+      offset += [self addHeadlineViewForInfo:info
+                                  toSubviews:subviews
+                                     atPoint:NSMakePoint(xPosition, offset)];
+      offset += kHeadlineSpacing;
     }
 
     // Create the description of the state.
@@ -202,31 +220,37 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
                                    toSubviews:subviews
                                       atPoint:NSMakePoint(xPosition, offset)];
 
-    // Add the title.
-    offset += kTitleSpacing;
-    offset += [self addTitleViewForInfo:info
-                             toSubviews:subviews
-                                atPoint:NSMakePoint(xPosition, offset)];
-
-    // Insert the image subview for sections that are appropriate.
-    if (hasImage) {
-      [self addImageViewForInfo:info toSubviews:subviews atOffset:offset];
+    if (info.type == PageInfoModel::SECTION_INFO_IDENTITY && certID_) {
+      offset += kVerticalSpacing;
+      offset += [self addCertificateButtonToSubviews:subviews atOffset:offset];
     }
 
+    // If at this point the description and optional headline and button are
+    // not as tall as the image, adjust the offset by the difference.
+    CGFloat imageBaselineDelta = imageBaseline - offset;
+    if (imageBaselineDelta > 0)
+      offset += imageBaselineDelta;
+
     // Add the separators.
-    if (i != 0) {
-      offset += kVerticalSpacing;
-      offset += [self addSeparatorToSubviews:subviews atOffset:offset];
-    }
+    offset += kVerticalSpacing;
+    offset += [self addSeparatorToSubviews:subviews atOffset:offset];
   }
 
-  // Replace the window's content.
-  [[[self window] contentView] setSubviews:subviews];
+  // The last item at the bottom of the window is the help center link.
+  offset += [self addHelpButtonToSubviews:subviews atOffset:offset];
+  offset += kVerticalSpacing;
 
-  offset += kFramePadding;
+  // Create the dummy view that uses flipped coordinates.
+  NSRect contentFrame = NSMakeRect(0, 0, kWindowWidth, offset);
+  scoped_nsobject<PageInfoContentView> contentView(
+      [[PageInfoContentView alloc] initWithFrame:contentFrame]);
+  [contentView setSubviews:subviews];
 
-  NSRect windowFrame = NSMakeRect(0, 0, kWindowWidth, 0);
-  windowFrame.size.height += offset;
+  // Replace the window's content.
+  [[[self window] contentView] setSubviews:
+      [NSArray arrayWithObject:contentView]];
+
+  NSRect windowFrame = NSMakeRect(0, 0, kWindowWidth, offset);
   windowFrame.size = [[[self window] contentView] convertSize:windowFrame.size
                                                        toView:nil];
   // Adjust the origin by the difference in height.
@@ -273,21 +297,21 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
 
 // Adds the title text field at the given x,y position, and returns the y
 // position for the next element.
-- (CGFloat)addTitleViewForInfo:(const PageInfoModel::SectionInfo&)info
-                    toSubviews:(NSMutableArray*)subviews
-                       atPoint:(NSPoint)point {
+- (CGFloat)addHeadlineViewForInfo:(const PageInfoModel::SectionInfo&)info
+                       toSubviews:(NSMutableArray*)subviews
+                          atPoint:(NSPoint)point {
   NSRect frame = NSMakeRect(point.x, point.y, kTextWidth, kImageSpacing);
-  scoped_nsobject<NSTextField> titleField(
+  scoped_nsobject<NSTextField> textField(
       [[NSTextField alloc] initWithFrame:frame]);
-  [self configureTextFieldAsLabel:titleField.get()];
-  [titleField setStringValue:base::SysUTF16ToNSString(info.title)];
+  [self configureTextFieldAsLabel:textField.get()];
+  [textField setStringValue:base::SysUTF16ToNSString(info.headline)];
   NSFont* font = [NSFont boldSystemFontOfSize:[NSFont smallSystemFontSize]];
-  [titleField setFont:font];
+  [textField setFont:font];
   frame.size.height +=
       [GTMUILocalizerAndLayoutTweaker sizeToFitFixedWidthTextField:
-          titleField];
-  [titleField setFrame:frame];
-  [subviews addObject:titleField.get()];
+          textField];
+  [textField setFrame:frame];
+  [subviews addObject:textField.get()];
   return NSHeight(frame);
 }
 
@@ -315,6 +339,9 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
 // Returns the y position for the next element.
 - (CGFloat)addCertificateButtonToSubviews:(NSMutableArray*)subviews
                                  atOffset:(CGFloat)offset {
+  // The certificate button should only be added if there is SSL information.
+  DCHECK(certID_);
+
   // Create the certificate button. The frame will be fixed up by GTM, so
   // use arbitrary values.
   NSRect frame = NSMakeRect(kTextXPosition, offset, 100, 14);
@@ -323,19 +350,17 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
   [GTMUILocalizerAndLayoutTweaker sizeToFitView:certButton];
 
   // By default, assume that we don't have certificate information to show.
-  [certButton setEnabled:NO];
-  if (certID_) {
-    scoped_refptr<net::X509Certificate> cert;
-    CertStore::GetSharedInstance()->RetrieveCert(certID_, &cert);
-
-    // Don't bother showing certificates if there isn't one. Gears runs
-    // with no OS root certificate.
-    if (cert.get() && cert->os_cert_handle()) {
-      [certButton setEnabled:YES];
-    }
+  scoped_refptr<net::X509Certificate> cert;
+  CertStore::GetSharedInstance()->RetrieveCert(certID_, &cert);
+
+  // Don't bother showing certificates if there isn't one. Gears runs
+  // with no OS root certificate.
+  if (!cert.get() || !cert->os_cert_handle()) {
+    // This should only ever happen in unit tests.
+    [certButton setEnabled:NO];
   }
 
-  return NSHeight(frame) + kVerticalSpacing;
+  return NSHeight([certButton frame]);
 }
 
 // Adds the state image at a pre-determined x position and the given y. This
@@ -344,7 +369,7 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
 - (void)addImageViewForInfo:(const PageInfoModel::SectionInfo&)info
                  toSubviews:(NSMutableArray*)subviews
                    atOffset:(CGFloat)offset {
-  NSRect frame = NSMakeRect(kFramePadding, offset - kImageSize, kImageSize,
+  NSRect frame = NSMakeRect(kFramePadding, offset, kImageSize,
       kImageSize);
   scoped_nsobject<NSImageView> imageView(
       [[NSImageView alloc] initWithFrame:frame]);
diff --git a/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm b/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm
index eb7c177..da8a9db 100644
--- a/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm
+++ b/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm
@@ -22,11 +22,11 @@ class FakeModel : public PageInfoModel {
   FakeModel() : PageInfoModel() {}
 
   void AddSection(SectionStateIcon icon_id,
-                  const string16& title,
+                  const string16& headline,
                   const string16& description,
                   SectionInfoType type) {
     sections_.push_back(SectionInfo(
-        icon_id, title, string16(), description, type));
+        icon_id, string16(), headline, description, type));
   }
 };
 
@@ -67,7 +67,13 @@ class PageInfoBubbleControllerTest : public CocoaTest {
     int link_count = 1;
     ++spacer_count;
 
-    for (NSView* view in [[window_ contentView] subviews]) {
+    // The window's only immediate child is an invisible view that has a flipped
+    // coordinate origin. It is into this that all views get placed.
+    NSArray* windowSubviews = [[window_ contentView] subviews];
+    EXPECT_EQ(1U, [windowSubviews count]);
+    NSArray* subviews = [[windowSubviews lastObject] subviews];
+
+    for (NSView* view in subviews) {
       if ([view isKindOfClass:[NSTextField class]]) {
         --text_count;
       } else if ([view isKindOfClass:[NSImageView class]]) {
@@ -113,28 +119,28 @@ class PageInfoBubbleControllerTest : public CocoaTest {
 
 TEST_F(PageInfoBubbleControllerTest, NoHistoryNoSecurity) {
   model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
-      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
+      string16(),
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY),
       PageInfoModel::SECTION_INFO_IDENTITY);
   model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
-      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
+      string16(),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
           ASCIIToUTF16("google.com")),
       PageInfoModel::SECTION_INFO_CONNECTION);
 
   CreateBubble();
-  CheckWindow(/*text=*/4, /*image=*/2, /*spacer=*/1, /*button=*/1);
+  CheckWindow(/*text=*/2, /*image=*/2, /*spacer=*/1, /*button=*/0);
 }
 
 
 TEST_F(PageInfoBubbleControllerTest, HistoryNoSecurity) {
   model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
-      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
+      string16(),
       l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY),
       PageInfoModel::SECTION_INFO_IDENTITY);
   model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
-      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
+      string16(),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
           ASCIIToUTF16("google.com")),
@@ -153,13 +159,13 @@ TEST_F(PageInfoBubbleControllerTest, HistoryNoSecurity) {
 
   [controller_ performLayout];
 
-  CheckWindow(/*text=*/6, /*image=*/3, /*spacer=*/2, /*button=*/1);
+  CheckWindow(/*text=*/4, /*image=*/3, /*spacer=*/2, /*button=*/0);
 }
 
 
 TEST_F(PageInfoBubbleControllerTest, NoHistoryMixedSecurity) {
   model_->AddSection(PageInfoModel::ICON_STATE_OK,
-      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
+      string16(),
       l10n_util::GetStringFUTF16(
           IDS_PAGE_INFO_SECURITY_TAB_SECURE_IDENTITY,
           ASCIIToUTF16("Goat Security Systems")),
@@ -177,17 +183,20 @@ TEST_F(PageInfoBubbleControllerTest, NoHistoryMixedSecurity) {
           IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_INSECURE_CONTENT_WARNING));
 
   model_->AddSection(PageInfoModel::ICON_STATE_OK,
-      l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
+      string16(),
       description,
       PageInfoModel::SECTION_INFO_CONNECTION);
 
+
   CreateBubble();
+  [controller_ setCertID:1];
+  [controller_ performLayout];
 
-  NSArray* subviews = [[window_ contentView] subviews];
-  CheckWindow(/*text=*/4, /*image=*/2, /*spacer=*/1, /*button=*/1);
+  CheckWindow(/*text=*/2, /*image=*/2, /*spacer=*/1, /*button=*/1);
 
   // Look for the over-sized box.
   NSString* targetDesc = base::SysUTF16ToNSString(description);
+  NSArray* subviews = [[window_ contentView] subviews];
   for (NSView* subview in subviews) {
     if ([subview isKindOfClass:[NSTextField class]]) {
       NSTextField* desc = static_cast<NSTextField*>(subview);