Skip to content

Commit

Permalink
Displaying Wallet errors to the user.
Browse files Browse the repository at this point in the history
BUG=164410
TBR=sail@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206192 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ahutter@chromium.org committed Jun 13, 2013
1 parent a83eeaf commit 4dbecc3
Show file tree
Hide file tree
Showing 18 changed files with 144 additions and 121 deletions.
14 changes: 14 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -10745,6 +10745,20 @@ Would you like to start <ph name="CONTROL_PANEL_APPLET_NAME">$1<ex>Add/Remove Pr
Warning: You are not connected to production Wallet servers. Issued cards are probably invalid.
</message>

<!--- Autofill dialog: Wallet error messages -->
<message name="IDS_AUTOFILL_WALLET_BUYER_ACCOUNT_ERROR" desc="Text explaining that the user's Wallet account cannot be used.">
There's something wrong with your Google Wallet account.
</message>
<message name="IDS_AUTOFILL_WALLET_UPGRADE_CHROME_ERROR" desc="Text explaining that user must upgrade Chrome to use Wallet.">
You must upgrade Chrome to use Google Wallet.
</message>
<message name="IDS_AUTOFILL_WALLET_SERVICE_UNAVAILABLE_ERROR" desc="Text explaining that Wallet is currently unavailable.">
Google Wallet is currently unavailable.
</message>
<message name="IDS_AUTOFILL_WALLET_UNKNOWN_ERROR" desc="Text explaining that Wallet encountered an unknown error.">
Google Wallet has encounted an unknown error.
</message>

