Add explanation text and legal message footer to upload bubble.

chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.mm

Index: chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.mm
diff --git a/chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.mm b/chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.mm
index 6915f4467d6682876361f76d5cc26bdecf66c146..28776796f2303c4dfc521b646387f761dc6603f5 100644
--- a/chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.mm
+++ b/chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.mm
@@ -21,11 +21,16 @@
 namespace {
 
 const CGFloat kDesiredBubbleWidth = 370;
+const CGFloat kDividerHeight = 1;
 const CGFloat kFramePadding = 16;
 const CGFloat kRelatedControlHorizontalPadding = 2;
+const CGFloat kRelatedControlVerticalPadding = 5;
 const CGFloat kUnrelatedControlVerticalPadding = 15;
 
-const SkColor kIconBorderColor = 0x10000000;  // SkColorSetARGB(10, 0, 0, 0);
+const SkColor kDividerColor = 0xFFE9E9E9;  // SkColorSetRGB(0xE9, 0xE9, 0xE9);
+const SkColor kFooterColor = 0xFFF5F5F5;   // SkColorSetRGB(0xF5, 0xF5, 0xF5);
+const SkColor kIconBorderColor = 0x10000000;  // SkColorSetARGB(0x10, 0, 0, 0);
+
 }
 
 namespace autofill {
@@ -49,10 +54,19 @@ base::string16 SaveCardBubbleViewBridge::GetWindowTitle() const {
   return controller_ ? controller_->GetWindowTitle() : base::string16();
 }
 
+base::string16 SaveCardBubbleViewBridge::GetExplanatoryMessage() const {
+  return controller_ ? controller_->GetExplanatoryMessage() : base::string16();
+}
+
 CreditCard SaveCardBubbleViewBridge::GetCard() const {
   return controller_ ? controller_->GetCard() : CreditCard();
 }
 
+const LegalMessageLines SaveCardBubbleViewBridge::GetLegalMessageLines() const {
+  return controller_ ? controller_->GetLegalMessageLines()
+                     : LegalMessageLines();
+}
+
 void SaveCardBubbleViewBridge::OnSaveButton() {
   if (controller_)
     controller_->OnSaveButton();
@@ -70,6 +84,11 @@ void SaveCardBubbleViewBridge::OnLearnMoreClicked() {
     controller_->OnLearnMoreClicked();
 }
 
+void SaveCardBubbleViewBridge::OnLegalMessageLinkClicked(const GURL& url) {
+  if (controller_)
+    controller_->OnLegalMessageLinkClicked(url);
+}
+
 void SaveCardBubbleViewBridge::OnBubbleClosed() {
   if (controller_)
     controller_->OnBubbleClosed();
@@ -91,31 +110,75 @@ void SaveCardBubbleViewBridge::Hide() {
 #pragma mark SaveCardBubbleViewCocoa
 
 @interface SaveCardBubbleViewCocoa ()
-+ (base::scoped_nsobject<NSTextField>)makeTextField;
-+ (base::scoped_nsobject<NSButton>)makeButton;
++ (base::scoped_nsobject<NSTextField>)makeLabel:(NSString*)text;
++ (base::scoped_nsobject<NSTextView>)makeWrappingLabel:(NSString*)text
+                                          withFontSize:(CGFloat)fontSize;
++ (base::scoped_nsobject<HyperlinkTextView>)makeHyperlinkText:(NSString*)text;
++ (base::scoped_nsobject<NSButton>)makeButton:(NSString*)text;
 @end
 
 @implementation SaveCardBubbleViewCocoa {
   autofill::SaveCardBubbleViewBridge* bridge_;  // Weak.
 }
 
-+ (base::scoped_nsobject<NSTextField>)makeTextField {
++ (base::scoped_nsobject<NSTextField>)makeLabel:(NSString*)text {
   base::scoped_nsobject<NSTextField> textField(
       [[NSTextField alloc] initWithFrame:NSZeroRect]);
   [textField setEditable:NO];
   [textField setSelectable:NO];
   [textField setDrawsBackground:NO];
   [textField setBezeled:NO];
+  [textField setStringValue:text];
+  [textField sizeToFit];
 
   return textField;
 }
 
-+ (base::scoped_nsobject<NSButton>)makeButton {
++ (base::scoped_nsobject<NSTextView>)makeWrappingLabel:(NSString*)text
+                                          withFontSize:(CGFloat)fontSize {
+  base::scoped_nsobject<NSTextView> textView(
+      [[NSTextView alloc] initWithFrame:NSZeroRect]);
+  NSDictionary* attributes =
+      @{NSFontAttributeName : [NSFont systemFontOfSize:fontSize]};
+  base::scoped_nsobject<NSAttributedString> attributedMessage(
+      [[NSAttributedString alloc] initWithString:text attributes:attributes]);
+  [[textView textStorage] setAttributedString:attributedMessage];
+  [[textView textContainer] setLineFragmentPadding:0.0f];
+  [textView setEditable:NO];
+  [textView setSelectable:NO];
+  [textView setDrawsBackground:NO];
+  [textView setVerticallyResizable:YES];
+  [textView setFrameSize:NSMakeSize(kDesiredBubbleWidth - (2 * kFramePadding),
+                                    MAXFLOAT)];
+  [textView sizeToFit];
+
+  return textView;
+}
+
++ (base::scoped_nsobject<HyperlinkTextView>)makeHyperlinkText:(NSString*)text {
+  base::scoped_nsobject<HyperlinkTextView> lineView(
+      [[HyperlinkTextView alloc] initWithFrame:NSZeroRect]);
+  [lineView setMessage:text
+              withFont:[NSFont systemFontOfSize:[NSFont systemFontSize]]
+          messageColor:[NSColor blackColor]];
+
+  [[lineView textContainer] setLineFragmentPadding:0.0f];
+  [lineView setVerticallyResizable:YES];
+  [lineView setFrameSize:NSMakeSize(kDesiredBubbleWidth - 2 * kFramePadding,
+                                    MAXFLOAT)];
+  [lineView sizeToFit];
+
+  return lineView;
+}
+
++ (base::scoped_nsobject<NSButton>)makeButton:(NSString*)text {
   base::scoped_nsobject<NSButton> button(
       [[NSButton alloc] initWithFrame:NSZeroRect]);
   [button setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]];
   [button setBezelStyle:NSRoundedBezelStyle];
   [[button cell] setControlSize:NSSmallControlSize];
+  [button setTitle:text];
+  [button sizeToFit];
 
   return button;
 }
@@ -160,24 +223,10 @@ void SaveCardBubbleViewBridge::Hide() {
 }
 
 - (void)loadView {
-  // Title is an NSTextView instead of an NSTextField to allow it to wrap.
+  // Title.
+  NSString* title = SysUTF16ToNSString(bridge_->GetWindowTitle());
   base::scoped_nsobject<NSTextView> titleLabel(
-      [[NSTextView alloc] initWithFrame:NSZeroRect]);
-  NSDictionary* attributes =
-      @{NSFontAttributeName : [NSFont systemFontOfSize:15.0]};
-  base::scoped_nsobject<NSAttributedString> attributedMessage(
-      [[NSAttributedString alloc]
-          initWithString:base::SysUTF16ToNSString(bridge_->GetWindowTitle())
-              attributes:attributes]);
-  [[titleLabel textStorage] setAttributedString:attributedMessage];
-  [[titleLabel textContainer] setLineFragmentPadding:0.0f];
-  [titleLabel setEditable:NO];
-  [titleLabel setSelectable:NO];
-  [titleLabel setDrawsBackground:NO];
-  [titleLabel setVerticallyResizable:YES];
-  [titleLabel setFrameSize:NSMakeSize(kDesiredBubbleWidth - (2 * kFramePadding),
-                                      MAXFLOAT)];
-  [titleLabel sizeToFit];
+      [SaveCardBubbleViewCocoa makeWrappingLabel:title withFontSize:15.0]);
 
   // Credit card info.
   autofill::CreditCard card = bridge_->GetCard();
@@ -196,85 +245,135 @@ void SaveCardBubbleViewBridge::Hide() {
                          .AsNSImage()];
   [cardIcon setFrameSize:[[cardIcon image] size]];
 
-  base::scoped_nsobject<NSTextField> lastFourLabel(
-      [SaveCardBubbleViewCocoa makeTextField]);
   // Midline horizontal ellipsis follwed by last four digits.
-  [lastFourLabel setStringValue:base::SysUTF16ToNSString(
-                                    base::UTF8ToUTF16("\xE2\x8B\xAF") +
-                                    card.LastFourDigits())];
-  [lastFourLabel sizeToFit];
+  base::scoped_nsobject<NSTextField> lastFourLabel([SaveCardBubbleViewCocoa
+      makeLabel:SysUTF16ToNSString(base::UTF8ToUTF16("\xE2\x8B\xAF") +

[groby-ooo-7-16, 2016/03/03 00:40:22]

I know, it's not your code, but can this be kEllipsis please?

[Justin Donnelly, 2016/03/03 18:45:10]

Do you mean kEllipsis from src/ui/gfx/text_elider.h? That's a regular ellipsis
not the midline version we need. Or did you just mean add a kEllipsis in this
file?

Either way, just FYI, I have a TODO that I'm going to tackle as soon as I'm done
with this bubble to replace all of these (here and in various other places) with
a common constant.

[groby-ooo-7-16, 2016/03/07 19:54:57]

SG - (I didn't mean the elider one)

+                                   card.LastFourDigits())]);
 
   base::scoped_nsobject<NSTextField> expirationDateLabel(
-      [SaveCardBubbleViewCocoa makeTextField]);
-  [expirationDateLabel
-      setStringValue:base::SysUTF16ToNSString(
-                         card.AbbreviatedExpirationDateForDisplay())];
-  [expirationDateLabel sizeToFit];
+      [SaveCardBubbleViewCocoa
+          makeLabel:base::SysUTF16ToNSString(
+                        card.AbbreviatedExpirationDateForDisplay())]);
+
+  // Explanatory text (only shown for upload).
+  base::scoped_nsobject<NSTextView> explanationLabel(

[groby-ooo-7-16, 2016/03/03 00:40:22]

Why are you still alloc/initing here?

[Justin Donnelly, 2016/03/03 18:45:09]

Having a zero-size label in the case where there's no explanation makes the
layout code a little simpler.

[groby-ooo-7-16, 2016/03/07 19:54:57]

You know you can send messages to nil, right? Return values are nil, 0, or a
zeroed out structure, depending on return type. Simplifies code even more :)

[Justin Donnelly, 2016/03/07 21:21:02]

I had forgotten that until you asked me why I was doing this. :) But then when I
tried to have explanationLabel be nil, I learned that you can't add nil as a
subview. I could make the setSubviews code be a bit more complicated and skip
adding it if it's nil. I'd prefer to leave this as-is but don't feel strongly
about it. I'm going to submit now but I'd be happy to change it later if you
feel the alternative is better.

+      [[NSTextView alloc] initWithFrame:NSZeroRect]);
+  base::string16 explanation = bridge_->GetExplanatoryMessage();
+  if (!explanation.empty()) {
+    explanationLabel.reset([SaveCardBubbleViewCocoa
+                               makeWrappingLabel:SysUTF16ToNSString(explanation)
+                                    withFontSize:[NSFont systemFontSize]]
+                               .release());

[groby-ooo-7-16, 2016/03/03 00:40:22]

Please don't use dot notation for message invocation.

[Justin Donnelly, 2016/03/03 18:45:09]

I couldn't figure out any other way to make this work. To be honest, despite
looking at the code many times, I'm still confused about exactly how
scoped_nsobject works. So I copied this approach from some other c/b/ui/cocoa
code. Right or wrong, it seems to be a not uncommon pattern[1].

Is there a better way?

[1]
https://code.google.com/p/chromium/codesearch#search/&q=%22.release()%22%20fi...

[groby-ooo-7-16, 2016/03/07 19:54:58]

Sigh. Misread the release as being a Cocoa message.

But yes, there's a simpler way - return an auto-released object instead of a
scoped one.  As for the "better" way - I personally believe that
scoped1.reset([scoped2 retain]) is the more cocoa-y way to do this, but YMMV.

[Justin Donnelly, 2016/03/07 21:21:02]

As discussed out-of-band, leaving this as is.

+  }
 
   // "Learn more" link.
-  base::scoped_nsobject<HyperlinkTextView> learnMoreLink(
-      [[HyperlinkTextView alloc] initWithFrame:NSZeroRect]);
   NSString* learnMoreString = l10n_util::GetNSString(IDS_LEARN_MORE);
-  [learnMoreLink
-        setMessage:learnMoreString
-          withFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]
-      messageColor:[NSColor blackColor]];
+  base::scoped_nsobject<HyperlinkTextView> learnMoreLink(
+      [SaveCardBubbleViewCocoa makeHyperlinkText:learnMoreString]);
   [learnMoreLink setDelegate:self];
+
   NSColor* linkColor =
       skia::SkColorToCalibratedNSColor(chrome_style::GetLinkColor());
-  [learnMoreLink addLinkRange:NSMakeRange(0, [learnMoreString length])
+  CGFloat length = [learnMoreString length];

[groby-ooo-7-16, 2016/03/03 00:40:22]

Might want to create the range here.

[Justin Donnelly, 2016/03/03 18:45:09]

Done.

+  [learnMoreLink addLinkRange:NSMakeRange(0, length)
                       withURL:nil
                     linkColor:linkColor];
-  NSTextStorage* text = [learnMoreLink textStorage];
-  [text addAttribute:NSUnderlineStyleAttributeName
-               value:[NSNumber numberWithInt:NSUnderlineStyleNone]
-               range:NSMakeRange(0, [learnMoreString length])];
-  [[learnMoreLink textContainer] setLineFragmentPadding:0.0f];
-  [learnMoreLink setVerticallyResizable:YES];
-  [learnMoreLink
-      setFrameSize:NSMakeSize(kDesiredBubbleWidth - 2 * kFramePadding,
-                              MAXFLOAT)];
-  [learnMoreLink sizeToFit];
+  [[learnMoreLink textStorage] addAttribute:NSUnderlineStyleAttributeName
+                                      value:@(NSUnderlineStyleNone)
+                                      range:NSMakeRange(0, length)];
 
   // Cancel button.
-  base::scoped_nsobject<NSButton> cancelButton(
-      [SaveCardBubbleViewCocoa makeButton]);
-  [cancelButton
-      setTitle:l10n_util::GetNSString(IDS_AUTOFILL_SAVE_CARD_PROMPT_DENY)];
-  [cancelButton sizeToFit];
+  base::scoped_nsobject<NSButton> cancelButton([SaveCardBubbleViewCocoa
+      makeButton:l10n_util::GetNSString(IDS_AUTOFILL_SAVE_CARD_PROMPT_DENY)]);
   [cancelButton setTarget:self];
   [cancelButton setAction:@selector(onCancelButton:)];
   [cancelButton setKeyEquivalent:@"\e"];
 
   // Save button.
-  base::scoped_nsobject<NSButton> saveButton(
-      [SaveCardBubbleViewCocoa makeButton]);
-  [saveButton
-      setTitle:l10n_util::GetNSString(IDS_AUTOFILL_SAVE_CARD_PROMPT_ACCEPT)];
-  [saveButton sizeToFit];
+  base::scoped_nsobject<NSButton> saveButton([SaveCardBubbleViewCocoa
+      makeButton:l10n_util::GetNSString(IDS_AUTOFILL_SAVE_CARD_PROMPT_ACCEPT)]);
   [saveButton setTarget:self];
   [saveButton setAction:@selector(onSaveButton:)];
   [saveButton setKeyEquivalent:@"\r"];
 
+  // Footer with legal text (only shown for upload).
+  base::scoped_nsobject<NSBox> divider(
+      [[NSBox alloc] initWithFrame:NSZeroRect]);
+  base::scoped_nsobject<NSView> footerView(
+      [[NSView alloc] initWithFrame:NSZeroRect]);
+  const autofill::LegalMessageLines& lines = bridge_->GetLegalMessageLines();
+  if (!lines.empty()) {
+    [divider setBoxType:NSBoxCustom];
+    [divider setBorderType:NSLineBorder];
+    [divider setBorderColor:skia::SkColorToCalibratedNSColor(kDividerColor)];
+    [divider setFrameSize:NSMakeSize(kDesiredBubbleWidth, kDividerHeight)];
+
+    [footerView setWantsLayer:YES];
+    [[footerView layer]
+        setBackgroundColor:skia::CGColorCreateFromSkColor(kFooterColor)];
+
+    CGFloat linesHeight = kFramePadding;
+    for (auto lineIter = lines.end() - 1; lineIter != lines.begin() - 1;

[groby-ooo-7-16, 2016/03/03 00:40:22]

 Why not build in the reverse direction and use (for const auto& line : lines)?
If that doesn't work, rbegin() and rend() are your friend and save you weird
arithmetic.

[Justin Donnelly, 2016/03/03 18:45:09]

The lines are built in cross-platform code also used by Views and Clank.
Sweet! Done.

[groby-ooo-7-16, 2016/03/07 19:54:57]

No, I mean building the UI layout in the reverse direction :) But rbegin/rend
works.

+         --lineIter) {
+      // Create the legal message line view.
+      base::scoped_nsobject<HyperlinkTextView> lineView([SaveCardBubbleViewCocoa
+          makeHyperlinkText:SysUTF16ToNSString(lineIter->text())]);
+      [lineView setDelegate:self];
+
+      // Add the links.
+      for (auto linkIter = lineIter->links().begin();

[groby-ooo-7-16, 2016/03/03 00:40:22]

C++11 loop?

[Justin Donnelly, 2016/03/03 18:45:09]

Done. So much nicer. (I couldn't find any examples of range based loops in
c/b/ui/cocoa so I wasn't sure they were allowed.)

+           linkIter != lineIter->links().end(); ++linkIter) {
+        CGFloat start = linkIter->range.start();
+        CGFloat length = linkIter->range.length();
+        [lineView addLinkRange:NSMakeRange(start, length)
+                       withURL:nil
+                     linkColor:linkColor];
+        [[lineView textStorage] addAttribute:NSUnderlineStyleAttributeName
+                                       value:@(NSUnderlineStyleNone)
+                                       range:NSMakeRange(start, length)];
+      }
+
+      // Add the line to the footer view.
+      [footerView addSubview:lineView];
+      [lineView setFrameOrigin:NSMakePoint(kFramePadding, linesHeight)];
+      linesHeight +=
+          [lineView frame].size.height + kRelatedControlVerticalPadding;
+    }
+    [footerView setFrameSize:NSMakeSize(kDesiredBubbleWidth,
+                                        linesHeight + kFramePadding)];
+  }
+
   // Layout.
+  [footerView setFrameOrigin:NSMakePoint(0, 0)];

[groby-ooo-7-16, 2016/03/03 00:40:23]

NSZeroPoint

[Justin Donnelly, 2016/03/03 18:45:10]

Done.

+
+  [divider setFrameOrigin:NSMakePoint(0, NSMaxY([footerView frame]))];
+
   [saveButton setFrameOrigin:
       NSMakePoint(kDesiredBubbleWidth - kFramePadding -
                       NSWidth([saveButton frame]),
-                  kFramePadding)];
+                  NSMaxY([divider frame]) + kFramePadding)];
   [cancelButton setFrameOrigin:
       NSMakePoint(NSMinX([saveButton frame]) -
                       kRelatedControlHorizontalPadding -
                       NSWidth([cancelButton frame]),
-                  kFramePadding)];
+                  NSMaxY([divider frame]) + kFramePadding)];
   [learnMoreLink setFrameOrigin:
       NSMakePoint(kFramePadding,
                   NSMidY([saveButton frame]) -
                       (NSHeight([learnMoreLink frame]) / 2.0))];
 
