Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 11293078: Integrating Online Wallet into Chrome. (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Description

Integrating Online Wallet into Chrome. This CL is modeled after the Gaia OAuth client. BUG=163609 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=174078

Patch Set 1 #

Patch Set 2 : Unit tests and more functionality #

Patch Set 3 : Adding more functionality and unit testse #

Patch Set 4 : More functionality for full wallet #

Patch Set 5 : More unit tests and functionality #

Total comments: 8

Patch Set 6 : More tests. Real encryption/decryption. #

Total comments: 3

Patch Set 7 : Changes from Dane's review #

Total comments: 46

Patch Set 8 : Fixes from Raman's code review #

Total comments: 20

Patch Set 9 : Rebase and most fixes from Albert's initial review. #

Total comments: 180

Patch Set 10 : Fixes from code review. #

Patch Set 11 : More fixes from code review #

Total comments: 57

Patch Set 12 : Fixes from latest code review #

Total comments: 7

Patch Set 13 : Fixed linter issue #

Total comments: 99

Patch Set 14 : Fixes from code review #

Total comments: 4

Patch Set 15 : Removing Core from WalletClient #

Patch Set 16 : Adding comments for flags #

Patch Set 17 : Fixing some errors in comments #

Patch Set 18 : Adding comment to string_number_conversions_unittest.cc #

Patch Set 19 : Fixing [chromium-style] error #

Total comments: 3

Patch Set 20 : Fixing HexStringToInt #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3404 lines, -14 lines) Patch
M base/string_number_conversions.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +7 lines, -0 lines 0 comments Download
M base/string_number_conversions.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 4 chunks +17 lines, -7 lines 0 comments Download
M base/string_number_conversions_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +64 lines, -7 lines 0 comments Download
A chrome/browser/autofill/wallet/cart.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +45 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/cart.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +24 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/cart_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +20 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/full_wallet.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +94 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/full_wallet.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +208 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/full_wallet_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +420 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_address.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +110 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_address.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +172 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_address_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +231 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +152 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +285 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_client_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +396 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_items.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +207 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_items.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +360 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_items_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +447 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_service_url.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +23 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_service_url.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +62 lines, -0 lines 0 comments Download
A chrome/browser/autofill/wallet/wallet_service_url_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +32 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +12 lines, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +8 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 51 (0 generated)
ahutter
https://codereview.chromium.org/11293078/diff/14002/chrome/browser/autofill/wallet/wallet_data_retriever.cc File chrome/browser/autofill/wallet/wallet_data_retriever.cc (right): https://codereview.chromium.org/11293078/diff/14002/chrome/browser/autofill/wallet/wallet_data_retriever.cc#newcode174 chrome/browser/autofill/wallet/wallet_data_retriever.cc:174: // TODO(ahutter): Probably want to do some checks on ...
4 years, 11 months ago (2012-11-15 20:59:53 UTC) #1
ahutter
4 years, 11 months ago (2012-11-15 21:44:58 UTC) #2
Dane Wallinga
https://codereview.chromium.org/11293078/diff/17001/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11293078/diff/17001/chrome/browser/autofill/wallet/full_wallet.cc#newcode41 chrome/browser/autofill/wallet/full_wallet.cc:41: scoped_ptr<FullWallet> FullWallet::FromDictionary(DictionaryValue* dictionary) { return raw pointer, change name ...
4 years, 11 months ago (2012-11-16 01:27:25 UTC) #3
ahutter
https://codereview.chromium.org/11293078/diff/17001/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11293078/diff/17001/chrome/browser/autofill/wallet/full_wallet.cc#newcode41 chrome/browser/autofill/wallet/full_wallet.cc:41: scoped_ptr<FullWallet> FullWallet::FromDictionary(DictionaryValue* dictionary) { On 2012/11/16 01:27:25, dgwallinga wrote: ...
4 years, 11 months ago (2012-11-16 19:46:41 UTC) #4
Raman Kakilate
will have to look some more files. https://codereview.chromium.org/11293078/diff/14002/chrome/browser/autofill/wallet/wallet_data_retriever.cc File chrome/browser/autofill/wallet/wallet_data_retriever.cc (right): https://codereview.chromium.org/11293078/diff/14002/chrome/browser/autofill/wallet/wallet_data_retriever.cc#newcode55 chrome/browser/autofill/wallet/wallet_data_retriever.cc:55: std::string GetRiskParams() ...
4 years, 11 months ago (2012-11-17 02:28:31 UTC) #5
Raman Kakilate
https://codereview.chromium.org/11293078/diff/22004/chrome/browser/autofill/wallet/wallet_address.h File chrome/browser/autofill/wallet/wallet_address.h (right): https://codereview.chromium.org/11293078/diff/22004/chrome/browser/autofill/wallet/wallet_address.h#newcode71 chrome/browser/autofill/wallet/wallet_address.h:71: void set_object_id(const std::string& object_id) { On 2012/11/17 02:28:31, Raman ...
4 years, 11 months ago (2012-11-17 04:10:51 UTC) #6
ahutter
https://codereview.chromium.org/11293078/diff/22004/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11293078/diff/22004/chrome/browser/autofill/wallet/full_wallet.cc#newcode25 chrome/browser/autofill/wallet/full_wallet.cc:25: const std::string& rest, On 2012/11/17 02:28:31, Raman Kakilate wrote: ...
4 years, 10 months ago (2012-11-27 00:46:12 UTC) #7
Albert Bodenhamer
A few initial comments. https://codereview.chromium.org/11293078/diff/28005/base/string_number_conversions.h File base/string_number_conversions.h (right): https://codereview.chromium.org/11293078/diff/28005/base/string_number_conversions.h#newcode98 base/string_number_conversions.h:98: BASE_EXPORT bool HexStringToInt64(const StringPiece& input, ...
4 years, 10 months ago (2012-11-28 23:50:56 UTC) #8
ahutter
https://codereview.chromium.org/11293078/diff/28005/base/string_number_conversions.h File base/string_number_conversions.h (right): https://codereview.chromium.org/11293078/diff/28005/base/string_number_conversions.h#newcode109 base/string_number_conversions.h:109: #endif // BASE_STRING_NUMBER_CONVERSIONS_H_ On 2012/11/28 23:50:56, Albert Bodenhamer wrote: ...
4 years, 10 months ago (2012-11-29 20:52:38 UTC) #9
Albert Bodenhamer
A few more. Sorry for delays. It seems to be the day of interruptions. Also, ...
4 years, 10 months ago (2012-11-30 01:06:01 UTC) #10
Albert Bodenhamer
This is big enough that I'd like to get a second set of eyes on ...
4 years, 10 months ago (2012-11-30 18:14:07 UTC) #11
Dan Beam (no longer on Chrome)
my brain was full at wallet_data_retriever.h, I'll come back later https://codereview.chromium.org/11293078/diff/32002/chrome/browser/autofill/wallet/cart_unittest.cc File chrome/browser/autofill/wallet/cart_unittest.cc (right): https://codereview.chromium.org/11293078/diff/32002/chrome/browser/autofill/wallet/cart_unittest.cc#newcode9 ...
4 years, 10 months ago (2012-11-30 19:55:52 UTC) #12
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/11293078/diff/32002/chrome/browser/autofill/wallet/wallet_data_retriever.cc File chrome/browser/autofill/wallet/wallet_data_retriever.cc (right): https://codereview.chromium.org/11293078/diff/32002/chrome/browser/autofill/wallet/wallet_data_retriever.cc#newcode26 chrome/browser/autofill/wallet/wallet_data_retriever.cc:26: namespace { nit: \n https://codereview.chromium.org/11293078/diff/32002/chrome/browser/autofill/wallet/wallet_data_retriever.cc#newcode41 chrome/browser/autofill/wallet/wallet_data_retriever.cc:41: request_type_(NO_PENDING_REQUEST) {} nit: ...
4 years, 10 months ago (2012-12-01 01:19:49 UTC) #13
ahutter
Thanks so much for looking at this Dan. I made all the changes you suggested. ...
4 years, 10 months ago (2012-12-01 04:06:50 UTC) #14
ahutter
More fixes from code reviews. I think this is ready for a final review.
4 years, 10 months ago (2012-12-05 18:24:27 UTC) #15
Albert Bodenhamer
Mac builder is complaining about uninitialized variable use. See the build log at: http://build.chromium.org/p/tryserver.chromium/builders/mac_rel/builds/79557/steps/compile/logs/stdio A ...
4 years, 10 months ago (2012-12-05 18:56:45 UTC) #16
Raman Kakilate
lgtm https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/wallet_items_unittest.cc File chrome/browser/autofill/wallet/wallet_items_unittest.cc (right): https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/wallet_items_unittest.cc#newcode287 chrome/browser/autofill/wallet/wallet_items_unittest.cc:287: class WalletItemsTest : public testing::Test { Optional: May ...
4 years, 10 months ago (2012-12-05 19:02:22 UTC) #17
Albert Bodenhamer
https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/wallet_items.h File chrome/browser/autofill/wallet/wallet_items.h (right): https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/wallet_items.h#newcode26 chrome/browser/autofill/wallet/wallet_items.h:26: // actions may be required before a purchase can ...
4 years, 10 months ago (2012-12-05 19:22:29 UTC) #18
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/11293078/diff/32002/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11293078/diff/32002/chrome/browser/autofill/wallet/full_wallet.cc#newcode40 chrome/browser/autofill/wallet/full_wallet.cc:40: FullWallet* FullWallet::CreateFullWallet(DictionaryValue* dictionary) { On 2012/12/01 04:06:51, ahutter wrote: ...
4 years, 10 months ago (2012-12-05 19:34:59 UTC) #19
Dan Beam (no longer on Chrome)
btw, you'll need a base/OWNERS review, candidates are: mark@chromium.org darin@chromium.org brettw@chromium.org willchan@chromium.org jar@chromium.org
4 years, 10 months ago (2012-12-05 19:36:11 UTC) #20
ahutter
Adding darin@ for base/ and chrome/ OWNERS approval. https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/cart.h File chrome/browser/autofill/wallet/cart.h (right): https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/cart.h#newcode20 chrome/browser/autofill/wallet/cart.h:20: // ...
4 years, 10 months ago (2012-12-05 23:37:23 UTC) #21
Albert Bodenhamer
lgtm Please wait for Dan and Darin before committing. https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/wallet_address_unittest.cc File chrome/browser/autofill/wallet/wallet_address_unittest.cc (right): https://codereview.chromium.org/11293078/diff/47002/chrome/browser/autofill/wallet/wallet_address_unittest.cc#newcode13 chrome/browser/autofill/wallet/wallet_address_unittest.cc:13: ...
4 years, 10 months ago (2012-12-06 00:36:43 UTC) #22
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_address.cc File chrome/browser/autofill/wallet/wallet_address.cc (right): https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_address.cc#newcode156 chrome/browser/autofill/wallet/wallet_address.cc:156: recipient_name_ == other.recipient_name_ && On 2012/12/06 00:36:43, Albert Bodenhamer ...
4 years, 10 months ago (2012-12-06 00:52:07 UTC) #23
ahutter
https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/full_wallet.cc#newcode35 chrome/browser/autofill/wallet/full_wallet.cc:35: DCHECK(required_actions_.size() > 0 || billing_address_); On 2012/12/06 00:36:43, Albert ...
4 years, 10 months ago (2012-12-06 19:23:47 UTC) #24
Dan Beam (no longer on Chrome)
lgtm https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_items.cc File chrome/browser/autofill/wallet/wallet_items.cc (right): https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_items.cc#newcode51 chrome/browser/autofill/wallet/wallet_items.cc:51: return NULL; nit: I'd personally group all critical ...
4 years, 10 months ago (2012-12-06 21:59:24 UTC) #25
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_items.cc File chrome/browser/autofill/wallet/wallet_items.cc (right): https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_items.cc#newcode51 chrome/browser/autofill/wallet/wallet_items.cc:51: return NULL; On 2012/12/06 21:59:24, Dan Beam wrote: > ...
4 years, 10 months ago (2012-12-06 22:00:14 UTC) #26
ahutter
Rebased and fixed linter issues and latest code review comments. https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_items.cc File chrome/browser/autofill/wallet/wallet_items.cc (right): https://codereview.chromium.org/11293078/diff/37010/chrome/browser/autofill/wallet/wallet_items.cc#newcode51 ...
4 years, 10 months ago (2012-12-06 22:44:19 UTC) #27
ahutter
Adding Brett for base/ and chrome/ OWNER approval.
4 years, 10 months ago (2012-12-06 22:46:27 UTC) #28
ahutter
On 2012/12/06 22:46:27, ahutter wrote: > Adding Brett for base/ and chrome/ OWNER approval. In ...
4 years, 10 months ago (2012-12-06 22:48:41 UTC) #29
Evan Stade
did you clear the CC field intentionally? https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/cart.h File chrome/browser/autofill/wallet/cart.h (right): https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/cart.h#newcode21 chrome/browser/autofill/wallet/cart.h:21: // spending ...
4 years, 10 months ago (2012-12-07 19:57:44 UTC) #30
Ilya Sherman
Some high-level comments: (1) When uploading a Chromium CL, please don't clear out the automatic ...
4 years, 10 months ago (2012-12-14 04:56:43 UTC) #31
Dan Beam (no longer on Chrome)
btw, when you upload an empty CL then add to it via repeated `git cl ...
4 years, 10 months ago (2012-12-14 04:59:57 UTC) #32
ahutter
https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/cart.h File chrome/browser/autofill/wallet/cart.h (right): https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/cart.h#newcode21 chrome/browser/autofill/wallet/cart.h:21: // spending limits on the generated proxy card. If ...
4 years, 10 months ago (2012-12-15 01:06:31 UTC) #33
Ilya Sherman
https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/full_wallet.h File chrome/browser/autofill/wallet/full_wallet.h (right): https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/full_wallet.h#newcode52 chrome/browser/autofill/wallet/full_wallet.h:52: const Address* shipping_address() const { return shipping_address_.get(); } On ...
4 years, 10 months ago (2012-12-15 01:23:59 UTC) #34
Ilya Sherman
https://codereview.chromium.org/11293078/diff/70001/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/11293078/diff/70001/chrome/common/chrome_switches.cc#newcode1367 chrome/common/chrome_switches.cc:1367: const char kWalletServiceUrl[] = "wallet-service-url"; Why do we have ...
4 years, 10 months ago (2012-12-15 01:40:12 UTC) #35
ahutter
On 2012/12/15 01:23:59, Ilya Sherman wrote: > https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/full_wallet.h > File chrome/browser/autofill/wallet/full_wallet.h (right): > > https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/full_wallet.h#newcode52 ...
4 years, 10 months ago (2012-12-15 02:13:07 UTC) #36
ahutter
https://codereview.chromium.org/11293078/diff/70001/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/11293078/diff/70001/chrome/common/chrome_switches.cc#newcode1367 chrome/common/chrome_switches.cc:1367: const char kWalletServiceUrl[] = "wallet-service-url"; On 2012/12/15 01:40:12, Ilya ...
4 years, 10 months ago (2012-12-15 02:22:22 UTC) #37
Evan Stade
lgtm https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/full_wallet.h File chrome/browser/autofill/wallet/full_wallet.h (right): https://codereview.chromium.org/11293078/diff/57004/chrome/browser/autofill/wallet/full_wallet.h#newcode66 chrome/browser/autofill/wallet/full_wallet.h:66: std::string cvn_; On 2012/12/15 01:06:31, ahutter wrote: > ...
4 years, 10 months ago (2012-12-15 02:22:37 UTC) #38
benquan
https://codereview.chromium.org/11293078/diff/22004/base/string_number_conversions.cc File base/string_number_conversions.cc (right): https://codereview.chromium.org/11293078/diff/22004/base/string_number_conversions.cc#newcode306 base/string_number_conversions.cc:306: class BaseHexIteratorRangeToInt64Traits Tests? https://codereview.chromium.org/11293078/diff/22004/chrome/browser/autofill/wallet/full_wallet.cc File chrome/browser/autofill/wallet/full_wallet.cc (right): https://codereview.chromium.org/11293078/diff/22004/chrome/browser/autofill/wallet/full_wallet.cc#newcode16 chrome/browser/autofill/wallet/full_wallet.cc:16: ...
4 years, 10 months ago (2012-12-15 02:48:59 UTC) #39
ahutter
Adding Sky for chrome/chrome_browser.gypi changes. https://codereview.chromium.org/11293078/diff/22004/base/string_number_conversions.cc File base/string_number_conversions.cc (right): https://codereview.chromium.org/11293078/diff/22004/base/string_number_conversions.cc#newcode306 base/string_number_conversions.cc:306: class BaseHexIteratorRangeToInt64Traits On 2012/12/15 ...
4 years, 10 months ago (2012-12-17 17:23:02 UTC) #40
sky
*gypi LGTM
4 years, 10 months ago (2012-12-17 17:42:08 UTC) #41
ahutter
Ping. Need OWNERs approval for changes in base/.
4 years, 10 months ago (2012-12-17 17:54:20 UTC) #42
ahutter
Adding willchan@ for OWNERS approval of changes in base/.
4 years, 10 months ago (2012-12-17 18:10:33 UTC) #43
willchan no longer on Chromium
I'll be back later to take a closer look, but please make sure there are ...
4 years, 10 months ago (2012-12-17 20:04:20 UTC) #44
ahutter
I fixed the [chromium-style] issue and the CL is now building locally using clang.
4 years, 10 months ago (2012-12-18 02:26:20 UTC) #45
willchan no longer on Chromium
lgtm once my nits are addressed https://codereview.chromium.org/11293078/diff/87001/base/string_number_conversions_unittest.cc File base/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/11293078/diff/87001/base/string_number_conversions_unittest.cc#newcode234 base/string_number_conversions_unittest.cc:234: {"100000000", -1, false}, ...
4 years, 10 months ago (2012-12-18 19:08:41 UTC) #46
ahutter
On 2012/12/18 19:08:41, willchan wrote: > lgtm once my nits are addressed > > It ...
4 years, 10 months ago (2012-12-19 18:43:27 UTC) #47
brettw
Fixing hex string sounds like it would take an hour or so. If you don't ...
4 years, 10 months ago (2012-12-19 19:15:16 UTC) #48
ahutter
Fixed Will's nits.
4 years, 10 months ago (2012-12-19 23:34:26 UTC) #49
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ahutter@chromium.org/11293078/92001
4 years, 10 months ago (2012-12-19 23:36:50 UTC) #50
commit-bot: I haz the power
4 years, 10 months ago (2012-12-20 02:46:30 UTC) #51
Message was sent while issue was closed.
Change committed as 174078
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa