Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(131)

Issue 11777007: Adds wallet::RequiredAction for when we start interacting with Online Wallet. (Closed)

Created:
7 years, 11 months ago by Dan Beam
Modified:
7 years, 11 months ago
CC:
chromium-reviews, Raman Kakilate, benquan, dhollowa+watch_chromium.org, ahutter, browser-components-watch_chromium.org, dbeam+watch-autofill_chromium.org, Dane Wallinga, dyu1, estade+watch_chromium.org, Albert Bodenhamer, Ilya Sherman
Visibility:
Public.

Description

Adds wallet::RequiredAction for when we start interacting with Online Wallet. Currently supported actions are: - create a wallet account - accept a Terms of Service - sign in for some reason - update an expiration date - [re-]enter a CVC - re-validate the current client-side form data (as it failed on the server) - upgrade minimal address BUG=165195, 163508 R=ahutter@chromium.org,isherman@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175388

Patch Set 1 #

Patch Set 2 : happy linters #

Patch Set 3 : comment fix #

Total comments: 7

Patch Set 4 : DVLOG(1) -> DLOG(ERROR) #

Patch Set 5 : class -> namespace #

Total comments: 12

Patch Set 6 : isherman@ review #

Total comments: 8

Patch Set 7 : ahutter@ review #

Total comments: 4

Patch Set 8 : superfluous includes #

Patch Set 9 : doc #