<!-- Autofill dialog: detail section -->
<message name="IDS_AUTOFILL_DIALOG_EDIT" desc="Text of link in autofill dialog which allows user to edit address + payment info.">
Edit
Expand Down
11 changes: 7 additions & 4 deletions chrome/browser/ui/autofill/account_chooser_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ AccountChooserModel::AccountChooserModel(
checked_item_(
prefs->GetBoolean(::prefs::kAutofillDialogPayWithoutWallet) ?
kAutofillItemId : kActiveWalletItemId),
had_wallet_error_(false),
metric_logger_(metric_logger),
dialog_type_(dialog_type) {
ReconstructMenuItems();
Expand Down Expand Up @@ -74,7 +73,7 @@ bool AccountChooserModel::IsCommandIdChecked(int command_id) const {

bool AccountChooserModel::IsCommandIdEnabled(int command_id) const {
// Currently, _any_ (non-sign-in) error disables _all_ Wallet accounts.
if (command_id != kAutofillItemId && had_wallet_error_)
if (command_id != kAutofillItemId && HadWalletError())
return false;

return true;
Expand Down Expand Up @@ -109,13 +108,17 @@ void AccountChooserModel::ExecuteCommand(int command_id, int event_flags) {
delegate_->AccountChoiceChanged();
}

void AccountChooserModel::SetHadWalletError() {
void AccountChooserModel::SetHadWalletError(const base::string16& message) {
// Any non-sign-in error disables all Wallet accounts.
had_wallet_error_ = true;
wallet_error_message_ = message;
ClearActiveWalletAccountName();
ExecuteCommand(kAutofillItemId, 0);
}

bool AccountChooserModel::HadWalletError() const {
return !wallet_error_message_.empty();
}

void AccountChooserModel::SetHadWalletSigninError() {
ClearActiveWalletAccountName();
ExecuteCommand(kAutofillItemId, 0);
Expand Down
17 changes: 10 additions & 7 deletions chrome/browser/ui/autofill/account_chooser_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,16 @@ class AccountChooserModel : public ui::SimpleMenuModel,
}

// Disables all Wallet accounts and switches to the local Autofill data.
// Should be called when the Wallet server returns an error.
void SetHadWalletError();
// Should be called when the Wallet server returns an error with the message
// to be displayed. If |message| is empty the error state will be cleared.
void SetHadWalletError(const base::string16& message);

bool HadWalletError() const;

// Switches the dialog to the local Autofill data.
// Should be called when the Online Wallet sign-in attempt has failed.
void SetHadWalletSigninError();

bool had_wallet_error() const { return had_wallet_error_; }

// Returns true if the selected account is an Online Wallet account.
bool WalletIsSelected() const;

Expand All @@ -97,6 +98,8 @@ class AccountChooserModel : public ui::SimpleMenuModel,
// Returns the command id of the current selection.
int checked_item() const { return checked_item_; }

base::string16 wallet_error_message() const { return wallet_error_message_; }

protected:
// Command IDs of the items in this menu; protected for the tests.
// kActiveWalletItemId is the currently active account.
Expand All @@ -115,9 +118,9 @@ class AccountChooserModel : public ui::SimpleMenuModel,
// The command id of the currently selected item.
int checked_item_;

// Whether there has been a Wallet error while the owning dialog has been
// open.
bool had_wallet_error_;
// The message to be displayed if there is a Wallet error. This message is
// only non-empty if a Wallet error has occurred.
base::string16 wallet_error_message_;

// For logging UMA metrics.
const AutofillMetrics& metric_logger_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ TEST_F(AccountChooserModelTest, HandlesError) {
ASSERT_TRUE(model()->IsCommandIdEnabled(
TestAccountChooserModel::kActiveWalletItemId));

model()->SetHadWalletError();
model()->SetHadWalletError(ASCIIToUTF16("Error"));
EXPECT_FALSE(model()->WalletIsSelected());
EXPECT_FALSE(model()->IsCommandIdEnabled(
TestAccountChooserModel::kActiveWalletItemId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest, AutocheckoutError) {

IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest, FillInputFromAutofill) {
InitializeControllerOfType(DIALOG_TYPE_REQUEST_AUTOCOMPLETE);
controller()->DisableWallet();
controller()->DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);

AutofillProfile full_profile(test::GetFullProfile());
controller()->GetTestingManager()->AddTestingProfile(&full_profile);
Expand Down Expand Up @@ -361,7 +361,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest,
// expected when Autofill is used to fill text inputs.
IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest, FillComboboxFromAutofill) {
InitializeControllerOfType(DIALOG_TYPE_REQUEST_AUTOCOMPLETE);
controller()->DisableWallet();
controller()->DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);

CreditCard card1;
test::SetCreditCardInfo(&card1, "JJ Smith", "4111111111111111", "12", "2018");
Expand Down Expand Up @@ -485,7 +485,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest, WalletCreditCardDisabled) {
// Ensure that expired cards trigger invalid suggestions.
IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest, ExpiredCard) {
InitializeControllerOfType(DIALOG_TYPE_REQUEST_AUTOCOMPLETE);
controller()->DisableWallet();
controller()->DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);

CreditCard verified_card(test::GetCreditCard());
verified_card.set_origin("Chrome settings");
Expand Down
64 changes: 41 additions & 23 deletions chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,32 @@ DialogSection SectionFromLocation(wallet::FormFieldError::Location location) {
return SECTION_MAX;
}

base::string16 WalletErrorMessage(wallet::WalletClient::ErrorType error_type) {
switch (error_type) {
case wallet::WalletClient::BUYER_ACCOUNT_ERROR:
return l10n_util::GetStringUTF16(IDS_AUTOFILL_WALLET_BUYER_ACCOUNT_ERROR);

case wallet::WalletClient::BAD_REQUEST:
case wallet::WalletClient::INVALID_PARAMS:
case wallet::WalletClient::UNSUPPORTED_API_VERSION:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WALLET_UPGRADE_CHROME_ERROR);

case wallet::WalletClient::SERVICE_UNAVAILABLE:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WALLET_SERVICE_UNAVAILABLE_ERROR);

case wallet::WalletClient::INTERNAL_ERROR:
case wallet::WalletClient::MALFORMED_RESPONSE:
case wallet::WalletClient::NETWORK_ERROR:
case wallet::WalletClient::UNKNOWN_ERROR:
return l10n_util::GetStringUTF16(IDS_AUTOFILL_WALLET_UNKNOWN_ERROR);
}

NOTREACHED();
return base::string16();
}

} // namespace

AutofillDialogController::~AutofillDialogController() {}
Expand Down Expand Up @@ -596,7 +622,7 @@ string16 AutofillDialogControllerImpl::LegalDocumentsText() {
}

DialogSignedInState AutofillDialogControllerImpl::SignedInState() const {
if (account_chooser_model_.had_wallet_error())
if (account_chooser_model_.HadWalletError())
return SIGN_IN_DISABLED;

if (signin_helper_ || !wallet_items_)
Expand Down Expand Up @@ -778,7 +804,7 @@ void AutofillDialogControllerImpl::OnWalletFormFieldError(

// Unrecoverable validation errors.
if (!wallet_server_validation_recoverable_)
DisableWallet();
DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);

if (view_)
view_->UpdateForErrors();
Expand Down Expand Up @@ -915,7 +941,7 @@ ui::MenuModel* AutofillDialogControllerImpl::MenuModelForSectionHack(
ui::MenuModel* AutofillDialogControllerImpl::MenuModelForAccountChooser() {
// If there were unrecoverable Wallet errors, or if there are choices other
// than "Pay without the wallet", show the full menu.
if (account_chooser_model_.had_wallet_error() ||
if (account_chooser_model_.HadWalletError() ||
account_chooser_model_.HasAccountsToChoose()) {
return &account_chooser_model_;
}
Expand Down Expand Up @@ -1487,13 +1513,13 @@ std::vector<DialogNotification> AutofillDialogControllerImpl::
CurrentNotifications() {
std::vector<DialogNotification> notifications;

if (account_chooser_model_.had_wallet_error()) {
// TODO(dbeam): pass along the Wallet error or remove from the translation.
if (account_chooser_model_.HadWalletError()) {
// TODO(dbeam): figure out a way to dismiss this error after a while.
notifications.push_back(DialogNotification(
DialogNotification::WALLET_ERROR,
l10n_util::GetStringFUTF16(IDS_AUTOFILL_DIALOG_COMPLETE_WITHOUT_WALLET,
ASCIIToUTF16("[Wallet-Error]."))));
l10n_util::GetStringFUTF16(
IDS_AUTOFILL_DIALOG_COMPLETE_WITHOUT_WALLET,
account_chooser_model_.wallet_error_message())));
} else if (should_show_wallet_promo_) {
if (IsPayingWithWallet() && HasCompleteWallet()) {
notifications.push_back(DialogNotification(
Expand Down Expand Up @@ -1771,7 +1797,7 @@ void AutofillDialogControllerImpl::OnDidAuthenticateInstrument(bool success) {
if (success)
GetFullWallet();
else
DisableWallet();
DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);
}

void AutofillDialogControllerImpl::OnDidGetFullWallet(
Expand Down Expand Up @@ -1801,7 +1827,7 @@ void AutofillDialogControllerImpl::OnDidGetFullWallet(
break;

default:
DisableWallet();
DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);
break;
}
}
Expand Down Expand Up @@ -1915,16 +1941,7 @@ void AutofillDialogControllerImpl::OnDidUpdateInstrument(

void AutofillDialogControllerImpl::OnWalletError(
wallet::WalletClient::ErrorType error_type) {
// TODO(dbeam): Do something with |error_type|. http://crbug.com/164410
DisableWallet();
}

void AutofillDialogControllerImpl::OnMalformedResponse() {
DisableWallet();
}

void AutofillDialogControllerImpl::OnNetworkError(int response_code) {
DisableWallet();
DisableWallet(error_type);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -2108,13 +2125,14 @@ void AutofillDialogControllerImpl::OnWalletSigninError() {
LogDialogLatencyToShow();
}

void AutofillDialogControllerImpl::DisableWallet() {
void AutofillDialogControllerImpl::DisableWallet(
wallet::WalletClient::ErrorType error_type) {
signin_helper_.reset();
wallet_items_.reset();
wallet_errors_.clear();
GetWalletClient()->CancelRequests();
SetIsSubmitting(false);
account_chooser_model_.SetHadWalletError();
account_chooser_model_.SetHadWalletError(WalletErrorMessage(error_type));
}

void AutofillDialogControllerImpl::SuggestionsUpdated() {
Expand Down Expand Up @@ -2804,7 +2822,7 @@ void AutofillDialogControllerImpl::HandleSaveOrUpdateRequiredActions(
iter != required_actions.end(); ++iter) {
if (*iter != wallet::INVALID_FORM_FIELD) {
// TODO(dbeam): handle this more gracefully.
DisableWallet();
DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);
}
}

Expand Down Expand Up @@ -2853,7 +2871,7 @@ void AutofillDialogControllerImpl::FinishSubmit() {
// stop trying to pay with Wallet on future runs of the dialog. On the other
// hand, if there was an error that prevented the user from having the choice
// of using Wallet, leave the pref alone.
if (!account_chooser_model_.had_wallet_error() &&
if (!account_chooser_model_.HadWalletError() &&
account_chooser_model_.HasAccountsToChoose()) {
profile_->GetPrefs()->SetBoolean(
::prefs::kAutofillDialogPayWithoutWallet,
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,6 @@ class AutofillDialogControllerImpl : public AutofillDialogController,
const std::vector<wallet::FormFieldError>& form_field_errors) OVERRIDE;
virtual void OnWalletError(
wallet::WalletClient::ErrorType error_type) OVERRIDE;
virtual void OnMalformedResponse() OVERRIDE;
virtual void OnNetworkError(int response_code) OVERRIDE;

// PersonalDataManagerObserver implementation.
virtual void OnPersonalDataChanged() OVERRIDE;
Expand Down Expand Up @@ -266,7 +264,7 @@ class AutofillDialogControllerImpl : public AutofillDialogController,

// Call to disable communication to Online Wallet for this dialog.
// Exposed for testing.
void DisableWallet();
void DisableWallet(wallet::WalletClient::ErrorType error_type);

// Returns whether Wallet is the current data source. Exposed for testing.
virtual bool IsPayingWithWallet() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void MockCallback(const FormStructure*, const std::string&) {}
base::Bind(MockCallback)),
metric_logger_(metric_logger) ,
runner_(runner) {
DisableWallet();
DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);
}

virtual ~TestAutofillDialogController() {}
Expand Down
8 changes: 0 additions & 8 deletions components/autofill/browser/autocheckout_request_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,6 @@ void AutocheckoutRequestManager::OnWalletError(
// Nothing to be done. |error_type| is logged by |metric_logger_|.
}

void AutocheckoutRequestManager::OnMalformedResponse() {
// Nothing to be done.
}

void AutocheckoutRequestManager::OnNetworkError(int response_code) {
// Nothing to be done. |response_code| is logged by |metric_logger_|.
}

AutocheckoutRequestManager::AutocheckoutRequestManager(
net::URLRequestContextGetter* request_context_getter)
: wallet_client_(request_context_getter, this) {
Expand Down
2 changes: 0 additions & 2 deletions components/autofill/browser/autocheckout_request_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ class AutocheckoutRequestManager : public base::SupportsUserData::Data,
const std::vector<wallet::FormFieldError>& form_field_errors) OVERRIDE;
virtual void OnWalletError(
wallet::WalletClient::ErrorType error_type) OVERRIDE;
virtual void OnMalformedResponse() OVERRIDE;
virtual void OnNetworkError(int response_code) OVERRIDE;

private:
// |request_context_getter| is passed in to construct |wallet_client_|.
Expand Down
5 changes: 3 additions & 2 deletions components/autofill/browser/autofill_metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,9 @@ class AutofillMetrics {
WALLET_INVALID_PARAMS,
// Online Wallet is down.
WALLET_SERVICE_UNAVAILABLE,
// User needs make a cheaper transaction or not use Online Wallet.
WALLET_SPENDING_LIMIT_EXCEEDED,
// User needs make a cheaper transaction or not use Online Wallet. This
// value has been deprecated.
WALLET_SPENDING_LIMIT_EXCEEDED_DEPRECATED,
// The server API version of the request is no longer supported.
WALLET_UNSUPPORTED_API_VERSION,
// Catch all error type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void EncryptionEscrowClient::OnURLFetchComplete(
DVLOG(1) << "Response body: " << data;

if (source->GetResponseCode() != net::HTTP_OK) {
observer_->OnNetworkError(source->GetResponseCode());
observer_->OnNetworkError();
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class EncryptionEscrowClientObserver {

// Called when a request fails due to a network error or if the response was
// invalid.
virtual void OnNetworkError(int response_code) = 0;
virtual void OnNetworkError() = 0;

// Called when a request fails due to a malformed response.
virtual void OnMalformedResponse() = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class MockEncryptionEscrowClientObserver :
void(const std::string& escrow_handle));
MOCK_METHOD0(OnDidMakeRequest, void());
MOCK_METHOD0(OnMalformedResponse, void());
MOCK_METHOD1(OnNetworkError, void(int response_code));
MOCK_METHOD0(OnNetworkError, void());
};

} // namespace
Expand Down Expand Up @@ -119,7 +119,7 @@ class EncryptionEscrowClientTest : public testing::Test {

TEST_F(EncryptionEscrowClientTest, NetworkError) {
EXPECT_CALL(observer_, OnDidMakeRequest()).Times(1);
EXPECT_CALL(observer_, OnNetworkError(net::HTTP_UNAUTHORIZED)).Times(1);
EXPECT_CALL(observer_, OnNetworkError()).Times(1);

encryption_escrow_client_->EscrowInstrumentInformation(*instrument_,
"obfuscated_gaia_id");
Expand Down
Loading

0 comments on commit 4dbecc3

Please sign in to comment.