Use ViewStack to convert card upload request CVC experiment to 2-step flow

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/autofill/save_card_bubble_views.h"
 
 #include <stddef.h>
 #include <memory>
 
 #include "base/strings/utf_string_conversions.h"
@@ skipping 36 matching lines @@
   }
   return label;
 }
 
 }  // namespace
 
 SaveCardBubbleViews::SaveCardBubbleViews(views::View* anchor_view,
                                          content::WebContents* web_contents,
                                          SaveCardBubbleController* controller)
     : LocationBarBubbleDelegateView(anchor_view, web_contents),
-      controller_(controller),
-      cvc_textfield_(nullptr),
-      learn_more_link_(nullptr) {
+      controller_(controller) {
   DCHECK(controller);
   views::BubbleDialogDelegateView::CreateBubble(this);
   chrome::RecordDialogCreation(chrome::DialogIdentifier::SAVE_CARD);
 }
 
 SaveCardBubbleViews::~SaveCardBubbleViews() {}
 
 void SaveCardBubbleViews::Show(DisplayReason reason) {
   ShowForReason(reason);
 }
@@ skipping 21 matching lines @@
       new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
 
   // Add a StyledLabel for each line of the legal message.
   for (const LegalMessageLine& line : controller_->GetLegalMessageLines())
     view->AddChildView(CreateLegalMessageLineLabel(line, this).release());
 
   return view;
 }
 
 bool SaveCardBubbleViews::Accept() {
+  // The main content ViewStack for local save and happy-path upload save should
+  // only ever have 1 View on it. Upload save can have a second View if CVC
+  // needs to be requested. Assert that the ViewStack has no more than 2 Views
+  // and that if it *does* have 2, it's because CVC is being requested.
+  DCHECK_LE(view_stack_->size(), 2U);
+  DCHECK(view_stack_->size() == 1 || controller_->ShouldRequestCvcFromUser());
+  if (controller_->ShouldRequestCvcFromUser() && view_stack_->size() == 1) {
+    // If user accepted upload but more info is needed, push the next view onto
+    // the stack.
+    view_stack_->Push(CreateRequestCvcView(), /*animate=*/true);
+    // Disable the Save button until a valid CVC is entered:
+    GetDialogClientView()->UpdateDialogButtons();
+    // Resize the bubble if it's grown larger:
+    SizeToContents();
+    return false;
+  }
+  // Otherwise, close the bubble as normal.
   if (controller_)
     controller_->OnSaveButton(cvc_textfield_ ? cvc_textfield_->text()
                                              : base::string16());
   return true;
 }
 
 bool SaveCardBubbleViews::Cancel() {
   if (controller_)
     controller_->OnCancelButton();
   return true;
@@ skipping 63 matching lines @@
       return;
     }
   }
 
   // |range| was not found.
   NOTREACHED();
 }
 
 // Create view containing everything except for the footnote.
 std::unique_ptr<views::View> SaveCardBubbleViews::CreateMainContentView() {
-  std::unique_ptr<View> view(new View());
+  auto view = base::MakeUnique<views::View>();
   view->SetLayoutManager(
       new views::BoxLayout(views::BoxLayout::kVertical, 0, 0,
                            views::kUnrelatedControlVerticalSpacing));
 
   // Add the card type icon, last four digits and expiration date.
   views::View* description_view = new views::View();
   description_view->SetLayoutManager(new views::BoxLayout(
       views::BoxLayout::kHorizontal, 0, 0, views::kRelatedButtonHSpacing));
   view->AddChildView(description_view);
 
   const CreditCard& card = controller_->GetCard();
   views::ImageView* card_type_icon = new views::ImageView();
   card_type_icon->SetImage(
       ResourceBundle::GetSharedInstance()
           .GetImageNamed(CreditCard::IconResourceId(card.network()))
           .AsImageSkia());
   card_type_icon->SetTooltipText(card.NetworkForDisplay());
   card_type_icon->SetBorder(
       views::CreateSolidBorder(1, SkColorSetA(SK_ColorBLACK, 10)));
   description_view->AddChildView(card_type_icon);
 
   description_view->AddChildView(new views::Label(
       base::string16(kMidlineEllipsis) + card.LastFourDigits()));
   description_view->AddChildView(
       new views::Label(card.AbbreviatedExpirationDateForDisplay()));
 
-  // Optionally add CVC request field if CVC was missing.
-  if (controller_->ShouldRequestCvcFromUser())
-    view->AddChildView(CreateRequestCvcView().release());
-
   // Optionally add label that will contain an explanation for upload.
   base::string16 explanation = controller_->GetExplanatoryMessage();
   if (!explanation.empty()) {
     views::Label* explanation_label = new views::Label(explanation);
     explanation_label->SetMultiLine(true);
     explanation_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     view->AddChildView(explanation_label);
   }
 
   return view;
 }
 
 std::unique_ptr<views::View> SaveCardBubbleViews::CreateRequestCvcView() {
-  std::unique_ptr<View> request_cvc_view = base::MakeUnique<views::View>();
-  request_cvc_view->SetLayoutManager(new views::BoxLayout(
+  auto request_cvc_view = base::MakeUnique<views::View>();
+  request_cvc_view->set_background(
+      views::Background::CreateThemedSolidBackground(
+          request_cvc_view.get(), ui::NativeTheme::kColorId_BubbleBackground));
+  request_cvc_view->SetLayoutManager(
+      new views::BoxLayout(views::BoxLayout::kVertical, 0, 0,
+                           views::kUnrelatedControlVerticalSpacing));
+  auto inner_request_cvc_view = base::MakeUnique<views::View>();

[Evan Stade, 2017/05/23 23:21:08]

can you explain why you need this inner view? It looks like request_cvc_view
only has this one child so what is its purpose?

[Jared Saul, 2017/05/23 23:57:50]

Yep, added a comment.  The reason is because if only the single child (the
horizontal inner_request_cvc_view) is added to the ViewStack, the textfield
stretches to fill the entire height of the bubble.  Wrapping that
horizontally-layed-out view in a containing view keeps the textfield at the
normal size of the font.  I can provide screenshots if you want them.

I expect the general layout of these views to change as UX provides more
direction and requirements.

[Evan Stade, 2017/05/24 00:20:06]

It sounds like you just need to use a different BoxLayout::CrossAxisAlignment so
the textfield doesn't stretch.

[Jared Saul, 2017/05/24 00:45:23]

Thanks!  That kept it from stretching.

+  inner_request_cvc_view->SetLayoutManager(new views::BoxLayout(
       views::BoxLayout::kHorizontal, 0, 0, views::kRelatedButtonHSpacing));
 
   DCHECK(!cvc_textfield_);
   cvc_textfield_ = CreateCvcTextfield();
   cvc_textfield_->set_controller(this);
-  request_cvc_view->AddChildView(cvc_textfield_);
+  inner_request_cvc_view->AddChildView(cvc_textfield_);
 
   views::ImageView* cvc_image = new views::ImageView();

[Evan Stade, 2017/05/23 23:21:08]

this is inconsistent with the smart pointer for inner_request_cvc_view. I prefer
not using smart pointers for views like this but one way or another it should be
consistent.

[Jared Saul, 2017/05/23 23:57:50]

+1 for consistency, but can you elaborate on what should change? 
CreateMainContentView() also has a smart pointer for the overall view, but one
views::View*, one views::ImageView*, and one views::Label*.  Should these all be
smart pointers as well?

Alternatively, if request_cvc_view stayed a smart pointer because it's being
returned, but inner_request_cvc_view and everything else was a views::View*,
would that be ideal instead?

[Evan Stade, 2017/05/24 00:20:06]

yes, this is my preference. Thanks.

[Jared Saul, 2017/05/24 00:45:24]

Acknowledged (inner_request_cvc_view went away anyway).

   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   cvc_image->SetImage(
       rb.GetImageSkiaNamed(controller_->GetCvcImageResourceId()));
-  request_cvc_view->AddChildView(cvc_image);
+  inner_request_cvc_view->AddChildView(cvc_image);
 
-  request_cvc_view->AddChildView(new views::Label(
+  inner_request_cvc_view->AddChildView(new views::Label(
       l10n_util::GetStringUTF16(IDS_AUTOFILL_SAVE_CARD_PROMPT_ENTER_CVC)));
+
+  request_cvc_view->AddChildView(inner_request_cvc_view.release());
   return request_cvc_view;
 }
 
 bool SaveCardBubbleViews::IsDialogButtonEnabled(ui::DialogButton button) const {
   if (button == ui::DIALOG_BUTTON_CANCEL)
     return true;
 
   DCHECK_EQ(ui::DIALOG_BUTTON_OK, button);
   return !cvc_textfield_ ||
          controller_->InputCvcIsValid(cvc_textfield_->text());
 }
 
 void SaveCardBubbleViews::ContentsChanged(views::Textfield* sender,
                                           const base::string16& new_contents) {
   DCHECK_EQ(cvc_textfield_, sender);
   GetDialogClientView()->UpdateDialogButtons();
 }
 
 void SaveCardBubbleViews::Init() {
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
-  AddChildView(CreateMainContentView().release());
+  view_stack_ = new ViewStack();
+  view_stack_->set_background(views::Background::CreateThemedSolidBackground(
+      view_stack_, ui::NativeTheme::kColorId_BubbleBackground));
+  view_stack_->Push(CreateMainContentView(), /*animate=*/false);
+  AddChildView(view_stack_);
 }
 
 }  // namespace autofill