[Web Payments] Add Cancel button to all sheets

chrome/browser/ui/views/payments/payment_sheet_view_controller.cc

 // Copyright 2016 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/payments/payment_sheet_view_controller.h"
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 #include <vector>
@@ skipping 180 matching lines @@
   layout->StartRow(0, 0);
   layout->AddView(CreatePaymentMethodRow().release());
   layout->StartRow(1, 0);
   layout->AddView(CreateContactInfoRow().release());
 
   return CreatePaymentView(
       CreateSheetHeaderView(
           false,
           l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_PAYMENT_SHEET_TITLE),
           this),
-      std::move(content_view));
+      std::move(content_view),
+      CreateButtonsView(this));

[Mathieu, 2017/01/31 22:53:12]

Perhaps CreateButtonsView could be part of CreatePaymentView, and instead we
just pass |this| here as the button listener? Or, moving CreatePaymentView into
PaymentRequestSheetController, so that we avoid passing |this|. 

I don't see each view creating it's own extra buttons in the future and passing
a different buttons view here. I could only see a view modifying the label of
the "Pay/Save/Done" button (achieved through a function override), and possibly
listening to the ButtonPressed event for the "Pay/Save/Done" button to override
default pay behavior, which we have the mechanism for below. But, perhaps I'm
missing some aspect of the mocks which would require extra buttons apart from
Cancel and "Pay/Save/Done".

[anthonyvd, 2017/02/01 15:35:23]

Many of the screens in the mocks have extra/different things other than
Pay/Cancel in that footer:

- Shipping Address, Payment Method, and Contact Info sheets have "Add
{Address|Card|Info}", "Edit", "Cancel" (no "Pay"). Their "Edit" button also
becomes "Hide Edit" in some cases.
- The editors have reminder text in there instead of buttons: "* indicates
required fields"

I see a few options:

1- This one where the controller creating the view passes everything they want
in the footer that's not "Cancel". We can have utility functions that create the
"Pay", "Edit", etc buttons easily.

2- Pass arguments to CreateButtonsView indicating which buttons should be added.
I feel like this gets out of hand quickly and leaks the controller's
responsibility into the generic/common code. In addition, this option makes it
impossible for controllers to cleanly keep a reference to their buttons, for
example if they need to control their disabled state.

3- Since CreateButtonsView takes a PaymentRequestSheetController*, have it call
into a function of that controller that either signals which buttons are
required or returns the actual buttons. That's pretty much a "pull" version of
1- and 2-.

I prefer #1 but I could obviously be missing something. WDYT?

[Mathieu, 2017/02/01 17:17:36]

Wow, I didn't realize that there were so many buttons. I had a look at the mocks
along with your explanation.

For simplicity let's split into the "left view" (contains either a label, or an
"Add [blah]" button) and the right view (Ideally always "Cancel" and
"Done/Pay/Save" -- Zach has sent feedback on "Edit").

The left view will be specific to each view controller, so they can pass the
whole view in CreatePaymentView (in a followup CL).

For the right view, since things are in flux and the Done/Pay/Save button is not
implemented now, we could defer? Though I still think my original suggestion
(each controller having to implement CreatePrimaryButton or
GetLabelForPrimaryButton, in line with your #3) makes sense... RE: the reference
to the button: you could get a reference through something like primary_button()
whenever you want to disable the button.

 }
 
 void PaymentSheetViewController::ButtonPressed(
     views::Button* sender, const ui::Event& event) {
   switch (sender->tag()) {
-    case static_cast<int>(PaymentRequestCommonTags::CLOSE_BUTTON_TAG):
-      dialog()->CloseDialog();
-      break;
-
     case static_cast<int>(
         PaymentSheetViewControllerTags::SHOW_ORDER_SUMMARY_BUTTON):
       dialog()->ShowOrderSummary();
       break;
 
     case static_cast<int>(PaymentSheetViewControllerTags::SHOW_SHIPPING_BUTTON):
       dialog()->ShowShippingListSheet();
       break;
 
     case static_cast<int>(
         PaymentSheetViewControllerTags::SHOW_PAYMENT_METHOD_BUTTON):
       dialog()->ShowPaymentMethodSheet();
       break;
 
     case static_cast<int>(
         PaymentSheetViewControllerTags::SHOW_CONTACT_INFO_BUTTON):
       // TODO(tmartino): Transition to contact info page once it exists.
       break;
 
     default:
-      NOTREACHED();
+      PaymentRequestSheetController::ButtonPressed(sender, event);
+      break;
   }
 }
 
 std::unique_ptr<views::View>
 PaymentSheetViewController::CreateOrderSummarySectionContent() {
   CurrencyFormatter* formatter = request()->GetOrCreateCurrencyFormatter(
       request()->details()->total->amount->currency,
       request()->details()->total->amount->currency_system,
       g_browser_process->GetApplicationLocale());
   base::string16 label_value = l10n_util::GetStringFUTF16(
@@ skipping 140 matching lines @@
       CreateContactInfoSectionContent(), std::unique_ptr<views::View>(nullptr),
       widest_name_column_view_width_);
   section->set_tag(static_cast<int>(
       PaymentSheetViewControllerTags::SHOW_CONTACT_INFO_BUTTON));
   section->set_id(
       static_cast<int>(DialogViewID::PAYMENT_SHEET_CONTACT_INFO_SECTION));
   return section;
 }
 
 }  // namespace payments