Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2864873002:
Adds feature flag to disable AutofillProfileComparator for card name. (Closed)
|
Created:
3 years, 7 months ago by csashi Modified:
3 years, 7 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, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdds feature flag to disable AutofillProfileComparator for card name.
This will allow us to revert back to previous simpler name comparisons
in case Chrome's comparator
(incorporated in https://codereview.chromium.org/2864483005/)
is more aggressive than payments server.
BUG=710974
Review-Url: https://codereview.chromium.org/2864873002
Cr-Commit-Position: refs/heads/master@{#470182}
Committed: https://chromium.googlesource.com/chromium/src/+/9f96db6af2442820fed423f5b1b745bc049ae0ad
Patch Set 1 #
Total comments: 2
Patch Set 2 : Removes TODO(crbug.com/666704) for OS_ANDROID. #Patch Set 3 : Removes TODO(crbug.com/666704) for OS_ANDROID. #Patch Set 4 : Restores TODO(crbug.com/666704) for OS_ANDROID for UploadCreditCard_NoCvcFieldOnForm_UserEntersCvc #
Messages
Total messages: 36 (26 generated)
csashi@google.com changed reviewers: + jsaul@google.com, rogerm@chromium.org, sebsg@chromium.org
Hi, Please take a look. Thanks! -sashi.
Description was changed from ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator is more aggressive than payments server. BUG=710974 ========== to ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator (incorporated in https://codereview.chromium.org/2864483005/) is more aggressive than payments server. BUG=710974 ==========
Description was changed from ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator (incorporated in https://codereview.chromium.org/2864483005/) is more aggressive than payments server. BUG=710974 ========== to ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator (incorporated in https://codereview.chromium.org/2864483005/) is more aggressive than payments server. BUG=710974 ==========
Description was changed from ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator (incorporated in https://codereview.chromium.org/2864483005/) is more aggressive than payments server. BUG=710974 ========== to ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator (incorporated in https://codereview.chromium.org/2864483005/) is more aggressive than payments server. BUG=710974 ==========
The CQ bit was checked by csashi@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
lgtm
https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... File components/autofill/core/browser/autofill_manager_unittest.cc (right): https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... components/autofill/core/browser/autofill_manager_unittest.cc:5628: MAYBE_UploadCreditCard_CCFormHasMiddleInitial_DisableComparator) { Do we need to start out with the added tests disabled on android? Have you seen evidence of the added tests' flakiness or are you just following the pattern?
lgtm
https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... File components/autofill/core/browser/autofill_manager_unittest.cc (right): https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... components/autofill/core/browser/autofill_manager_unittest.cc:5628: MAYBE_UploadCreditCard_CCFormHasMiddleInitial_DisableComparator) { On 2017/05/08 15:38:16, Roger McFarlane (Chromium) wrote: > Do we need to start out with the added tests disabled on android? > > Have you seen evidence of the added tests' flakiness or are you just following > the pattern? Just following the pattern. I can try removing it.
The CQ bit was checked by csashi@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by csashi@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by csashi@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/08 16:17:14, csashi wrote: > https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... > File components/autofill/core/browser/autofill_manager_unittest.cc (right): > > https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... > components/autofill/core/browser/autofill_manager_unittest.cc:5628: > MAYBE_UploadCreditCard_CCFormHasMiddleInitial_DisableComparator) { > On 2017/05/08 15:38:16, Roger McFarlane (Chromium) wrote: > > Do we need to start out with the added tests disabled on android? > > > > Have you seen evidence of the added tests' flakiness or are you just following > > the pattern? > > Just following the pattern. I can try removing it. Removing it did not work. Restoring it. See: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.andro...
The CQ bit was checked by csashi@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/05/08 19:47:43, csashi wrote: > On 2017/05/08 16:17:14, csashi wrote: > > > https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... > > File components/autofill/core/browser/autofill_manager_unittest.cc (right): > > > > > https://codereview.chromium.org/2864873002/diff/1/components/autofill/core/br... > > components/autofill/core/browser/autofill_manager_unittest.cc:5628: > > MAYBE_UploadCreditCard_CCFormHasMiddleInitial_DisableComparator) { > > On 2017/05/08 15:38:16, Roger McFarlane (Chromium) wrote: > > > Do we need to start out with the added tests disabled on android? > > > > > > Have you seen evidence of the added tests' flakiness or are you just > following > > > the pattern? > > > > Just following the pattern. I can try removing it. > > Removing it did not work. Restoring it. See: > https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.andro... I disabled UploadCreditCard_NoCvcFieldOnForm_UserEntersCvc for Android. The others passed on Android but I may have got lucky. We may need to disable more. I will submit this and disable other tests if needed in a follow up.
The CQ bit was checked by csashi@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org, jsaul@google.com Link to the patchset: https://codereview.chromium.org/2864873002/#ps60001 (title: "Restores TODO(crbug.com/666704) for OS_ANDROID for UploadCreditCard_NoCvcFieldOnForm_UserEntersCvc")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494281764021480,
"parent_rev": "475dadd52b797253bb2563c09112a31f7ce1078f", "commit_rev":
"9f96db6af2442820fed423f5b1b745bc049ae0ad"}
Message was sent while issue was closed.
Description was changed from ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator (incorporated in https://codereview.chromium.org/2864483005/) is more aggressive than payments server. BUG=710974 ========== to ========== Adds feature flag to disable AutofillProfileComparator for card name. This will allow us to revert back to previous simpler name comparisons in case Chrome's comparator (incorporated in https://codereview.chromium.org/2864483005/) is more aggressive than payments server. BUG=710974 Review-Url: https://codereview.chromium.org/2864873002 Cr-Commit-Position: refs/heads/master@{#470182} Committed: https://chromium.googlesource.com/chromium/src/+/9f96db6af2442820fed423f5b1b7... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/9f96db6af2442820fed423f5b1b7... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
