Add a FullWallet constructor for data obtained from SDK.

Add a FullWallet constructor for data obtained from SDK.

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

[aruslan, 2013-08-02 19:02:48]

Please take a look, thanks!

[Dan Beam, 2013-08-02 19:08:38]

where is this used?

[aruslan, 2013-08-02 19:12:04]

On 2013/08/02 19:08:38, Dan Beam wrote:
> where is this used?

When I construct the FullWallet from the SDK response:
https://chromiumcodereview.appspot.com/21124012/diff/1/chrome/browser/ui/andr...
(it should read FullWallet::CreateFullWalletFromClearText)

[Dan Beam, 2013-08-06 03:38:16]

is there any particular reason you want to stuff this into a full wallet instead
of new class called SdkResponse or something like that?  it's kind of odd that
you're passing blank strings and short-circuiting the decryption in
FullWallet...

https://codereview.chromium.org/21928004/diff/2003/components/autofill/conten...
File components/autofill/content/browser/wallet/full_wallet.cc (right):

https://codereview.chromium.org/21928004/diff/2003/components/autofill/conten...
components/autofill/content/browser/wallet/full_wallet.cc:151:
std::vector<RequiredAction>()));
nit: this seems to fit in 80 cols like this:

  scoped_ptr<FullWallet> wallet(new FullWallet(
      expiration_month,
      expiration_year,
      std::string()
      std::string(),
      billing_address.Pass(),
      shipping_address.Pass(),
      std::vector<RequiredAction>()));

https://codereview.chromium.org/21928004/diff/2003/components/autofill/conten...
components/autofill/content/browser/wallet/full_wallet.cc:151:
std::vector<RequiredAction>()));
are you sure this can never has required actions?

https://codereview.chromium.org/21928004/diff/2003/components/autofill/conten...
components/autofill/content/browser/wallet/full_wallet.cc:153: wallet->cvn_ =
cvn;
this is kind of nasty...

https://codereview.chromium.org/21928004/diff/2003/components/autofill/conten...
File components/autofill/content/browser/wallet/full_wallet_unittest.cc (right):

https://codereview.chromium.org/21928004/diff/2003/components/autofill/conten...
components/autofill/content/browser/wallet/full_wallet_unittest.cc:517:
full_wallet->GetInfo(CREDIT_CARD_EXP_DATE_2_DIGIT_YEAR));
nit: maybe test addresses?

https://codereview.chromium.org/21928004/diff/2003/components/autofill/conten...
components/autofill/content/browser/wallet/full_wallet_unittest.cc:519: 
^H

[aruslan, 2013-08-07 01:18:15]

On 2013/08/06 03:38:16, Dan Beam wrote:
> is there any particular reason you want to stuff this into a full wallet
instead
> of new class called SdkResponse or something like that?  it's kind of odd that
> you're passing blank strings and short-circuiting the decryption in
> FullWallet...

I need some reliable way of filling the form out of the Wallet and Autofill
responses.
I didn't want to create yet another set of models/wrappers with yet another set
of obscure rules for the form filling.

Do we have a better option?

[Dan Beam, 2013-08-07 03:56:24]

guess not and others are ok with it, so lgtm

[aruslan, 2013-08-07 18:21:16]

Thanks, Dan!

https://chromiumcodereview.appspot.com/21928004/diff/2003/components/autofill...
File components/autofill/content/browser/wallet/full_wallet.cc (right):

https://chromiumcodereview.appspot.com/21928004/diff/2003/components/autofill...
components/autofill/content/browser/wallet/full_wallet.cc:151:
std::vector<RequiredAction>()));
On 2013/08/06 03:38:17, Dan Beam wrote:
> nit: this seems to fit in 80 cols like this:
> 
>   scoped_ptr<FullWallet> wallet(new FullWallet(
>       expiration_month,
>       expiration_year,
>       std::string()
>       std::string(),
>       billing_address.Pass(),
>       shipping_address.Pass(),
>       std::vector<RequiredAction>()));

Done.

https://chromiumcodereview.appspot.com/21928004/diff/2003/components/autofill...
components/autofill/content/browser/wallet/full_wallet.cc:151:
std::vector<RequiredAction>()));
On 2013/08/06 03:38:17, Dan Beam wrote:
> are you sure this can never has required actions?

Yes -- this is a final (non-negotiable) response; all necessary actions should
have already taken place.

https://chromiumcodereview.appspot.com/21928004/diff/2003/components/autofill...
components/autofill/content/browser/wallet/full_wallet.cc:153: wallet->cvn_ =
cvn;
On 2013/08/06 03:38:17, Dan Beam wrote:
> this is kind of nasty...

Another option was to add a new constructor, and it was even more confusing. 
This one at least makes it clear what exactly goes on.

https://chromiumcodereview.appspot.com/21928004/diff/2003/components/autofill...
File components/autofill/content/browser/wallet/full_wallet_unittest.cc (right):

https://chromiumcodereview.appspot.com/21928004/diff/2003/components/autofill...
components/autofill/content/browser/wallet/full_wallet_unittest.cc:517:
full_wallet->GetInfo(CREDIT_CARD_EXP_DATE_2_DIGIT_YEAR));
On 2013/08/06 03:38:17, Dan Beam wrote:
> nit: maybe test addresses?

Done.

https://chromiumcodereview.appspot.com/21928004/diff/2003/components/autofill...
components/autofill/content/browser/wallet/full_wallet_unittest.cc:519: 
On 2013/08/06 03:38:17, Dan Beam wrote:
> ^H

Done.

[commit-bot: I haz the power, 2013-08-07 18:57:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/21928004/19001

[commit-bot: I haz the power, 2013-08-07 19:57:41]

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

[commit-bot: I haz the power, 2013-08-07 20:02:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/21928004/19001

[commit-bot: I haz the power, 2013-08-07 22:25:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/21928004/19001

[commit-bot: I haz the power, 2013-08-08 19:43:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/21928004/42001

[commit-bot: I haz the power, 2013-08-08 20:28:01]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-08-08 20:34:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/21928004/42001

[commit-bot: I haz the power, 2013-08-08 20:34:53]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 500: 
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>500 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered an error and could not complete your request.<p>If
the problem persists, please <A
HREF="http://code.google.com/appengine/community.html">report</A> your problem
and mention this error message and the query that caused it.</h2>
<h2></h2>
</body></html>

[commit-bot: I haz the power, 2013-08-08 20:59:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/21928004/42001

[commit-bot: I haz the power, 2013-08-08 23:35:53]

Change committed as 216486