rAc: better Wallet error messages.

rAc: better Wallet error messages.

Only the controller-side changes for now. The Views will have to implement the linkification, for now they just show the text with no linkifying.

BUG=266130, 266131, 260940
R=isherman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220631

[Ilya Sherman, 2013-08-29 00:26:11]

Could you add some test coverage?  Otherwise LGTM % nits.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/account_chooser_model.h (right):

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/account_chooser_model.h:84: bool HadWalletError()
const;
nit: inline/hacker_case?

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/account_chooser_model.h:119: bool wallet_error_;
nit: Perhaps |has_wallet_error_|?

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right):

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:96: const char
kKycStatusUrl[] = "https://wallet.google.com/kyc";
nit: Please eschew acronyms like "bla" and "kyc" in favor of something more
transparent / less jargony.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:376: break;
nit: This looks a little weird without a comment.  Ditto below.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:445: NOTREACHED();
nit: DCHECK(!text.empty());

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:449: return
scoped_ptr<DialogNotification>(new DialogNotification(
nit: make_scoped_ptr

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_types.cc (right):

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_types.cc:35: }
Please add test coverage for this logic.

[Evan Stade, 2013-08-30 02:52:21]

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/account_chooser_model.h (right):

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/account_chooser_model.h:84: bool HadWalletError()
const;
On 2013/08/29 00:26:11, Ilya Sherman wrote:
> nit: inline/hacker_case?

Done.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/account_chooser_model.h:119: bool wallet_error_;
On 2013/08/29 00:26:11, Ilya Sherman wrote:
> nit: Perhaps |has_wallet_error_|?

Done.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right):

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:96: const char
kKycStatusUrl[] = "https://wallet.google.com/kyc";
On 2013/08/29 00:26:11, Ilya Sherman wrote:
> nit: Please eschew acronyms like "bla" and "kyc" in favor of something more
> transparent / less jargony.

Done.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:376: break;
On 2013/08/29 00:26:11, Ilya Sherman wrote:
> nit: This looks a little weird without a comment.  Ditto below.

Done.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:445: NOTREACHED();
On 2013/08/29 00:26:11, Ilya Sherman wrote:
> nit: DCHECK(!text.empty());

Done.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:449: return
scoped_ptr<DialogNotification>(new DialogNotification(
On 2013/08/29 00:26:11, Ilya Sherman wrote:
> nit: make_scoped_ptr

Done.

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_types.cc (right):

https://codereview.chromium.org/23495006/diff/3001/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_types.cc:35: }
On 2013/08/29 00:26:11, Ilya Sherman wrote:
> Please add test coverage for this logic.

Done.

[commit-bot: I haz the power, 2013-08-30 02:52:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/23495006/11001

[commit-bot: I haz the power, 2013-08-30 04:30:19]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[Evan Stade, 2013-08-30 19:41:12]

Committed patchset #3 manually as r220631 (presubmit successful).