-  [cardIcon setFrameOrigin:
+  [explanationLabel setFrameOrigin:
       NSMakePoint(kFramePadding,
                   NSMaxY([saveButton frame]) +
                       kUnrelatedControlVerticalPadding)];
+
+  NSView* viewBelowIcon =
+      ([explanationLabel frame].size.height > 0) ? explanationLabel.get()
+                                                 : saveButton.get();
+  [cardIcon setFrameOrigin:
+      NSMakePoint(kFramePadding,
+                  NSMaxY([viewBelowIcon frame]) +
+                      kUnrelatedControlVerticalPadding)];
   [lastFourLabel setFrameOrigin:
       NSMakePoint(NSMaxX([cardIcon frame]) + kRelatedControlHorizontalPadding,
                   NSMidY([cardIcon frame]) -
@@ -290,8 +389,8 @@ void SaveCardBubbleViewBridge::Hide() {
                   NSMaxY([cardIcon frame]) + kUnrelatedControlVerticalPadding)];
 
   [[[self window] contentView] setSubviews:@[
-    titleLabel, cardIcon, lastFourLabel, expirationDateLabel, learnMoreLink,
-    cancelButton, saveButton
+    titleLabel, cardIcon, lastFourLabel, expirationDateLabel, explanationLabel,
+    learnMoreLink, cancelButton, saveButton, divider, footerView
   ]];
 
   // Update window frame.
@@ -315,6 +414,26 @@ void SaveCardBubbleViewBridge::Hide() {
     clickedOnLink:(id)link

[groby-ooo-7-16, 2016/03/03 00:40:23]

Instead of doing the whole iteration thing - you can give IDs to links. Why not
use that?

[Justin Donnelly, 2016/03/03 18:45:10]

I'm not quite sure what you mean. UIView's tag property? Or passing the URL in
addLinkRange:withURL:linkColor:? I don't do that latter because the other
bubbles don't (I think in order to make the links not draggable). Like you
alluded to yesterday, we do seem to revel in fighting the platform. :)

Or did you mean something else?

[groby-ooo-7-16, 2016/03/07 19:54:57]

I like draggable links - it's a more OSXy thing. But if all bubbles don't do it,
fine.

(Also, you can add custom attributes to any NSAttributed string, but that's
probably overkill...)

           atIndex:(NSUInteger)charIndex {
   DCHECK(bridge_);
+
+  // Check each of the links in each of the legal message lines ot see if they
+  // are the source of the click.
+  const autofill::LegalMessageLines& lines = bridge_->GetLegalMessageLines();
+  for (auto lineIter = lines.begin(); lineIter != lines.end(); ++lineIter) {

[groby-ooo-7-16, 2016/03/03 00:40:23]

C++11 loop?

[Justin Donnelly, 2016/03/03 18:45:10]

Done.

+    if (lineIter->text() ==
+        base::SysNSStringToUTF16([[textView textStorage] string])) {

[groby-ooo-7-16, 2016/03/07 19:54:57]

The text comparison thing just bugs me - but since we manually line break the
legal message, I suppose we have to. (Why do we manually break?)

[Justin Donnelly, 2016/03/07 21:21:02]

Payments wanted it that way for reasons I can no longer recall.

+      for (auto linkIter = lineIter->links().begin();

[groby-ooo-7-16, 2016/03/03 00:40:23]

You know what I'm going to say... :)

[Justin Donnelly, 2016/03/03 18:45:10]

Done. :)

+           linkIter != lineIter->links().end(); ++linkIter) {
+        if (linkIter->range.start() <= charIndex &&

[groby-ooo-7-16, 2016/03/07 19:54:58]

FWIW: You could do NSLocationInRange(charIndex, NSMakeRange(...))

bit more text, possibly more readable. It's your choice, I have no strong
preference.

+            charIndex < linkIter->range.end()) {
+          bridge_->OnLegalMessageLinkClicked(linkIter->url);
+          return YES;
+        }
+      }
+    }
+  }
+
+  // If none of the legal message links are the source of the click, the source
+  // must be the learn more link.
   bridge_->OnLearnMoreClicked();
   return YES;
 }