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

Issue 2877473002: Move autofill_regex_constants to core/common. (Closed)

Created:
3 years, 7 months ago by pkalinnikov
Modified:
3 years, 7 months ago
Reviewers:
Mathieu, dvadym
CC:
chromium-reviews, rouslan+autofill_chromium.org, rogerm+autofillwatch_chromium.org, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, mathp+autofillwatch_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move autofill_regex_constants to core/common. Before this CL some files in core/common included the autofill_regex_constants from core/browser, and that violated layering. This CL moves the constants to the right place. This change is also necessary to allow using the constants from renedrer code in order to fix some false positives in password autofilling. Additionally, the files have been reformatted automatically. BUG=674151 Review-Url: https://codereview.chromium.org/2877473002 Cr-Commit-Position: refs/heads/master@{#470875} Committed: https://chromium.googlesource.com/chromium/src/+/bff6eb30da83bf8b1277e2bbbb8d55467da2bbb6

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fix tests build. #

Patch Set 3 : Remove illegal dependency. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+136 lines, -558 lines) Patch
M components/autofill/core/browser/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
M components/autofill/core/browser/address_field.cc View 1 chunk +1 line, -1 line 0 comments Download
D components/autofill/core/browser/autofill_regex_constants.h View 1 chunk +0 lines, -67 lines 0 comments Download
D components/autofill/core/browser/autofill_regex_constants.cc View 1 chunk +0 lines, -342 lines 0 comments Download
M components/autofill/core/browser/credit_card_field.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/autofill/core/browser/email_field.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/autofill/core/browser/name_field.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/autofill/core/browser/phone_field.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/autofill/core/browser/validation.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/autofill/core/common/BUILD.gn View 1 2 2 chunks +2 lines, -1 line 0 comments Download
A + components/autofill/core/common/autofill_regex_constants.h View 0 chunks +-1 lines, --1 lines 0 comments Download
A + components/autofill/core/common/autofill_regex_constants.cc View 6 chunks +128 lines, -140 lines 0 comments Download
M components/autofill/core/common/autofill_regexes_unittest.cc View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 22 (17 generated)
pkalinnikov
PTAL. What bug can I attach this to? https://codereview.chromium.org/2877473002/diff/1/components/autofill/core/common/autofill_regex_constants.cc File components/autofill/core/common/autofill_regex_constants.cc (right): https://codereview.chromium.org/2877473002/diff/1/components/autofill/core/common/autofill_regex_constants.cc#newcode40 components/autofill/core/common/autofill_regex_constants.cc:40: "|^住所$|住所1" ...
3 years, 7 months ago (2017-05-10 13:45:14 UTC) #4
pkalinnikov
https://codereview.chromium.org/2877473002/diff/1/components/autofill/core/common/autofill_regex_constants.cc File components/autofill/core/common/autofill_regex_constants.cc (right): https://codereview.chromium.org/2877473002/diff/1/components/autofill/core/common/autofill_regex_constants.cc#newcode40 components/autofill/core/common/autofill_regex_constants.cc:40: "|^住所$|住所1" // ja-JP On 2017/05/10 13:45:13, pkalinnikov wrote: > ...
3 years, 7 months ago (2017-05-10 13:54:23 UTC) #9
Mathieu
lgtm
3 years, 7 months ago (2017-05-10 18:35:10 UTC) #17
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/2877473002/40001
3 years, 7 months ago (2017-05-11 08:27:54 UTC) #19
commit-bot: I haz the power
3 years, 7 months ago (2017-05-11 08:35:06 UTC) #22
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bff6eb30da83bf8b1277e2bbbb8d...

Powered by Google App Engine
This is Rietveld 408576698