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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 6673079: Reduce boxing and unboxing of AutofillFieldType (Closed)

Created:
9 years, 9 months ago by Ilya Sherman
Modified:
9 years, 7 months ago
Reviewers:
dhollowa
CC:
chromium-reviews, ncarter (slow), Raghu Simha, idana, James Hawkins, kkania, pam+watch_chromium.org, Paweł Hajdan Jr., Ilya Sherman, tim (not reviewing), dhollowa
Visibility:
Public.

Description

Reduce boxing and unboxing of AutofillFieldType BUG=none TEST=compiles Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=78518

Patch Set 1 #

Patch Set 2 : Compile on linux. #

Patch Set 3 : Compile on Windows. #

Total comments: 20

Patch Set 4 : Denitting #

Patch Set 5 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+559 lines, -671 lines) Patch
M chrome/browser/autofill/address.h View 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/autofill/address.cc View 1 chunk +12 lines, -13 lines 0 comments Download
M chrome/browser/autofill/address_unittest.cc View 1 2 3 2 chunks +13 lines, -14 lines 0 comments Download
M chrome/browser/autofill/autofill_common_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/autofill/autofill_field.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/autofill/autofill_field.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/autofill/autofill_ie_toolbar_import_win.cc View 1 2 3 chunks +10 lines, -11 lines 0 comments Download
M chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc View 1 2 3 1 chunk +22 lines, -38 lines 0 comments Download
chrome/browser/autofill/autofill_manager.h View 1 2 3 4 3 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/autofill/autofill_manager.cc View 1 2 3 4 10 chunks +31 lines, -29 lines 0 comments Download
M chrome/browser/autofill/autofill_manager_unittest.cc View 2 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/autofill/autofill_merge_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/autofill/autofill_profile.h View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/autofill/autofill_profile.cc View 9 chunks +31 lines, -33 lines 0 comments Download
M chrome/browser/autofill/autofill_profile_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/autofill/contact_info.h View 4 chunks +10 lines, -10 lines 0 comments Download
M chrome/browser/autofill/contact_info.cc View 3 chunks +33 lines, -43 lines 0 comments Download
M chrome/browser/autofill/credit_card.h View 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/autofill/credit_card.cc View 8 chunks +19 lines, -22 lines 0 comments Download
M chrome/browser/autofill/form_group.h View 2 chunks +4 lines, -5 lines 0 comments Download
M chrome/browser/autofill/form_group.cc View 5 chunks +5 lines, -9 lines 0 comments Download
M chrome/browser/autofill/personal_data_manager.cc View 1 2 3 4 6 chunks +21 lines, -26 lines 0 comments Download
M chrome/browser/autofill/personal_data_manager_mac.mm View 1 2 3 4 chunks +29 lines, -43 lines 0 comments Download
M chrome/browser/autofill/personal_data_manager_unittest.cc View 4 chunks +9 lines, -9 lines 0 comments Download
M chrome/browser/autofill/phone_number.h View 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/autofill/phone_number.cc View 2 chunks +11 lines, -12 lines 0 comments Download
M chrome/browser/autofill/select_control_handler.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/autofill/select_control_handler.cc View 1 2 chunks +4 lines, -7 lines 0 comments Download
M chrome/browser/autofill/select_control_handler_unittest.cc View 8 chunks +16 lines, -32 lines 0 comments Download
chrome/browser/automation/testing_automation_provider.cc View 1 2 3 4 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/sync/glue/autofill_model_associator.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/sync/glue/autofill_profile_change_processor.cc View 1 2 3 1 chunk +13 lines, -18 lines 0 comments Download
M chrome/browser/sync/glue/autofill_profile_model_associator.cc View 5 chunks +10 lines, -10 lines 0 comments Download
M chrome/browser/sync/glue/autofill_profile_model_associator_unittest.cc View 1 2 3 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/options/autofill_options_handler.cc View 1 2 3 4 chunks +32 lines, -48 lines 0 comments Download
M chrome/browser/webdata/web_data_service_unittest.cc View 4 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/webdata/web_database.cc View 1 2 3 19 chunks +63 lines, -77 lines 0 comments Download
M chrome/browser/webdata/web_database_unittest.cc View 17 chunks +111 lines, -111 lines 0 comments Download
M chrome/test/live_sync/live_autofill_sync_test.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/live_sync/two_client_live_autofill_sync_test.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Ilya Sherman
9 years, 9 months ago (2011-03-16 09:19:27 UTC) #1
dhollowa
http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/address_unittest.cc File chrome/browser/autofill/address_unittest.cc (right): http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/address_unittest.cc#newcode50 chrome/browser/autofill/address_unittest.cc:50: ASCIIToUTF16("United States")); nit: indent. http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc File chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc (right): http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc#newcode167 ...
9 years, 9 months ago (2011-03-16 17:07:11 UTC) #2
Ilya Sherman
9 years, 9 months ago (2011-03-17 03:42:29 UTC) #3 
You can tell I just used find/replace to make this change the first time around
-- thanks for keeping me honest =)

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/addr...
File chrome/browser/autofill/address_unittest.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/addr...
chrome/browser/autofill/address_unittest.cc:50: ASCIIToUTF16("United States"));
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: indent.

