Disabling wallet and mocking out more of WalletClient while testing.

Disabling wallet and mocking out more of WalletClient while testing.

R=isherman@chromium.org
BUG=258817
TEST=less flakes

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

[Ilya Sherman, 2013-07-11 01:06:30]

https://chromiumcodereview.appspot.com/18888003/diff/5001/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/18888003/diff/5001/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc:693:
controller()->DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);
Why is this needed in just this one particular test?  Docs might be helpful.

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
File components/autofill/content/browser/wallet/test_wallet_client.h (right):

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
components/autofill/content/browser/wallet/test_wallet_client.h:15: class
TestWalletClient : public WalletClient {
nit: This class should probably be named "MockWalletClient" rather than
"TestWalletClient", since it's mostly got lots of methods mocked out.

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
components/autofill/content/browser/wallet/test_wallet_client.h:15: class
TestWalletClient : public WalletClient {
nit: Might be nice to document this class, including mentioning that methods
that would make network requests are all mocked out.

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
components/autofill/content/browser/wallet/test_wallet_client.h:61:
scoped_ptr<Address> billing_address) {
nit: OVERRIDE

[Dan Beam, 2013-07-11 01:29:36]

https://chromiumcodereview.appspot.com/18888003/diff/5001/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/18888003/diff/5001/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc:693:
controller()->DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);
On 2013/07/11 01:06:30, Ilya Sherman wrote:
> Why is this needed in just this one particular test?  Docs might be helpful.

there's 3 other cases in this file.  in theory it shouldn't be necessary now i
guess... it was a stopgap solution to blocking requests before.  do you want me
to remove?

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
File components/autofill/content/browser/wallet/test_wallet_client.h (right):

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
components/autofill/content/browser/wallet/test_wallet_client.h:15: class
TestWalletClient : public WalletClient {
On 2013/07/11 01:06:30, Ilya Sherman wrote:
> nit: Might be nice to document this class, including mentioning that methods
> that would make network requests are all mocked out.

Done.

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
components/autofill/content/browser/wallet/test_wallet_client.h:15: class
TestWalletClient : public WalletClient {
On 2013/07/11 01:06:30, Ilya Sherman wrote:
> nit: This class should probably be named "MockWalletClient" rather than
> "TestWalletClient", since it's mostly got lots of methods mocked out.

Done.

https://chromiumcodereview.appspot.com/18888003/diff/5001/components/autofill...
components/autofill/content/browser/wallet/test_wallet_client.h:61:
scoped_ptr<Address> billing_address) {
On 2013/07/11 01:06:30, Ilya Sherman wrote:
> nit: OVERRIDE

Done.

[Dan Beam, 2013-07-11 01:34:36]

https://chromiumcodereview.appspot.com/18888003/diff/5001/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
(right):

https://chromiumcodereview.appspot.com/18888003/diff/5001/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc:693:
controller()->DisableWallet(wallet::WalletClient::UNKNOWN_ERROR);
On 2013/07/11 01:29:36, Dan Beam wrote:
> On 2013/07/11 01:06:30, Ilya Sherman wrote:
> > Why is this needed in just this one particular test?  Docs might be helpful.
> 
> there's 3 other cases in this file.  in theory it shouldn't be necessary now i
> guess... it was a stopgap solution to blocking requests before.  do you want
me
> to remove?

and... they're gone.

[Ilya Sherman, 2013-07-11 03:26:22]

LGTM, thanks.

[commit-bot: I haz the power, 2013-07-11 03:53:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/18888003/18001

[commit-bot: I haz the power, 2013-07-11 05:07:48]

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

[commit-bot: I haz the power, 2013-07-11 05:19:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/18888003/18001

[commit-bot: I haz the power, 2013-07-11 06:01:51]

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

[commit-bot: I haz the power, 2013-07-11 17:47:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/18888003/18001

[commit-bot: I haz the power, 2013-07-11 18:46:41]

Change committed as 211177