Skip to content

Commit

Permalink
Handle invalid instrument and AMEX from wallet in autofill dialog.
Browse files Browse the repository at this point in the history 
BUG=233048

Review URL: https://chromiumcodereview.appspot.com/15216003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@201673 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
benquan@chromium.org committed May 23, 2013
1 parent 30d73a5 commit 45882fd
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 13 deletions.
49 changes: 42 additions & 7 deletions chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
}
}

Expand All @@ -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();
Expand Down
58 changes: 58 additions & 0 deletions chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions components/autofill/browser/wallet/wallet_items.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
32 changes: 28 additions & 4 deletions components/autofill/browser/wallet/wallet_test_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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");
}
Expand Down
2 changes: 2 additions & 0 deletions components/autofill/browser/wallet/wallet_test_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 45882fd

Please sign in to comment.