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

Issue 2711543002: Experiment to add bank name in autofill ui. (Closed)

Created:
3 years, 10 months ago by Shanfeng
Modified:
3 years, 6 months ago
CC:
chromium-reviews, rouslan+autofill_chromium.org, rogerm+autofillwatch_chromium.org, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, mathp+autofillwatch_chromium.org, asvitkine+watch_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org, srahim+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Experiment to add bank name in autofill ui. BUG=734265 Review-Url: https://codereview.chromium.org/2711543002 Cr-Commit-Position: refs/heads/master@{#481059} Committed: https://chromium.googlesource.com/chromium/src/+/b0045cb0dec61387b7cc05049c2878b406bc298b

Patch Set 1 #

Patch Set 2 : Use only one variation #

Patch Set 3 : Add tests, fix bugs, and fix historgrams #

Patch Set 4 : add more unit test #

Total comments: 1

Patch Set 5 : Leave alone TypeAndLastFourDigests and use a new function for bank name #

Patch Set 6 : Don't show bank name if bank name is empty even though feature flag on #

Total comments: 4

Patch Set 7 : Address comments #

Total comments: 1

Patch Set 8 : Address comments #

Patch Set 9 : Add new column in chrome database #

Patch Set 10 : Fix autofill table #

Patch Set 11 : Add space #

Patch Set 12 : Fix space diff when do schema change #

Patch Set 13 : Add comments about the flag #

Total comments: 14

Patch Set 14 : For git sync #

Patch Set 15 : For git sync and file format #

Total comments: 8

Patch Set 16 : Address comments and sync #

Total comments: 2

Patch Set 17 : Address comments #

Patch Set 18 : sync to head #

Patch Set 19 : Fix sync issues #

Total comments: 14

Patch Set 20 : Address comments #

Patch Set 21 : Add metrics #

Patch Set 22 : Fix unit test #

Patch Set 23 : Fix and add more metrics unit tests #

Total comments: 19

Patch Set 24 : address comments #

Total comments: 8

Patch Set 25 : Address comments #

Total comments: 15

Patch Set 26 : Address comments #

Total comments: 2

Patch Set 27 : address comment #

Total comments: 10

Patch Set 28 : Adress comment #

Patch Set 29 : Sync to head #

Total comments: 6

Patch Set 30 : Update histograms #

Patch Set 31 : Fix build file #

Patch Set 32 : Address comments #

