Add card details and legal message to Android save credit card infobar.

chrome/browser/ui/views/autofill/save_card_bubble_views.cc

Index: chrome/browser/ui/views/autofill/save_card_bubble_views.cc
diff --git a/chrome/browser/ui/views/autofill/save_card_bubble_views.cc b/chrome/browser/ui/views/autofill/save_card_bubble_views.cc
index 41e7293b0e1d89305421c131a973c3d64f291a1e..e174ea4cb5b08c171c58bc4be75af06a396ad865 100644
--- a/chrome/browser/ui/views/autofill/save_card_bubble_views.cc
+++ b/chrome/browser/ui/views/autofill/save_card_bubble_views.cc
@@ -10,6 +10,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/ui/autofill/autofill_dialog_types.h"
 #include "chrome/browser/ui/autofill/save_card_bubble_controller.h"
+#include "components/autofill/core/browser/legal_message_line.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/views/bubble/bubble_frame_view.h"
@@ -41,12 +42,11 @@ const bool kIsOkButtonOnLeftSide = false;
 #endif
 
 scoped_ptr<views::StyledLabel> CreateLegalMessageLineLabel(
-    const SaveCardBubbleController::LegalMessageLine& line,
+    const LegalMessageLine& line,
     views::StyledLabelListener* listener) {
   scoped_ptr<views::StyledLabel> label(
-      new views::StyledLabel(line.text, listener));
-  for (const SaveCardBubbleController::LegalMessageLine::Link& link :
-       line.links) {
+      new views::StyledLabel(line.text(), listener));
+  for (const LegalMessageLine::Link& link : line.links()) {
     label->AddStyleRange(link.range,
                          views::StyledLabel::RangeStyleInfo::CreateForLink());
   }
@@ -122,8 +122,8 @@ void SaveCardBubbleViews::StyledLabelLinkClicked(views::StyledLabel* label,
 
   const auto& links =
       controller_->GetLegalMessageLines()[label->parent()->GetIndexOf(label)]
-          .links;
-  for (const SaveCardBubbleController::LegalMessageLine::Link& link : links) {
+          ->links();
+  for (const LegalMessageLine::Link& link : links) {
     if (link.range == range) {
       controller_->OnLegalMessageLinkClicked(link.url);
       return;
@@ -229,9 +229,8 @@ scoped_ptr<views::View> SaveCardBubbleViews::CreateFootnoteView() {
       views::Background::CreateSolidBackground(kLightShadingColor));
 
   // Add a StyledLabel for each line of the legal message.
-  for (const SaveCardBubbleController::LegalMessageLine& line :
-       controller_->GetLegalMessageLines()) {
-    view->AddChildView(CreateLegalMessageLineLabel(line, this).release());
+  for (const auto& line : controller_->GetLegalMessageLines()) {

[Evan Stade, 2016/01/08 03:14:12]

nit: only use auto for long and complicated types (i.e. those with generics),
otherwise it reduces readability.

[Peter Kasting, 2016/01/08 03:18:43]

I think using auto here is a win.

I also wouldn't generalize as "only for long and complicated types".  auto can
be a win almost anywhere if the actual underlying type is irrelevant to the
reader.  Here knowing the type of |line| buys you nothing.

The Google style guide intentionally avoids specifics like "long and
complicated" and instead uses this sentence: "Use auto to avoid type names that
are just clutter."  There are specific examples given, and a typename with
generics in it is one, but not the only one.

It's a judgment call for sure though.

[Evan Stade, 2016/01/08 03:23:57]

You think 

for (const auto& line : controller_->GetLegalMessageLines())

is more readable than

for (const LegalMessageLine& line : controller_->GetLegalMessageLines())

?

the difference in length is minimal, but the auto makes my brain work hard to
figure out what it resolves to. I think auto is a dangerous tool in that regard;
I like strongly typed languages and this mimics weak typing.

[Peter Kasting, 2016/01/08 07:50:59]

Yes.  I care that I'm getting a line from GetLegalMessageLines().  The name
|line| tells me everything I need to know about what the line is; the precise
typename doesn't help me and simply obscures the details I care about.

It's not a huge difference, but I definitely prefer auto.
You may be swimming upstream on this one; I think auto usage will continue to
spread widely.  In a way it does mimic weak typing -- it gets the brevity
benefits of weak typing but without having to use an actual
weakly-typed-everywhere language.  To me that's a good thing, and from what I've
seen around the codebase, I'm not the only one.

[please use gerrit instead, 2016/01/12 00:47:25]

Thank you both for thoughtful discussion on this topic. I am going to side with
Peter here in particular, because "auto" is actually hiding
"scoped_ptr<LegalMessageLine>", which is quite long.

+    view->AddChildView(CreateLegalMessageLineLabel(*line, this).release());
   }
 
   return view;