New issue
Advanced search Search tips

Issue 682379 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Remove direct use of base::Time::Now() in Autofill

Project Member Reported by se...@chromium.org, Jan 18 2017

Issue description

In Autofill, everytime the time is needed, a direct call to base::Time::Now() is made. This makes the tests flake sometimes. It would be good to make it possible to inject a TestClock for the tests.

Places to change

autofill_data_model
autofill_profile
credit_card
autofill_manager
personal_data_manager
credit_card_field
full_card_request
card_unmask_prompt_impl
 

Comment 1 by se...@chromium.org, Jan 27 2017

Cc: se...@chromium.org
 Issue 627404  has been merged into this issue.

Comment 2 by se...@chromium.org, Jan 27 2017

 Issue 629507  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c21ca772b51f0b9f12879713dd74a6b09d88316

commit 8c21ca772b51f0b9f12879713dd74a6b09d88316
Author: sebsg <sebsg@chromium.org>
Date: Wed Feb 01 19:20:05 2017

[Autofill] Remove direct use of base::Time::Now() in Autofill

Adds a standard way to get the current time in Autofill. With the AutofillClock, it is now possible to inject a test clock that can return specific times when queried.

I only made effective use of it in the PersonalDataManagerTest for now.

BUG= 682379 

Review-Url: https://codereview.chromium.org/2639403002
Cr-Commit-Position: refs/heads/master@{#447572}

[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/BUILD.gn
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/autofill_data_model.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/autofill_profile.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/credit_card.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/credit_card_field.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/payments/full_card_request.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/personal_data_manager_unittest.cc
[add] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/test_autofill_clock.cc
[add] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/test_autofill_clock.h
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/browser/webdata/autofill_table.cc
[modify] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/common/BUILD.gn
[add] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/common/autofill_clock.cc
[add] https://crrev.com/8c21ca772b51f0b9f12879713dd74a6b09d88316/components/autofill/core/common/autofill_clock.h

Comment 4 by se...@chromium.org, Feb 1 2017

Status: Fixed (was: Started)

Comment 5 by se...@chromium.org, Feb 6 2017

Labels: Merge-Request-57
Project Member

Comment 6 by sheriffbot@chromium.org, Feb 6 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please merge your change to M57 branch 2987 before 5:00 PM PT, Tuesday (02/07/17) so we can pick it up for next Beta release. Thank you.

Comment 8 by se...@chromium.org, Feb 6 2017

Labels: -Hotlist-Merge-Approved -Merge-Approved-57
Removed the merge request

Sign in to add a comment