Patch Set 33 : Experiment to add bank name in autofill ui. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+495 lines, -12 lines) Patch
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/flag_descriptions.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/flag_descriptions.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +6 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_experiments.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 2 chunks +4 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_experiments.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 2 chunks +6 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +8 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_metrics.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 3 chunks +14 lines, -1 line 0 comments Download
M components/autofill/core/browser/autofill_metrics.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 5 chunks +20 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_metrics_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 9 chunks +143 lines, -0 lines 0 comments Download
M components/autofill/core/browser/credit_card.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +8 lines, -0 lines 0 comments Download
M components/autofill/core/browser/credit_card.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 4 chunks +14 lines, -0 lines 0 comments Download
M components/autofill/core/browser/credit_card_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 2 chunks +32 lines, -0 lines 0 comments Download
M components/autofill/core/browser/personal_data_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 21 22 23 1 chunk +6 lines, -1 line 0 comments Download
M components/autofill/core/browser/personal_data_manager_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 3 chunks +71 lines, -2 lines 0 comments Download
M components/autofill/core/browser/webdata/autofill_table.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +2 lines, -0 lines 0 comments Download
M components/autofill/core/browser/webdata/autofill_table.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 8 chunks +28 lines, -5 lines 0 comments Download
M components/autofill/core/browser/webdata/autofill_table_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 1 chunk +22 lines, -0 lines 0 comments Download
M components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
A components/test/data/web_database/version_72.sql View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +32 lines, -0 lines 0 comments Download
M components/webdata/common/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 1 chunk +1 line, -0 lines 0 comments Download
M components/webdata/common/web_database.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +1 line, -1 line 0 comments Download
M components/webdata/common/web_database_migration_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 2 chunks +40 lines, -1 line 0 comments Download
M tools/metrics/histograms/enums.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 3 chunks +9 lines, -1 line 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 132 (66 generated)
Shanfeng
This is a draft to show bank name in autofill ui. We may need to ...
3 years, 10 months ago (2017-02-22 00:44:36 UTC) #2
Jared Saul
https://codereview.chromium.org/2711543002/diff/60001/components/autofill/core/browser/credit_card_unittest.cc File components/autofill/core/browser/credit_card_unittest.cc (right): https://codereview.chromium.org/2711543002/diff/60001/components/autofill/core/browser/credit_card_unittest.cc#newcode156 components/autofill/core/browser/credit_card_unittest.cc:156: summary6); What's the difference between the summary/Label instance and ...
3 years, 10 months ago (2017-02-22 02:49:45 UTC) #3
Shanfeng
On 2017/02/22 02:49:45, Jared Saul wrote: > https://codereview.chromium.org/2711543002/diff/60001/components/autofill/core/browser/credit_card_unittest.cc > File components/autofill/core/browser/credit_card_unittest.cc (right): > > https://codereview.chromium.org/2711543002/diff/60001/components/autofill/core/browser/credit_card_unittest.cc#newcode156 ...
3 years, 10 months ago (2017-02-23 01:22:27 UTC) #4
Jared Saul
https://codereview.chromium.org/2711543002/diff/100001/components/autofill/core/browser/credit_card.h File components/autofill/core/browser/credit_card.h (right): https://codereview.chromium.org/2711543002/diff/100001/components/autofill/core/browser/credit_card.h#newcode112 components/autofill/core/browser/credit_card.h:112: // A lable for this credit card formatted as ...
3 years, 10 months ago (2017-02-23 19:23:33 UTC) #5
Shanfeng
Thanks for the review. https://codereview.chromium.org/2711543002/diff/100001/components/autofill/core/browser/credit_card.h File components/autofill/core/browser/credit_card.h (right): https://codereview.chromium.org/2711543002/diff/100001/components/autofill/core/browser/credit_card.h#newcode112 components/autofill/core/browser/credit_card.h:112: // A lable for this ...
3 years, 9 months ago (2017-02-28 19:22:21 UTC) #6
Jared Saul
https://codereview.chromium.org/2711543002/diff/120001/components/autofill/core/browser/credit_card_unittest.cc File components/autofill/core/browser/credit_card_unittest.cc (right): https://codereview.chromium.org/2711543002/diff/120001/components/autofill/core/browser/credit_card_unittest.cc#newcode169 components/autofill/core/browser/credit_card_unittest.cc:169: EXPECT_EQ(UTF8ToUTF16(kUTF8MidlineEllipsis + "5100"), obfuscated2); This is a good case ...
3 years, 9 months ago (2017-02-28 19:42:23 UTC) #7
Shanfeng
On 2017/02/28 19:42:23, Jared Saul wrote: > https://codereview.chromium.org/2711543002/diff/120001/components/autofill/core/browser/credit_card_unittest.cc > File components/autofill/core/browser/credit_card_unittest.cc (right): > > https://codereview.chromium.org/2711543002/diff/120001/components/autofill/core/browser/credit_card_unittest.cc#newcode169 ...
3 years, 9 months ago (2017-02-28 22:18:40 UTC) #8
Shanfeng
On 2017/02/28 22:18:40, Shanfeng wrote: > On 2017/02/28 19:42:23, Jared Saul wrote: > > > ...
3 years, 9 months ago (2017-03-03 22:27:05 UTC) #23
Jared Saul
https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/credit_card.cc File components/autofill/core/browser/credit_card.cc (right): https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/credit_card.cc#newcode522 components/autofill/core/browser/credit_card.cc:522: size_t bank_name_length = 8; Any particular reason behind the ...
3 years, 9 months ago (2017-03-03 23:40:09 UTC) #24
Shanfeng
Thanks a lot! https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/credit_card.cc File components/autofill/core/browser/credit_card.cc (right): https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/credit_card.cc#newcode522 components/autofill/core/browser/credit_card.cc:522: size_t bank_name_length = 8; On 2017/03/03 ...
3 years, 9 months ago (2017-03-04 00:01:08 UTC) #25
Jared Saul
https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/webdata/autofill_table.cc File components/autofill/core/browser/webdata/autofill_table.cc (right): https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/webdata/autofill_table.cc#newcode1947 components/autofill/core/browser/webdata/autofill_table.cc:1947: " bank_name VARCHAR)")) { On 2017/03/04 00:01:08, Shanfeng wrote: ...
3 years, 9 months ago (2017-03-10 19:51:55 UTC) #26
Shanfeng
Thanks for your review! https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/webdata/autofill_table.cc File components/autofill/core/browser/webdata/autofill_table.cc (right): https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/webdata/autofill_table.cc#newcode1947 components/autofill/core/browser/webdata/autofill_table.cc:1947: " bank_name VARCHAR)")) { On ...
3 years, 9 months ago (2017-03-10 21:49:57 UTC) #27
Mathieu
Drive by on a Friday https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/credit_card.cc File components/autofill/core/browser/credit_card.cc (right): https://codereview.chromium.org/2711543002/diff/240001/components/autofill/core/browser/credit_card.cc#newcode517 components/autofill/core/browser/credit_card.cc:517: // mapping from long ...
3 years, 8 months ago (2017-03-31 18:25:10 UTC) #28
Shanfeng
Thanks for reviewing this. Since we are sure to run this bank name experiment, please ...
3 years, 8 months ago (2017-04-12 01:58:59 UTC) #29
sebsg
Nice! Couple comments https://codereview.chromium.org/2711543002/diff/280001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/2711543002/diff/280001/chrome/browser/about_flags.cc#newcode2481 chrome/browser/about_flags.cc:2481: nit: remove new-line https://codereview.chromium.org/2711543002/diff/280001/components/autofill/core/browser/credit_card.cc File components/autofill/core/browser/credit_card.cc ...
3 years, 8 months ago (2017-04-12 22:18:15 UTC) #30
sebsg
https://codereview.chromium.org/2711543002/diff/300001/components/autofill/core/browser/credit_card_unittest.cc File components/autofill/core/browser/credit_card_unittest.cc (right): https://codereview.chromium.org/2711543002/diff/300001/components/autofill/core/browser/credit_card_unittest.cc#newcode149 components/autofill/core/browser/credit_card_unittest.cc:149: TEST(CreditCardTest, BankNameAndLastFourDigitsStrings) { Could you please add a test ...
3 years, 8 months ago (2017-04-12 22:32:36 UTC) #31
Shanfeng
Thanks for the review. https://codereview.chromium.org/2711543002/diff/280001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/2711543002/diff/280001/chrome/browser/about_flags.cc#newcode2481 chrome/browser/about_flags.cc:2481: On 2017/04/12 22:18:15, sebsg wrote: ...
3 years, 8 months ago (2017-04-13 18:29:09 UTC) #32
Shanfeng
ping~ On 2017/04/13 18:29:09, Shanfeng wrote: > Thanks for the review. > > https://codereview.chromium.org/2711543002/diff/280001/chrome/browser/about_flags.cc > ...
3 years, 7 months ago (2017-05-08 18:29:29 UTC) #33
Shanfeng
Just synced to head. Please help take another look. Thanks!
3 years, 6 months ago (2017-06-06 00:18:17 UTC) #40
Jared Saul
I'm still pretty concerned about how we're going to measure this. There will be a ...
3 years, 6 months ago (2017-06-07 01:22:59 UTC) #41
jiahuiguo
On 2017/06/07 01:22:59, Jared Saul wrote: > I'm still pretty concerned about how we're going ...
3 years, 6 months ago (2017-06-07 04:24:47 UTC) #42
Jared Saul
>> I'm still pretty concerned about how we're going to measure this. There will >> ...
3 years, 6 months ago (2017-06-07 16:34:18 UTC) #43
Shanfeng
On 2017/06/07 16:34:18, Jared Saul wrote: > >> I'm still pretty concerned about how we're ...
3 years, 6 months ago (2017-06-07 17:19:54 UTC) #44
Shanfeng
Thanks https://codereview.chromium.org/2711543002/diff/420001/components/autofill/core/browser/credit_card_unittest.cc File components/autofill/core/browser/credit_card_unittest.cc (right): https://codereview.chromium.org/2711543002/diff/420001/components/autofill/core/browser/credit_card_unittest.cc#newcode177 components/autofill/core/browser/credit_card_unittest.cc:177: EXPECT_EQ(UTF8ToUTF16(std::string(kUTF8MidlineEllipsis) + "5100"), obfuscated2); On 2017/06/07 01:22:59, Jared ...
3 years, 6 months ago (2017-06-07 17:20:17 UTC) #45
sebsg
As mentioned in the meeting, I think we will need the help of someone from ...
3 years, 6 months ago (2017-06-07 19:47:10 UTC) #46
Shanfeng
On 2017/06/07 19:47:10, sebsg wrote: > As mentioned in the meeting, I think we will ...
3 years, 6 months ago (2017-06-12 18:07:20 UTC) #47
sebsg
Almost there! :) You can probably add pkasting@ for the web database stuff https://codereview.chromium.org/2711543002/diff/500001/chrome/browser/flag_descriptions.cc File ...
3 years, 6 months ago (2017-06-12 21:17:01 UTC) #48
Shanfeng
Thanks https://codereview.chromium.org/2711543002/diff/500001/chrome/browser/flag_descriptions.cc File chrome/browser/flag_descriptions.cc (right): https://codereview.chromium.org/2711543002/diff/500001/chrome/browser/flag_descriptions.cc#newcode2510 chrome/browser/flag_descriptions.cc:2510: "If enabled, display the issuer bank name of ...
3 years, 6 months ago (2017-06-12 23:34:58 UTC) #50
Peter Kasting
It looks like you added me as a reviewer. Please say exactly what you want ...
3 years, 6 months ago (2017-06-12 23:38:07 UTC) #51
Shanfeng
On 2017/06/12 23:38:07, Peter Kasting wrote: > It looks like you added me as a ...
3 years, 6 months ago (2017-06-12 23:47:09 UTC) #52
Peter Kasting
On 2017/06/12 23:47:09, Shanfeng wrote: > On 2017/06/12 23:38:07, Peter Kasting wrote: > > It ...
3 years, 6 months ago (2017-06-12 23:49:02 UTC) #53
Peter Kasting
I just went ahead and reviewed all the webdata and web_database files. LGTM https://codereview.chromium.org/2711543002/diff/520001/components/autofill/core/browser/webdata/autofill_table_unittest.cc File ...
3 years, 6 months ago (2017-06-13 00:04:03 UTC) #54
sebsg
https://codereview.chromium.org/2711543002/diff/500001/components/autofill/core/browser/autofill_metrics.h File components/autofill/core/browser/autofill_metrics.h (right): https://codereview.chromium.org/2711543002/diff/500001/components/autofill/core/browser/autofill_metrics.h#newcode546 components/autofill/core/browser/autofill_metrics.h:546: FORM_EVENT_SUGGESTIONS_SHOWN_WITH_BANK_NAME_AVAILABLE_ONCE, On 2017/06/12 23:34:58, Shanfeng wrote: > On 2017/06/12 ...
3 years, 6 months ago (2017-06-13 13:36:04 UTC) #55
Shanfeng
Thanks. Please help take another look. https://codereview.chromium.org/2711543002/diff/520001/components/autofill/core/browser/personal_data_manager_unittest.cc File components/autofill/core/browser/personal_data_manager_unittest.cc (right): https://codereview.chromium.org/2711543002/diff/520001/components/autofill/core/browser/personal_data_manager_unittest.cc#newcode3960 components/autofill/core/browser/personal_data_manager_unittest.cc:3960: // All cards ...
3 years, 6 months ago (2017-06-13 20:39:58 UTC) #56
sebsg
LGTM Good job :)
3 years, 6 months ago (2017-06-13 22:34:54 UTC) #57
jiahuiguo
lgtm, can you add more details in the description or probably a crbug to track ...
3 years, 6 months ago (2017-06-14 05:51:28 UTC) #58
Jared Saul
lgtm https://codereview.chromium.org/2711543002/diff/540001/components/autofill/core/browser/autofill_metrics.cc File components/autofill/core/browser/autofill_metrics.cc (right): https://codereview.chromium.org/2711543002/diff/540001/components/autofill/core/browser/autofill_metrics.cc#newcode1032 components/autofill/core/browser/autofill_metrics.cc:1032: FORM_EVENT_SUGGESTIONS_SHOWN_WITH_BANK_NAME_AVAILABLE_ONCE); indent -4 https://codereview.chromium.org/2711543002/diff/540001/components/autofill/core/browser/autofill_metrics.h File components/autofill/core/browser/autofill_metrics.h (right): https://codereview.chromium.org/2711543002/diff/540001/components/autofill/core/browser/autofill_metrics.h#newcode552 ...
3 years, 6 months ago (2017-06-14 16:49:16 UTC) #59
Shanfeng
Thanks https://codereview.chromium.org/2711543002/diff/420001/components/autofill/core/browser/autofill_experiments.h File components/autofill/core/browser/autofill_experiments.h (right): https://codereview.chromium.org/2711543002/diff/420001/components/autofill/core/browser/autofill_experiments.h#newcode39 components/autofill/core/browser/autofill_experiments.h:39: extern const char kAutofillCreditCardBankNameDisplayInTitle[]; On 2017/06/14 05:51:28, jiahuiguo ...
3 years, 6 months ago (2017-06-14 17:48:25 UTC) #60
Jared Saul
https://codereview.chromium.org/2711543002/diff/540001/components/autofill/core/browser/autofill_metrics.cc File components/autofill/core/browser/autofill_metrics.cc (right): https://codereview.chromium.org/2711543002/diff/540001/components/autofill/core/browser/autofill_metrics.cc#newcode1032 components/autofill/core/browser/autofill_metrics.cc:1032: FORM_EVENT_SUGGESTIONS_SHOWN_WITH_BANK_NAME_AVAILABLE_ONCE); On 2017/06/14 17:48:25, Shanfeng wrote: > On 2017/06/14 ...
3 years, 6 months ago (2017-06-14 17:54:10 UTC) #61
Shanfeng
Thanks https://codereview.chromium.org/2711543002/diff/560001/components/autofill/core/browser/autofill_metrics_unittest.cc File components/autofill/core/browser/autofill_metrics_unittest.cc (right): https://codereview.chromium.org/2711543002/diff/560001/components/autofill/core/browser/autofill_metrics_unittest.cc#newcode191 components/autofill/core/browser/autofill_metrics_unittest.cc:191: // name On 2017/06/14 17:54:10, Jared Saul wrote: ...
3 years, 6 months ago (2017-06-14 21:29:34 UTC) #62
Mathieu
Just a few comments! https://codereview.chromium.org/2711543002/diff/580001/components/autofill/core/browser/autofill_metrics.h File components/autofill/core/browser/autofill_metrics.h (right): https://codereview.chromium.org/2711543002/diff/580001/components/autofill/core/browser/autofill_metrics.h#newcode549 components/autofill/core/browser/autofill_metrics.h:549: enum BankNameExperimentFormEvent { I wouldn't ...
3 years, 6 months ago (2017-06-16 18:27:05 UTC) #63
Shanfeng
Thanks https://codereview.chromium.org/2711543002/diff/580001/components/autofill/core/browser/autofill_metrics.h File components/autofill/core/browser/autofill_metrics.h (right): https://codereview.chromium.org/2711543002/diff/580001/components/autofill/core/browser/autofill_metrics.h#newcode549 components/autofill/core/browser/autofill_metrics.h:549: enum BankNameExperimentFormEvent { On 2017/06/16 18:27:05, Mathieu wrote: ...
3 years, 6 months ago (2017-06-16 20:51:47 UTC) #64
Mathieu
lgtm
3 years, 6 months ago (2017-06-17 00:57:05 UTC) #69
Shanfeng
+rkaplow for histogram approval. Thanks
3 years, 6 months ago (2017-06-19 17:01:44 UTC) #76
rkaplow1
https://codereview.chromium.org/2711543002/diff/620001/tools/metrics/histograms/enums.xml File tools/metrics/histograms/enums.xml (right): https://codereview.chromium.org/2711543002/diff/620001/tools/metrics/histograms/enums.xml#newcode2590 tools/metrics/histograms/enums.xml:2590: + <int value="0" label="Suggestions shown with bank name available ...
3 years, 6 months ago (2017-06-19 18:00:01 UTC) #81
Shanfeng
Thanks! https://codereview.chromium.org/2711543002/diff/620001/tools/metrics/histograms/enums.xml File tools/metrics/histograms/enums.xml (right): https://codereview.chromium.org/2711543002/diff/620001/tools/metrics/histograms/enums.xml#newcode2590 tools/metrics/histograms/enums.xml:2590: + <int value="0" label="Suggestions shown with bank name ...
3 years, 6 months ago (2017-06-19 20:03:23 UTC) #84
rkaplow
On 2017/06/19 20:03:23, Shanfeng wrote: > Thanks! > > https://codereview.chromium.org/2711543002/diff/620001/tools/metrics/histograms/enums.xml > File tools/metrics/histograms/enums.xml (right): > ...
3 years, 6 months ago (2017-06-20 14:13:34 UTC) #89
rkaplow
lgtm
3 years, 6 months ago (2017-06-20 14:13:49 UTC) #90
Shanfeng
On 2017/06/20 14:13:34, rkaplow wrote: > On 2017/06/19 20:03:23, Shanfeng wrote: > > Thanks! > ...
3 years, 6 months ago (2017-06-20 16:46:49 UTC) #93
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2711543002/680001
3 years, 6 months ago (2017-06-20 18:56:22 UTC) #100
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/468855)
3 years, 6 months ago (2017-06-20 19:06:41 UTC) #102
Shanfeng
On 2017/06/20 19:06:41, commit-bot: I haz the power wrote: > Try jobs failed on following ...
3 years, 6 months ago (2017-06-20 20:27:56 UTC) #103
Peter Kasting
LGTM; you're probably free to just commit without rereview in a case of a small ...
3 years, 6 months ago (2017-06-20 20:59:46 UTC) #104
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2711543002/680001
3 years, 6 months ago (2017-06-20 21:01:36 UTC) #106
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/469023)
3 years, 6 months ago (2017-06-20 21:10:06 UTC) #108
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2711543002/680001
3 years, 6 months ago (2017-06-20 21:11:41 UTC) #110
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/469032)
3 years, 6 months ago (2017-06-20 21:21:03 UTC) #112
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2711543002/680001
3 years, 6 months ago (2017-06-20 22:07:52 UTC) #114
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/469134)
3 years, 6 months ago (2017-06-20 22:16:36 UTC) #116
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2711543002/680001
3 years, 6 months ago (2017-06-21 01:09:13 UTC) #122
commit-bot: I haz the power
Committed patchset #32 (id:680001) as https://chromium.googlesource.com/chromium/src/+/b0045cb0dec61387b7cc05049c2878b406bc298b
3 years, 6 months ago (2017-06-21 01:16:05 UTC) #126
lunalu
FYI, you are not supposed to include an type attribute in <enum> in enums.xml This ...
3 years, 6 months ago (2017-06-21 17:19:05 UTC) #128
Shanfeng
On 2017/06/21 17:19:05, lunalu wrote: > FYI, you are not supposed to include an type ...
3 years, 6 months ago (2017-06-21 17:27:15 UTC) #129
chromium-reviews
I am fixing it. No worries. Just for the future reference. On Jun 21, 2017 ...
3 years, 6 months ago (2017-06-21 17:33:51 UTC) #130
loonybear
This is the second time I saw this bug. I wonder something is wrong in ...
3 years, 6 months ago (2017-06-21 17:34:48 UTC) #131
Shanfeng
3 years, 6 months ago (2017-06-21 17:45:23 UTC) #132
Message was sent while issue was closed.
On 2017/06/21 17:34:48, loonybear wrote:
> This is the second time I saw this bug. I wonder something is wrong in the
> automated script.
> 
> If you could look into that, that'd be great.
> 
> Thanks,
> Luna
> 
> On Jun 21, 2017 10:27, <mailto:szhangcs@google.com> wrote:
> 
> > On 2017/06/21 17:19:05, lunalu wrote:
> > > FYI, you are not supposed to include an type attribute in <enum> in
> > enums.xml
> > >
> > > This breaks use counter enum presubmit checker
> >
> > I see. I believe this is introduce by syncing. I'll make a fix soon. I'm
> > wondering why I can still submit the code.
> >
> > https://codereview.chromium.org/2711543002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Thanks. I've made https://codereview.chromium.org/2947163002/. Feel free to
ignore if you already got a fix.
As for the automated script. I actually don't know which one are you referring
to. Maybe you could ping the owner. That would be more efficient.

Powered by Google App Engine
This is Rietveld 408576698