Patch Set 10 : ahutter@ review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+208 lines, -34 lines) Patch
M chrome/browser/autofill/wallet/full_wallet.h View 1 2 3 4 5 6 5 chunks +8 lines, -6 lines 0 comments Download
M chrome/browser/autofill/wallet/full_wallet.cc View 1 2 3 4 5 6 7 8 9 3 chunks +13 lines, -5 lines 0 comments Download
M chrome/browser/autofill/wallet/full_wallet_unittest.cc View 1 2 3 4 5 6 5 chunks +41 lines, -4 lines 0 comments Download
A chrome/browser/autofill/wallet/required_action.h View 1 2 3 4 5 6 7 8 1 chunk +38 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/required_action.cc View 1 2 3 4 5 1 chunk +49 lines, -0 lines 0 comments Download
M chrome/browser/autofill/wallet/wallet_items.h View 1 2 3 4 4 chunks +9 lines, -10 lines 0 comments Download
M chrome/browser/autofill/wallet/wallet_items.cc View 1 2 3 4 5 6 7 8 9 3 chunks +12 lines, -5 lines 0 comments Download
M chrome/browser/autofill/wallet/wallet_items_unittest.cc View 1 2 3 4 5 6 3 chunks +36 lines, -4 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
Dan Beam
+thestig@ for .gypi +isherman@ for chrome/browser/autofill/wallet +ahutter@ for correctness and FYI
7 years, 11 months ago (2013-01-05 03:04:24 UTC) #1
Ilya Sherman
https://codereview.chromium.org/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.cc#newcode49 chrome/browser/autofill/wallet/full_wallet.cc:49: RequiredAction action(RequiredAction::ParseFromString(action_string)); Add a DCHECK that the parse succeeded? ...
7 years, 11 months ago (2013-01-05 03:15:06 UTC) #2
Dan Beam
https://codereview.chromium.org/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.cc#newcode49 chrome/browser/autofill/wallet/full_wallet.cc:49: RequiredAction action(RequiredAction::ParseFromString(action_string)); On 2013/01/05 03:15:06, Ilya Sherman wrote: > ...
7 years, 11 months ago (2013-01-05 03:17:37 UTC) #3
Ilya Sherman
https://chromiumcodereview.appspot.com/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.cc#newcode49 chrome/browser/autofill/wallet/full_wallet.cc:49: RequiredAction action(RequiredAction::ParseFromString(action_string)); On 2013/01/05 03:17:38, Dan Beam wrote: > ...
7 years, 11 months ago (2013-01-05 03:32:58 UTC) #4
Dan Beam
https://chromiumcodereview.appspot.com/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.h File chrome/browser/autofill/wallet/full_wallet.h (right): https://chromiumcodereview.appspot.com/11777007/diff/5001/chrome/browser/autofill/wallet/full_wallet.h#newcode23 chrome/browser/autofill/wallet/full_wallet.h:23: class RequiredAction; On 2013/01/05 03:32:58, Ilya Sherman wrote: > ...
7 years, 11 months ago (2013-01-05 03:50:15 UTC) #5
Dan Beam
https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/wallet_items.cc File chrome/browser/autofill/wallet/wallet_items.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/wallet_items.cc#newcode356 chrome/browser/autofill/wallet/wallet_items.cc:356: required_actions_ == other.required_actions_; I definitely said "whoopsies" out loud ...
7 years, 11 months ago (2013-01-05 03:53:47 UTC) #6
Ilya Sherman
LGTM, thanks :) https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/required_action.cc File chrome/browser/autofill/wallet/required_action.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/required_action.cc#newcode21 chrome/browser/autofill/wallet/required_action.cc:21: } Optional nit: I would write ...
7 years, 11 months ago (2013-01-05 23:30:48 UTC) #7
Dan Beam
https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/wallet_items.cc File chrome/browser/autofill/wallet/wallet_items.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/wallet_items.cc#newcode356 chrome/browser/autofill/wallet/wallet_items.cc:356: required_actions_ == other.required_actions_; On 2013/01/05 23:30:49, Ilya Sherman wrote: ...
7 years, 11 months ago (2013-01-07 06:21:26 UTC) #8
Dan Beam
https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/required_action.cc File chrome/browser/autofill/wallet/required_action.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/4004/chrome/browser/autofill/wallet/required_action.cc#newcode21 chrome/browser/autofill/wallet/required_action.cc:21: } On 2013/01/05 23:30:49, Ilya Sherman wrote: > Optional ...
7 years, 11 months ago (2013-01-07 15:44:53 UTC) #9
ahutter
https://chromiumcodereview.appspot.com/11777007/diff/17010/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/17010/chrome/browser/autofill/wallet/full_wallet.cc#newcode50 chrome/browser/autofill/wallet/full_wallet.cc:50: if (ActionAppliesToFullWallet(action)) Throwing these out probably isn't the right ...
7 years, 11 months ago (2013-01-07 17:50:11 UTC) #10
Dan Beam
Just returning a NULL scoped_ptr<WalletItems|FullWallet> if we hit an invalid required action now. https://chromiumcodereview.appspot.com/11777007/diff/17010/chrome/browser/autofill/wallet/full_wallet.cc File ...
7 years, 11 months ago (2013-01-07 18:29:08 UTC) #11
ahutter
lgtm. Thanks for picking this up! https://chromiumcodereview.appspot.com/11777007/diff/23001/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/23001/chrome/browser/autofill/wallet/full_wallet.cc#newcode51 chrome/browser/autofill/wallet/full_wallet.cc:51: DLOG(ERROR) << "Response ...
7 years, 11 months ago (2013-01-07 18:36:12 UTC) #12
Dan Beam
https://chromiumcodereview.appspot.com/11777007/diff/23001/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://chromiumcodereview.appspot.com/11777007/diff/23001/chrome/browser/autofill/wallet/full_wallet.cc#newcode51 chrome/browser/autofill/wallet/full_wallet.cc:51: DLOG(ERROR) << "Response from Google wallet with bad required ...
7 years, 11 months ago (2013-01-07 18:52:02 UTC) #13
James Hawkins
.gypi LGTM
7 years, 11 months ago (2013-01-07 18:59:12 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dbeam@chromium.org/11777007/1017
7 years, 11 months ago (2013-01-07 19:04:58 UTC) #15
commit-bot: I haz the power
7 years, 11 months ago (2013-01-07 20:49:33 UTC) #16
Retried try job too often on win_aura for step(s) interactive_ui_tests

Powered by Google App Engine
This is Rietveld 408576698