Done.

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc:167:
profile1[0].value);
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: A number of these should fit on single line now.

Done.

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/pers...
File chrome/browser/autofill/personal_data_manager.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/pers...
chrome/browser/autofill/personal_data_manager.cc:210: if
(AutofillType(field_type).subgroup() ==
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: indent, or single line maybe?

Doesn't fit on a single line :(

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/pers...
File chrome/browser/autofill/personal_data_manager_mac.mm (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/autofill/pers...
chrome/browser/autofill/personal_data_manager_mac.mm:114:
profile->SetInfo(NAME_FIRST,
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: A number of these should fit on single line now.

Done.

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/sync/glue/aut...
File chrome/browser/sync/glue/autofill_profile_change_processor.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/sync/glue/aut...
chrome/browser/sync/glue/autofill_profile_change_processor.cc:329:
specifics.set_name_first(UTF16ToUTF8(
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: A number of these should fit on single line now.

Done.

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/sync/glue/aut...
File chrome/browser/sync/glue/autofill_profile_model_associator_unittest.cc
(right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/sync/glue/aut...
chrome/browser/sync/glue/autofill_profile_model_associator_unittest.cc:257:
EXPECT_EQ(
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: should be able to pull up to the "(" now.

Done.

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/ui/views/auto...
File chrome/browser/ui/views/autofill_profiles_view_win.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/ui/views/auto...
chrome/browser/ui/views/autofill_profiles_view_win.cc:1195: (field ==
TEXT_PHONE_PHONE ? PHONE_HOME_COUNTRY_CODE :
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: Should be able to pull some of these ternaries to a same line now.

Just one of them

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/autofill_options_handler.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/autofill_options_handler.cc:269:
address.SetString("fullName",
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: A number of these should fit on single line now.

Done.

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/webdata/web_d...
File chrome/browser/webdata/web_database.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/browser/webdata/web_d...
chrome/browser/webdata/web_database.cc:2903: profile.SetInfo(ADDRESS_HOME_LINE1,
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: A number of these should fit on single line now.

Done.

http://codereview.chromium.org/6673079/diff/3042/chrome/test/live_sync/two_cl...
File chrome/test/live_sync/two_client_live_autofill_sync_test.cc (right):

http://codereview.chromium.org/6673079/diff/3042/chrome/test/live_sync/two_cl...
chrome/test/live_sync/two_client_live_autofill_sync_test.cc:161:
profile1.SetInfo(PHONE_FAX_WHOLE_NUMBER,
On 2011/03/16 17:07:11, dhollowa wrote:
> nit: Single line?

Done.

Powered by Google App Engine
This is Rietveld 408576698