[Payments] Selecting incomplete items will open editors

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

Index: chrome/browser/ui/views/payments/payment_method_view_controller.cc
diff --git a/chrome/browser/ui/views/payments/payment_method_view_controller.cc b/chrome/browser/ui/views/payments/payment_method_view_controller.cc
index 1bdb8dcdc563f8fc868726f4afedb82861ca0fcf..404c9f220df3274335bd2e251d6b4507e91a125e 100644
--- a/chrome/browser/ui/views/payments/payment_method_view_controller.cc
+++ b/chrome/browser/ui/views/payments/payment_method_view_controller.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/ui/views/payments/payment_request_views_util.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/payments/content/payment_request_state.h"
+#include "components/payments/core/autofill_payment_instrument.h"
 #include "components/payments/core/payment_instrument.h"
 #include "components/strings/grit/components_strings.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -46,7 +47,7 @@ class PaymentMethodListItem : public payments::PaymentRequestItemList::Item {
   // Does not take ownership of |instrument|, which  should not be null and
   // should outlive this object. |list| is the PaymentRequestItemList object
   // that will own this.
-  PaymentMethodListItem(PaymentInstrument* instrument,
+  PaymentMethodListItem(AutofillPaymentInstrument* instrument,
                         PaymentRequestSpec* spec,
                         PaymentRequestState* state,
                         PaymentRequestItemList* list,
@@ -93,16 +94,13 @@ class PaymentMethodListItem : public payments::PaymentRequestItemList::Item {
     }
   }
 
-  bool CanBeSelected() const override {
-    // TODO(anthonyvd): Check for card completedness.
-    return true;
-  }
+  bool CanBeSelected() const override { return instrument_->IsValid(); }
 
   void PerformSelectionFallback() override {
-    // TODO(anthonyvd): Open the editor pre-populated with this card's data.
+    dialog_->ShowCreditCardEditor(instrument_->credit_card());
   }
 
-  PaymentInstrument* instrument_;
+  AutofillPaymentInstrument* instrument_;

[anthonyvd, 2017/04/07 20:08:09]

I don't think we want this, even if it's a temporary thing. Is there a way we
can make this instrument type agnostic right away?

What about adding a function on PaymentInstrument (something like
instrument_type()) that returns an enum value corresponding to the concrete
instrument's type? This way we can switch here to create derived
PaymentMethodListItem types.

[Mathieu, 2017/04/09 00:35:45]

You're right! Added a switch on the type() so that this will fail to compile if
we add a new instrument type.

   PaymentRequestDialogView* dialog_;
 
   DISALLOW_COPY_AND_ASSIGN(PaymentMethodListItem);
@@ -120,9 +118,12 @@ PaymentMethodViewController::PaymentMethodViewController(
 
   for (const std::unique_ptr<PaymentInstrument>& instrument :
        available_instruments) {
+    // TODO(crbug.com/709194): When this feature supports instruments that are
+    // not credit cards, the cast below is invalid.
     std::unique_ptr<PaymentMethodListItem> item =
         base::MakeUnique<PaymentMethodListItem>(
-            instrument.get(), spec, state, &payment_method_list_, dialog,
+            static_cast<AutofillPaymentInstrument*>(instrument.get()), spec,
+            state, &payment_method_list_, dialog,
             instrument.get() == state->selected_instrument());
     payment_method_list_.AddItem(std::move(item));
   }