Commit a1315a2d authored by estade's avatar estade Committed by Commit bot

Add checkbox to card unmasking prompt

- Add pref to make checkbox sticky
- Implement checkbox as per-card, not per-device
- Implement UI on Views

BUG=448587

Review URL: https://codereview.chromium.org/910043002

Cr-Commit-Position: refs/heads/master@{#316298}
parent bf9908c5
...@@ -11104,6 +11104,9 @@ Chrome ran out of memory. ...@@ -11104,6 +11104,9 @@ Chrome ran out of memory.
<message name="IDS_AUTOFILL_CARD_UNMASK_PROMPT_INSTRUCTIONS_EXPIRED_AMEX" desc="Text explaining what the user should do in the card unmasking dialog to update an expired amex card."> <message name="IDS_AUTOFILL_CARD_UNMASK_PROMPT_INSTRUCTIONS_EXPIRED_AMEX" desc="Text explaining what the user should do in the card unmasking dialog to update an expired amex card.">
Enter the expiration date and four digit verification code from the front of your credit card Enter the expiration date and four digit verification code from the front of your credit card
</message> </message>
<message name="IDS_AUTOFILL_CARD_UNMASK_PROMPT_STORAGE_CHECKBOX" desc="Text for checkbox in card unmasking dialog that allows user to store a Wallet card on their local device. If checked, the dialog won't show up again for the given credit card.">
Don't ask again for this card
</message>
<message name="IDS_APPEARANCE_GROUP_NAME" desc="The title of the appearance group"> <message name="IDS_APPEARANCE_GROUP_NAME" desc="The title of the appearance group">
Appearance Appearance
......
...@@ -67,7 +67,8 @@ void CardUnmaskPromptViewAndroid::OnUserInput(JNIEnv* env, ...@@ -67,7 +67,8 @@ void CardUnmaskPromptViewAndroid::OnUserInput(JNIEnv* env,
controller_->OnUnmaskResponse( controller_->OnUnmaskResponse(
base::android::ConvertJavaStringToUTF16(env, cvc), base::android::ConvertJavaStringToUTF16(env, cvc),
base::android::ConvertJavaStringToUTF16(env, month), base::android::ConvertJavaStringToUTF16(env, month),
base::android::ConvertJavaStringToUTF16(env, year)); base::android::ConvertJavaStringToUTF16(env, year),
false);
} }
void CardUnmaskPromptViewAndroid::PromptDismissed(JNIEnv* env, jobject obj) { void CardUnmaskPromptViewAndroid::PromptDismissed(JNIEnv* env, jobject obj) {
......
...@@ -19,7 +19,8 @@ class CardUnmaskPromptController { ...@@ -19,7 +19,8 @@ class CardUnmaskPromptController {
virtual void OnUnmaskDialogClosed() = 0; virtual void OnUnmaskDialogClosed() = 0;
virtual void OnUnmaskResponse(const base::string16& cvc, virtual void OnUnmaskResponse(const base::string16& cvc,
const base::string16& exp_month, const base::string16& exp_month,
const base::string16& exp_year) = 0; const base::string16& exp_year,
bool should_store_pan) = 0;
// State. // State.
virtual content::WebContents* GetWebContents() = 0; virtual content::WebContents* GetWebContents() = 0;
...@@ -27,6 +28,7 @@ class CardUnmaskPromptController { ...@@ -27,6 +28,7 @@ class CardUnmaskPromptController {
virtual base::string16 GetInstructionsMessage() const = 0; virtual base::string16 GetInstructionsMessage() const = 0;
virtual int GetCvcImageRid() const = 0; virtual int GetCvcImageRid() const = 0;
virtual bool ShouldRequestExpirationDate() const = 0; virtual bool ShouldRequestExpirationDate() const = 0;
virtual bool GetStoreLocallyStartState() const = 0;
virtual bool InputTextIsValid(const base::string16& input_text) const = 0; virtual bool InputTextIsValid(const base::string16& input_text) const = 0;
}; };
......
...@@ -5,11 +5,15 @@ ...@@ -5,11 +5,15 @@
#include "chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.h" #include "chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/prefs/pref_service.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/autofill/risk_util.h" #include "chrome/browser/autofill/risk_util.h"
#include "chrome/browser/ui/autofill/card_unmask_prompt_view.h" #include "chrome/browser/ui/autofill/card_unmask_prompt_view.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "components/autofill/core/common/autofill_pref_names.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/web_contents.h"
#include "grit/theme_resources.h" #include "grit/theme_resources.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -53,13 +57,19 @@ void CardUnmaskPromptControllerImpl::OnUnmaskDialogClosed() { ...@@ -53,13 +57,19 @@ void CardUnmaskPromptControllerImpl::OnUnmaskDialogClosed() {
void CardUnmaskPromptControllerImpl::OnUnmaskResponse( void CardUnmaskPromptControllerImpl::OnUnmaskResponse(
const base::string16& cvc, const base::string16& cvc,
const base::string16& exp_month, const base::string16& exp_month,
const base::string16& exp_year) { const base::string16& exp_year,
bool should_store_pan) {
card_unmask_view_->DisableAndWaitForVerification(); card_unmask_view_->DisableAndWaitForVerification();
DCHECK(!cvc.empty()); DCHECK(!cvc.empty());
pending_response_.cvc = cvc; pending_response_.cvc = cvc;
pending_response_.exp_month = exp_month; pending_response_.exp_month = exp_month;
pending_response_.exp_year = exp_year; pending_response_.exp_year = exp_year;
pending_response_.should_store_pan = should_store_pan;
// Remember the last choice the user made (on this device).
user_prefs::UserPrefs::Get(web_contents_->GetBrowserContext())->SetBoolean(
prefs::kAutofillWalletImportStorageCheckboxState, should_store_pan);
if (!pending_response_.risk_data.empty()) if (!pending_response_.risk_data.empty())
delegate_->OnUnmaskResponse(pending_response_); delegate_->OnUnmaskResponse(pending_response_);
} }
...@@ -102,6 +112,14 @@ bool CardUnmaskPromptControllerImpl::ShouldRequestExpirationDate() const { ...@@ -102,6 +112,14 @@ bool CardUnmaskPromptControllerImpl::ShouldRequestExpirationDate() const {
return card_.GetServerStatus() == CreditCard::EXPIRED; return card_.GetServerStatus() == CreditCard::EXPIRED;
} }
bool CardUnmaskPromptControllerImpl::GetStoreLocallyStartState() const {
// TODO(estade): Don't even offer to save on Linux? Offer to save but
// default to false?
PrefService* prefs =
user_prefs::UserPrefs::Get(web_contents_->GetBrowserContext());
return prefs->GetBoolean(prefs::kAutofillWalletImportStorageCheckboxState);
}
bool CardUnmaskPromptControllerImpl::InputTextIsValid( bool CardUnmaskPromptControllerImpl::InputTextIsValid(
const base::string16& input_text) const { const base::string16& input_text) const {
base::string16 trimmed_text; base::string16 trimmed_text;
......
...@@ -30,13 +30,15 @@ class CardUnmaskPromptControllerImpl : public CardUnmaskPromptController { ...@@ -30,13 +30,15 @@ class CardUnmaskPromptControllerImpl : public CardUnmaskPromptController {
void OnUnmaskDialogClosed() override; void OnUnmaskDialogClosed() override;
void OnUnmaskResponse(const base::string16& cvc, void OnUnmaskResponse(const base::string16& cvc,
const base::string16& exp_month, const base::string16& exp_month,
const base::string16& exp_year) override; const base::string16& exp_year,
bool should_store_pan) override;
content::WebContents* GetWebContents() override; content::WebContents* GetWebContents() override;
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
base::string16 GetInstructionsMessage() const override; base::string16 GetInstructionsMessage() const override;
int GetCvcImageRid() const override; int GetCvcImageRid() const override;
bool ShouldRequestExpirationDate() const override; bool ShouldRequestExpirationDate() const override;
bool GetStoreLocallyStartState() const override;
bool InputTextIsValid(const base::string16& input_text) const override; bool InputTextIsValid(const base::string16& input_text) const override;
private: private:
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "grit/theme_resources.h" #include "grit/theme_resources.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
#include "ui/views/controls/button/checkbox.h"
#include "ui/views/controls/combobox/combobox.h" #include "ui/views/controls/combobox/combobox.h"
#include "ui/views/controls/combobox/combobox_listener.h" #include "ui/views/controls/combobox/combobox_listener.h"
#include "ui/views/controls/image_view.h" #include "ui/views/controls/image_view.h"
...@@ -37,7 +38,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView, ...@@ -37,7 +38,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView,
cvc_input_(nullptr), cvc_input_(nullptr),
month_input_(nullptr), month_input_(nullptr),
year_input_(nullptr), year_input_(nullptr),
message_label_(nullptr) {} message_label_(nullptr),
storage_checkbox_(nullptr) {}
~CardUnmaskPromptViews() override { ~CardUnmaskPromptViews() override {
if (controller_) if (controller_)
...@@ -158,7 +160,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView, ...@@ -158,7 +160,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView,
: base::string16(), : base::string16(),
year_input_ year_input_
? year_combobox_model_.GetItemAt(year_input_->selected_index()) ? year_combobox_model_.GetItemAt(year_input_->selected_index())
: base::string16()); : base::string16(),
storage_checkbox_ ? storage_checkbox_->checked() : false);
return false; return false;
} }
...@@ -218,6 +221,11 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView, ...@@ -218,6 +221,11 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView,
message_label_ = new views::Label(); message_label_ = new views::Label();
input_row->AddChildView(message_label_); input_row->AddChildView(message_label_);
message_label_->SetVisible(false); message_label_->SetVisible(false);
storage_checkbox_ = new views::Checkbox(l10n_util::GetStringUTF16(
IDS_AUTOFILL_CARD_UNMASK_PROMPT_STORAGE_CHECKBOX));
storage_checkbox_->SetChecked(controller_->GetStoreLocallyStartState());
AddChildView(storage_checkbox_);
} }
void ClosePrompt() { GetWidget()->Close(); } void ClosePrompt() { GetWidget()->Close(); }
...@@ -237,6 +245,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView, ...@@ -237,6 +245,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView,
// as well as a better error message. // as well as a better error message.
views::Label* message_label_; views::Label* message_label_;
views::Checkbox* storage_checkbox_;
DISALLOW_COPY_AND_ASSIGN(CardUnmaskPromptViews); DISALLOW_COPY_AND_ASSIGN(CardUnmaskPromptViews);
}; };
......
...@@ -187,6 +187,11 @@ void AutofillManager::RegisterProfilePrefs( ...@@ -187,6 +187,11 @@ void AutofillManager::RegisterProfilePrefs(
prefs::kAutofillWalletImportEnabled, prefs::kAutofillWalletImportEnabled,
true, true,
user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
// This choice is made on a per-device basis, so it's not syncable.
registry->RegisterBooleanPref(
prefs::kAutofillWalletImportStorageCheckboxState,
true,
user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
registry->RegisterBooleanPref( registry->RegisterBooleanPref(
prefs::kAutofillAuxiliaryProfilesEnabled, prefs::kAutofillAuxiliaryProfilesEnabled,
...@@ -716,7 +721,7 @@ void AutofillManager::OnLoadedServerPredictions( ...@@ -716,7 +721,7 @@ void AutofillManager::OnLoadedServerPredictions(
} }
void AutofillManager::OnUnmaskResponse(const UnmaskResponse& response) { void AutofillManager::OnUnmaskResponse(const UnmaskResponse& response) {
unmasking_cvc_ = response.cvc; unmask_response_ = response;
// TODO(estade): use month/year. // TODO(estade): use month/year.
real_pan_client_.UnmaskCard(unmasking_card_, base::UTF16ToASCII(response.cvc), real_pan_client_.UnmaskCard(unmasking_card_, base::UTF16ToASCII(response.cvc),
response.risk_data); response.risk_data);
...@@ -726,7 +731,7 @@ void AutofillManager::OnUnmaskPromptClosed() { ...@@ -726,7 +731,7 @@ void AutofillManager::OnUnmaskPromptClosed() {
real_pan_client_.CancelRequest(); real_pan_client_.CancelRequest();
driver_->RendererShouldClearPreviewedForm(); driver_->RendererShouldClearPreviewedForm();
unmasking_card_ = CreditCard(); unmasking_card_ = CreditCard();
unmasking_cvc_.clear(); unmask_response_ = UnmaskResponse();
} }
IdentityProvider* AutofillManager::GetIdentityProvider() { IdentityProvider* AutofillManager::GetIdentityProvider() {
...@@ -737,7 +742,9 @@ void AutofillManager::OnDidGetRealPan(const std::string& real_pan) { ...@@ -737,7 +742,9 @@ void AutofillManager::OnDidGetRealPan(const std::string& real_pan) {
if (!real_pan.empty()) { if (!real_pan.empty()) {
unmasking_card_.set_record_type(CreditCard::FULL_SERVER_CARD); unmasking_card_.set_record_type(CreditCard::FULL_SERVER_CARD);
unmasking_card_.SetNumber(base::UTF8ToUTF16(real_pan)); unmasking_card_.SetNumber(base::UTF8ToUTF16(real_pan));
personal_data_->UpdateServerCreditCard(unmasking_card_); if (unmask_response_.should_store_pan)
personal_data_->UpdateServerCreditCard(unmasking_card_);
FillCreditCardForm(unmasking_query_id_, unmasking_form_, unmasking_field_, FillCreditCardForm(unmasking_query_id_, unmasking_form_, unmasking_field_,
unmasking_card_); unmasking_card_);
} }
...@@ -1068,10 +1075,10 @@ void AutofillManager::FillOrPreviewDataModelForm( ...@@ -1068,10 +1075,10 @@ void AutofillManager::FillOrPreviewDataModelForm(
if (is_credit_card && if (is_credit_card &&
cached_field->Type().GetStorableType() == cached_field->Type().GetStorableType() ==
CREDIT_CARD_VERIFICATION_CODE) { CREDIT_CARD_VERIFICATION_CODE) {
// If this is |unmasking_card_|, |unmasking_cvc_| should be non-empty // If this is |unmasking_card_|, |unmask_response_,cvc| should be
// and vice versa. // non-empty and vice versa.
DCHECK_EQ(&unmasking_card_ == data_model, !unmasking_cvc_.empty()); value = unmask_response_.cvc;
value = unmasking_cvc_; DCHECK_EQ(&unmasking_card_ == data_model, value.empty());
} }
// Must match ForEachMatchingFormField() in form_autofill_util.cc. // Must match ForEachMatchingFormField() in form_autofill_util.cc.
......
...@@ -371,8 +371,8 @@ class AutofillManager : public AutofillDownloadManager::Observer, ...@@ -371,8 +371,8 @@ class AutofillManager : public AutofillDownloadManager::Observer,
// A copy of the credit card that's currently being unmasked, and data about // A copy of the credit card that's currently being unmasked, and data about
// the form. // the form.
CreditCard unmasking_card_; CreditCard unmasking_card_;
// CVC is not part of CreditCard, so store it separately. // A copy of the latest card unmasking response.
base::string16 unmasking_cvc_; UnmaskResponse unmask_response_;
int unmasking_query_id_; int unmasking_query_id_;
FormData unmasking_form_; FormData unmasking_form_;
FormFieldData unmasking_field_; FormFieldData unmasking_field_;
......
...@@ -6,7 +6,8 @@ ...@@ -6,7 +6,8 @@
namespace autofill { namespace autofill {
CardUnmaskDelegate::UnmaskResponse::UnmaskResponse() {} CardUnmaskDelegate::UnmaskResponse::UnmaskResponse()
: should_store_pan(false) {}
CardUnmaskDelegate::UnmaskResponse::~UnmaskResponse() {} CardUnmaskDelegate::UnmaskResponse::~UnmaskResponse() {}
} // namespace autofill } // namespace autofill
...@@ -21,6 +21,7 @@ class CardUnmaskDelegate { ...@@ -21,6 +21,7 @@ class CardUnmaskDelegate {
base::string16 cvc; base::string16 cvc;
base::string16 exp_month; base::string16 exp_month;
base::string16 exp_year; base::string16 exp_year;
bool should_store_pan;
// Risk fingerprint. // Risk fingerprint.
std::string risk_data; std::string risk_data;
......
...@@ -43,6 +43,11 @@ const char kAutofillUseMacAddressBook[] = "autofill.use_mac_address_book"; ...@@ -43,6 +43,11 @@ const char kAutofillUseMacAddressBook[] = "autofill.use_mac_address_book";
// enabled. // enabled.
const char kAutofillWalletImportEnabled[] = "autofill.wallet_import_enabled"; const char kAutofillWalletImportEnabled[] = "autofill.wallet_import_enabled";
// Boolean that allows the "Don't ask again for this card" checkbox to be
// sticky.
const char kAutofillWalletImportStorageCheckboxState[] =
"autofill.wallet_import_storage_checkbox_state";
// Enables/disables the Wallet card and address feature. Set via sync // Enables/disables the Wallet card and address feature. Set via sync
// experiment. Even if this is false, the feature can still be enabled via the // experiment. Even if this is false, the feature can still be enabled via the
// command line flag flags::kEnableWalletCardImport. // command line flag flags::kEnableWalletCardImport.
......
...@@ -18,6 +18,7 @@ extern const char kAutofillNegativeUploadRate[]; ...@@ -18,6 +18,7 @@ extern const char kAutofillNegativeUploadRate[];
extern const char kAutofillPositiveUploadRate[]; extern const char kAutofillPositiveUploadRate[];
extern const char kAutofillUseMacAddressBook[]; extern const char kAutofillUseMacAddressBook[];
extern const char kAutofillWalletImportEnabled[]; extern const char kAutofillWalletImportEnabled[];
extern const char kAutofillWalletImportStorageCheckboxState[];
extern const char kAutofillWalletSyncExperimentEnabled[]; extern const char kAutofillWalletSyncExperimentEnabled[];
} // namespace prefs } // namespace prefs
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment