diff options
5 files changed, 132 insertions, 13 deletions
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc index 59bf9ea..a5f0266 100644 --- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc +++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc @@ -95,6 +95,9 @@ const char kSameAsBillingKey[] = "same-as-billing"; // time. const char kAutofillDialogOrigin[] = "Chrome Autofill dialog"; +// HSL shift to gray out an image. +const color_utils::HSL kGrayImageShift = {-1, 0, 0.8}; + // Returns true if |input| should be shown when |field_type| has been requested. bool InputTypeMatchesFieldType(const DetailInput& input, AutofillFieldType field_type) { @@ -236,6 +239,15 @@ string16 GetValueForType(const DetailOutputMap& output, return string16(); } +// Check if a given MaskedInstrument is allowed for the purchase. +bool IsInstrumentAllowed( + const wallet::WalletItems::MaskedInstrument& instrument) { + return (instrument.status() == wallet::WalletItems::MaskedInstrument::VALID || + instrument.status() == wallet::WalletItems::MaskedInstrument::PENDING) && + instrument.type() != wallet::WalletItems::MaskedInstrument::AMEX && + instrument.type() != wallet::WalletItems::MaskedInstrument::UNKNOWN; +} + // Signals that the user has opted in to geolocation services. Factored out // into a separate method because all interaction with the geolocation provider // needs to happen on the IO thread, which is not the thread @@ -1047,10 +1059,9 @@ gfx::Image AutofillDialogControllerImpl::IconForField( int idr = card_idrs[i]; gfx::ImageSkia card_image = *rb.GetImageSkiaNamed(idr); if (card.IconResourceId() != idr) { - color_utils::HSL shift = {-1, 0, 0.8}; SkBitmap disabled_bitmap = SkBitmapOperations::CreateHSLShiftedBitmap(*card_image.bitmap(), - shift); + kGrayImageShift); card_image = gfx::ImageSkia::CreateFrom1xBitmap(disabled_bitmap); } @@ -1819,17 +1830,33 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() { if (!IsSubmitPausedOn(wallet::VERIFY_CVV)) { const std::vector<wallet::WalletItems::MaskedInstrument*>& instruments = wallet_items_->instruments(); + std::string first_active_instrument_key; + std::string default_instrument_key; for (size_t i = 0; i < instruments.size(); ++i) { + bool allowed = IsInstrumentAllowed(*instruments[i]); + gfx::Image icon = instruments[i]->CardIcon(); + if (!allowed && !icon.IsEmpty()) { + // Create a grayed disabled icon. + SkBitmap disabled_bitmap = SkBitmapOperations::CreateHSLShiftedBitmap( + *icon.ToSkBitmap(), kGrayImageShift); + icon = gfx::Image( + gfx::ImageSkia::CreateFrom1xBitmap(disabled_bitmap)); + } std::string key = base::IntToString(i); suggested_cc_billing_.AddKeyedItemWithSublabelAndIcon( key, instruments[i]->DisplayName(), instruments[i]->DisplayNameDetail(), - instruments[i]->CardIcon()); - - if (instruments[i]->object_id() == - wallet_items_->default_instrument_id()) { - suggested_cc_billing_.SetCheckedItem(key); + icon); + suggested_cc_billing_.SetEnabled(key, allowed); + + if (allowed) { + if (first_active_instrument_key.empty()) + first_active_instrument_key = key; + if (instruments[i]->object_id() == + wallet_items_->default_instrument_id()) { + default_instrument_key = key; + } } } @@ -1841,6 +1868,14 @@ void AutofillDialogControllerImpl::SuggestionsUpdated() { kManageItemsKey, l10n_util::GetStringUTF16( IDS_AUTOFILL_DIALOG_MANAGE_BILLING_DETAILS)); + + // Determine which instrument item should be selected. + if (!default_instrument_key.empty()) + suggested_cc_billing_.SetCheckedItem(default_instrument_key); + else if (!first_active_instrument_key.empty()) + suggested_cc_billing_.SetCheckedItem(first_active_instrument_key); + else + suggested_cc_billing_.SetCheckedItem(kAddNewItemKey); } } else { PersonalDataManager* manager = GetManager(); diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc index 423badc..ae90f33 100644 --- a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc +++ b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc @@ -643,6 +643,52 @@ TEST_F(AutofillDialogControllerTest, WalletDefaultItems) { IsItemCheckedAt(4)); } +// Tests that invalid and AMEX default instruments are ignored. +TEST_F(AutofillDialogControllerTest, SelectInstrument) { + scoped_ptr<wallet::WalletItems> wallet_items = wallet::GetTestWalletItems(); + // Tests if default instrument is invalid, then, the first valid instrument is + // selected instead of the default instrument. + wallet_items->AddInstrument(wallet::GetTestNonDefaultMaskedInstrument()); + wallet_items->AddInstrument(wallet::GetTestNonDefaultMaskedInstrument()); + wallet_items->AddInstrument(wallet::GetTestMaskedInstrumentInvalid()); + wallet_items->AddInstrument(wallet::GetTestNonDefaultMaskedInstrument()); + + controller()->OnDidGetWalletItems(wallet_items.Pass()); + // 4 suggestions and "add", "manage". + EXPECT_EQ(6, + controller()->MenuModelForSection(SECTION_CC_BILLING)->GetItemCount()); + EXPECT_TRUE(controller()->MenuModelForSection(SECTION_CC_BILLING)-> + IsItemCheckedAt(0)); + + // Tests if default instrument is AMEX, then, the first valid instrument is + // selected instead of the default instrument. + wallet_items = wallet::GetTestWalletItems(); + wallet_items->AddInstrument(wallet::GetTestNonDefaultMaskedInstrument()); + wallet_items->AddInstrument(wallet::GetTestNonDefaultMaskedInstrument()); + wallet_items->AddInstrument(wallet::GetTestMaskedInstrumentAmex()); + wallet_items->AddInstrument(wallet::GetTestNonDefaultMaskedInstrument()); + + controller()->OnDidGetWalletItems(wallet_items.Pass()); + // 4 suggestions and "add", "manage". + EXPECT_EQ(6, + controller()->MenuModelForSection(SECTION_CC_BILLING)->GetItemCount()); + EXPECT_TRUE(controller()->MenuModelForSection(SECTION_CC_BILLING)-> + IsItemCheckedAt(0)); + + // Tests if only have AMEX and invalid instrument, then "add" is selected. + wallet_items = wallet::GetTestWalletItems(); + wallet_items->AddInstrument(wallet::GetTestMaskedInstrumentInvalid()); + wallet_items->AddInstrument(wallet::GetTestMaskedInstrumentAmex()); + + controller()->OnDidGetWalletItems(wallet_items.Pass()); + // 2 suggestions and "add", "manage". + EXPECT_EQ(4, + controller()->MenuModelForSection(SECTION_CC_BILLING)->GetItemCount()); + // "add" + EXPECT_TRUE(controller()->MenuModelForSection(SECTION_CC_BILLING)-> + IsItemCheckedAt(2)); +} + TEST_F(AutofillDialogControllerTest, SaveAddress) { EXPECT_CALL(*controller()->GetView(), ModelChanged()).Times(1); EXPECT_CALL(*controller()->GetTestingWalletClient(), @@ -665,6 +711,18 @@ TEST_F(AutofillDialogControllerTest, SaveInstrument) { controller()->OnAccept(); } +TEST_F(AutofillDialogControllerTest, SaveInstrumentWithInvalidInstruments) { + EXPECT_CALL(*controller()->GetView(), ModelChanged()).Times(1); + EXPECT_CALL(*controller()->GetTestingWalletClient(), + SaveInstrument(_, _, _)).Times(1); + + scoped_ptr<wallet::WalletItems> wallet_items = wallet::GetTestWalletItems(); + wallet_items->AddAddress(wallet::GetTestShippingAddress()); + wallet_items->AddInstrument(wallet::GetTestMaskedInstrumentInvalid()); + controller()->OnDidGetWalletItems(wallet_items.Pass()); + controller()->OnAccept(); +} + TEST_F(AutofillDialogControllerTest, SaveInstrumentAndAddress) { EXPECT_CALL(*controller()->GetTestingWalletClient(), SaveInstrumentAndAddress(_, _, _, _)).Times(1); diff --git a/components/autofill/browser/wallet/wallet_items.h b/components/autofill/browser/wallet/wallet_items.h index 14c4521..f9707da 100644 --- a/components/autofill/browser/wallet/wallet_items.h +++ b/components/autofill/browser/wallet/wallet_items.h @@ -108,8 +108,8 @@ class WalletItems { private: friend class WalletItemsTest; - friend scoped_ptr<MaskedInstrument> GetTestMaskedInstrumentWithId( - const std::string&); + friend scoped_ptr<MaskedInstrument> GetTestMaskedInstrumentWithDetails( + const std::string&, Type type, Status status); FRIEND_TEST_ALL_PREFIXES(::autofill::WalletInstrumentWrapperTest, GetInfoCreditCardExpMonth); FRIEND_TEST_ALL_PREFIXES(::autofill::WalletInstrumentWrapperTest, diff --git a/components/autofill/browser/wallet/wallet_test_util.cc b/components/autofill/browser/wallet/wallet_test_util.cc index f34e82a..22a05af 100644 --- a/components/autofill/browser/wallet/wallet_test_util.cc +++ b/components/autofill/browser/wallet/wallet_test_util.cc @@ -28,20 +28,30 @@ int FutureYear() { } // namespace -scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentWithId( - const std::string& id) { +scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentWithDetails( + const std::string& id, + WalletItems::MaskedInstrument::Type type, + WalletItems::MaskedInstrument::Status status) { return scoped_ptr<WalletItems::MaskedInstrument>( new WalletItems::MaskedInstrument(ASCIIToUTF16("descriptive_name"), - WalletItems::MaskedInstrument::VISA, + type, std::vector<base::string16>(), ASCIIToUTF16("1111"), 12, FutureYear(), GetTestAddress(), - WalletItems::MaskedInstrument::VALID, + status, id)); } +scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentWithId( + const std::string& id) { + return GetTestMaskedInstrumentWithDetails( + id, + WalletItems::MaskedInstrument::VISA, + WalletItems::MaskedInstrument::VALID); +} + scoped_ptr<Address> GetTestAddress() { return scoped_ptr<Address>(new Address("US", ASCIIToUTF16("recipient_name"), @@ -86,6 +96,20 @@ scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrument() { return GetTestMaskedInstrumentWithId("default_instrument_id"); } +scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentInvalid() { + return GetTestMaskedInstrumentWithDetails( + "default_instrument_id", + WalletItems::MaskedInstrument::VISA, + WalletItems::MaskedInstrument::DECLINED); +} + +scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentAmex() { + return GetTestMaskedInstrumentWithDetails( + "default_instrument_id", + WalletItems::MaskedInstrument::AMEX, + WalletItems::MaskedInstrument::VALID); +} + scoped_ptr<WalletItems::MaskedInstrument> GetTestNonDefaultMaskedInstrument() { return GetTestMaskedInstrumentWithId("instrument_id"); } diff --git a/components/autofill/browser/wallet/wallet_test_util.h b/components/autofill/browser/wallet/wallet_test_util.h index 316c5d7..75f4e56 100644 --- a/components/autofill/browser/wallet/wallet_test_util.h +++ b/components/autofill/browser/wallet/wallet_test_util.h @@ -20,6 +20,8 @@ scoped_ptr<FullWallet> GetTestFullWallet(); scoped_ptr<Instrument> GetTestInstrument(); scoped_ptr<WalletItems::LegalDocument> GetTestLegalDocument(); scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrument(); +scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentInvalid(); +scoped_ptr<WalletItems::MaskedInstrument> GetTestMaskedInstrumentAmex(); scoped_ptr<WalletItems::MaskedInstrument> GetTestNonDefaultMaskedInstrument(); scoped_ptr<Address> GetTestSaveableAddress(); scoped_ptr<Address> GetTestShippingAddress(); |