[ScopedTaskEnvironment] Non-zero initial NowTicks() under MOCK_TIME

This is a prerequisite for MOCK_TIME on ThreadPool because
base::internal::WorkerThread doesn't support TimeTicks::Now() being
zero.

And it's also a definite source of pain given the number of codesites
found which had to work around this.

Bug: 946657
Change-Id: I115b4af77e5095454b2d505915884215e82d7f9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1677107
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Sergey Volk <servolk@chromium.org>
Reviewed-by: David Staessens <dstaessens@chromium.org>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Nicolas Zea <zea@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#673413}

base/test/scoped_task_environment.cc

@@ -243,11 +243,9 @@ class ScopedTaskEnvironment::MockTimeDomain
   // Protects |now_ticks_|
   mutable Lock now_ticks_lock_;
 
-  // Only ever written to from the main sequence.
-  // TODO(alexclarke): We can't override now by default with now_ticks_ starting
-  // from zero because many tests have checks that TimeTicks::Now() returns a
-  // non-zero result.
-  TimeTicks now_ticks_;
+  // Only ever written to from the main sequence. Start from real Now() instead
+  // of zero to give a more realistic view to tests.
+  TimeTicks now_ticks_{base::subtle::TimeTicksNowIgnoringOverride()};
 };
 
 ScopedTaskEnvironment::MockTimeDomain*

base/test/scoped_task_environment.h

@@ -223,8 +223,10 @@ class ScopedTaskEnvironment {
   // process. See time.h.
   const Clock* GetMockClock() const;
 
-  // Only valid for instances with a MOCK_TIME MainThreadType.
-  // Returns the current virtual tick time (initially starting at 0).
+  // Only valid for instances with a MOCK_TIME MainThreadType. Returns the
+  // current virtual tick time (based on a realistic Now(), sampled when this
+  // ScopedTaskEnvironment was created, and manually advanced from that point
+  // on).
   base::TimeTicks NowTicks() const;
 
   // Only valid for instances with a MOCK_TIME MainThreadType. Returns the

base/test/scoped_task_environment_unittest.cc

@@ -436,10 +436,6 @@ TEST_F(ScopedTaskEnvironmentTest, CrossThreadTaskPostingDoesntAffectMockTime) {
       ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
       ScopedTaskEnvironment::NowSource::MAIN_THREAD_MOCK_TIME);
 
-  // ThreadPool WorkerThreads don't like when TimeTicks::Now() evaluates to 0.
-  // TODO(gab): Make ScopedTaskEnvironment not start at 0.
-  scoped_task_environment.FastForwardBy(TimeDelta::FromMilliseconds(100));
-
   int count = 0;
 
   // Post tasks delayd between 0 and 999 seconds.

base/timer/lap_timer_unittest.cc

@@ -30,9 +30,6 @@ TEST(LapTimer, UsageExample) {
       ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
       ScopedTaskEnvironment::NowSource::MAIN_THREAD_MOCK_TIME);
 
-  // Advance time a little bit so that TimeTicks::Now().is_null() becomes false.
-  scoped_task_environment.FastForwardBy(kTimeAdvance);
-
   LapTimer timer(kWarmupRuns, kTimeLimit, kTimeCheckInterval);
 
   EXPECT_FALSE(timer.HasTimeLimitExpired());

chrome/browser/chromeos/login/saml/saml_password_expiry_notification_unittest.cc

@@ -31,8 +31,6 @@ namespace chromeos {
 
 namespace {
 
-constexpr base::TimeDelta kStartTime = base::TimeDelta::FromMilliseconds(1);
-
 constexpr base::TimeDelta kOneHour = base::TimeDelta::FromHours(1);
 constexpr base::TimeDelta kOneDay = base::TimeDelta::FromDays(1);
 constexpr base::TimeDelta kAdvanceWarningTime = base::TimeDelta::FromDays(14);
@@ -46,9 +44,6 @@ inline base::string16 utf16(const char* ascii) {
 class SamlPasswordExpiryNotificationTest : public testing::Test {
  public:
   void SetUp() override {
-    // Advance time a little bit so that Time::Now().is_null() becomes false.
-    test_environment_.FastForwardBy(kStartTime);
-
     ASSERT_TRUE(profile_manager_.SetUp());
     profile_ = profile_manager_.CreateTestingProfile("test");
     profile_->GetPrefs()->SetBoolean(prefs::kSamlInSessionPasswordChangeEnabled,

chrome/browser/notifications/notification_trigger_scheduler_unittest.cc

@@ -19,8 +19,6 @@ using ::testing::_;
 
 namespace {
 
-constexpr base::TimeDelta kTimeAdvance = base::TimeDelta::FromMilliseconds(1);
-
 std::unique_ptr<TestingProfileManager> CreateTestingProfileManager() {
   std::unique_ptr<TestingProfileManager> profile_manager(
       new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
@@ -45,11 +43,6 @@ class NotificationTriggerSchedulerTest : public testing::Test {
             base::test::ScopedTaskEnvironment::NowSource::
                 MAIN_THREAD_MOCK_TIME) {}
 
-  void SetUp() override {
-    // Advance time a little bit so TimeTicks::Now().is_null() becomes false.
-    thread_bundle_.FastForwardBy(kTimeAdvance);
-  }
-
   class ProfileTestData {
    public:
     ProfileTestData(TestingProfileManager* profile_manager,

chrome/browser/upgrade_detector/upgrade_detector_impl_unittest.cc

@@ -208,10 +208,6 @@ TEST_F(UpgradeDetectorImplTest, VariationsCriticalChanges) {
 // elevated: 16h
 // high: 24h
 TEST_F(UpgradeDetectorImplTest, TestPeriodChanges) {
-  // Fast forward a little bit to get away from zero ticks, which has special
-  // meaning in the detector.
-  FastForwardBy(base::TimeDelta::FromHours(1));
-
   TestUpgradeDetectorImpl upgrade_detector(GetMockClock(), GetMockTickClock());
   ::testing::StrictMock<MockUpgradeObserver> mock_observer(&upgrade_detector);
 
@@ -406,10 +402,6 @@ TEST_P(UpgradeDetectorImplTimerTest, TestNotificationTimer) {
   static constexpr base::TimeDelta kTwentyMinues =
       base::TimeDelta::FromMinutes(20);
 
-  // Fast forward a little bit to get away from zero ticks, which has special
-  // meaning in the detector.
-  FastForwardBy(base::TimeDelta::FromHours(1));
-
   TestUpgradeDetectorImpl detector(GetMockClock(), GetMockTickClock());
   ::testing::StrictMock<MockUpgradeObserver> mock_observer(&detector);
 

chromecast/base/metrics/cast_metrics_helper_unittest.cc

@@ -83,13 +83,15 @@ class CastMetricsHelperTest : public ::testing::Test {
 };
 
 TEST_F(CastMetricsHelperTest, RecordEventWithValue) {
-  const base::TimeDelta time = base::TimeDelta::FromSeconds(5);
-  scoped_task_environment_.FastForwardBy(time);
+  const auto expected_time = scoped_task_environment_.NowTicks();
 
-  EXPECT_CALL(metrics_sink_,
-              OnAction(AllOf(HasString("name", kEvent),
-                             HasDouble("time", time.InMicroseconds()),
-                             HasInt("value", kValue))));
+  EXPECT_CALL(
+      metrics_sink_,
+      OnAction(
+          AllOf(HasString("name", kEvent),
+                HasDouble("time",
+                          (expected_time - base::TimeTicks()).InMicroseconds()),
+                HasInt("value", kValue))));
   metrics_helper_.RecordApplicationEventWithValue(kEvent, kValue);
 }
 
@@ -98,13 +100,14 @@ TEST_F(CastMetricsHelperTest, RecordApplicationEvent) {
   metrics_helper_.DidCompleteLoad(kAppId1, kSessionId1);
   metrics_helper_.UpdateSDKInfo(kSdkVersion);
 
-  const base::TimeDelta time = base::TimeDelta::FromSeconds(5);
-  scoped_task_environment_.FastForwardBy(time);
+  const auto expected_time = scoped_task_environment_.NowTicks();
 
   EXPECT_CALL(
       metrics_sink_,
       OnAction(AllOf(
-          HasString("name", kEvent), HasDouble("time", time.InMicroseconds()),
+          HasString("name", kEvent),
+          HasDouble("time",
+                    (expected_time - base::TimeTicks()).InMicroseconds()),
           HasString("app_id", kAppId1), HasString("session_id", kSessionId1),
           HasString("sdk_version", kSdkVersion))));
   metrics_helper_.RecordApplicationEvent(kEvent);
@@ -115,13 +118,14 @@ TEST_F(CastMetricsHelperTest, RecordApplicationEventWithValue) {
   metrics_helper_.DidCompleteLoad(kAppId1, kSessionId1);
   metrics_helper_.UpdateSDKInfo(kSdkVersion);
 
-  const base::TimeDelta time = base::TimeDelta::FromSeconds(5);
-  scoped_task_environment_.FastForwardBy(time);
+  const auto expected_time = scoped_task_environment_.NowTicks();
 
   EXPECT_CALL(
       metrics_sink_,
       OnAction(AllOf(
-          HasString("name", kEvent), HasDouble("time", time.InMicroseconds()),
+          HasString("name", kEvent),
+          HasDouble("time",
+                    (expected_time - base::TimeTicks()).InMicroseconds()),
           HasString("app_id", kAppId1), HasString("session_id", kSessionId1),
           HasString("sdk_version", kSdkVersion), HasInt("value", kValue))));
   metrics_helper_.RecordApplicationEventWithValue(kEvent, kValue);
@@ -131,25 +135,25 @@ TEST_F(CastMetricsHelperTest, LogTimeToFirstPaint) {
   metrics_helper_.DidStartLoad(kAppId1);
   metrics_helper_.DidCompleteLoad(kAppId1, kSessionId1);
 
-  const base::TimeDelta time_to_first_paint = base::TimeDelta::FromSeconds(5);
-  scoped_task_environment_.FastForwardBy(time_to_first_paint);
+  constexpr base::TimeDelta kTimeToFirstPaint = base::TimeDelta::FromSeconds(5);
+  scoped_task_environment_.FastForwardBy(kTimeToFirstPaint);
 
   EXPECT_CALL(metrics_sink_, OnTimeEvent(AllOf(HasSubstr(kAppId1),
                                                HasSubstr("TimeToFirstPaint")),
-                                         time_to_first_paint, _, _, _));
+                                         kTimeToFirstPaint, _, _, _));
   metrics_helper_.LogTimeToFirstPaint();
 }
 
 TEST_F(CastMetricsHelperTest, LogTimeToFirstAudio) {
   metrics_helper_.DidStartLoad(kAppId1);
   metrics_helper_.DidCompleteLoad(kAppId1, kSessionId1);
 
-  const base::TimeDelta time_to_first_audio = base::TimeDelta::FromSeconds(5);
-  scoped_task_environment_.FastForwardBy(time_to_first_audio);
+  constexpr base::TimeDelta kTimeToFirstAudio = base::TimeDelta::FromSeconds(5);
+  scoped_task_environment_.FastForwardBy(kTimeToFirstAudio);
 
   EXPECT_CALL(metrics_sink_, OnTimeEvent(AllOf(HasSubstr(kAppId1),
                                                HasSubstr("TimeToFirstAudio")),
-                                         time_to_first_audio, _, _, _));
+                                         kTimeToFirstAudio, _, _, _));
   metrics_helper_.LogTimeToFirstAudio();
 }
 

components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc

@@ -183,9 +183,6 @@ class RemoteSuggestionsFetcherImplTest : public testing::Test {
     UserClassifier::RegisterProfilePrefs(utils_.pref_service()->registry());
     user_classifier_ = std::make_unique<UserClassifier>(
         utils_.pref_service(), base::DefaultClock::GetInstance());
-    // Increase initial time such that ticks are non-zero.
-    scoped_task_environment_.FastForwardBy(
-        base::TimeDelta::FromMilliseconds(1234));
     ResetFetcher();
   }
 

content/browser/notifications/platform_notification_context_trigger_unittest.cc

@@ -62,8 +62,6 @@ class PlatformNotificationContextTriggerTest : public ::testing::Test {
   }
 
   void SetUp() override {
-    // Advance time a little bit so TimeTicks::Now().is_null() becomes false.
-    thread_bundle_.FastForwardBy(base::TimeDelta::FromMilliseconds(1));
     scoped_feature_list_.InitAndEnableFeature(features::kNotificationTriggers);
     platform_notification_context_ =
         base::MakeRefCounted<PlatformNotificationContextImpl>(

content/common/tab_switch_time_recorder_unittest.cc

@@ -53,9 +53,6 @@ constexpr base::TimeDelta kOtherDuration =
 class TabSwitchTimeRecorderTest : public testing::Test {
  protected:
   void SetUp() override {
-    // Start with non-zero time.
-    scoped_task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(123));
-
     // Expect all histograms to be empty.
     ExpectHistogramsEmptyExcept({});
   }

media/gpu/chromeos/platform_video_frame_pool_unittest.cc

@@ -20,10 +20,6 @@ class PlatformVideoFramePoolTest
   PlatformVideoFramePoolTest()
       : scoped_task_environment_(
             base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME) {
-    // Seed test clock with some dummy non-zero value to avoid confusion with
-    // empty base::TimeTicks values.
-    test_clock_.Advance(base::TimeDelta::FromSeconds(1234));
-
     pool_.reset(new PlatformVideoFramePool(
         base::BindRepeating(&VideoFrame::CreateZeroInitializedFrame),
         &test_clock_));

net/disk_cache/disk_cache_fuzzer.cc

@@ -89,12 +89,6 @@ struct InitGlobals {
         base::test::ScopedTaskEnvironment::MainThreadType::IO_MOCK_TIME,
         base::test::ScopedTaskEnvironment::NowSource::MAIN_THREAD_MOCK_TIME);
 
-    // Work around for a DCHECK issue: DCHECK(!TimeTicks::Now().is_null())
-    // Since MOCK_TIME starts at 0 the starting time is considered null.
-    // So make it non-zero.
-    scoped_task_environment_->FastForwardBy(
-        base::TimeDelta::FromMilliseconds(1));
-
     // Disable noisy logging as per "libFuzzer in Chrome" documentation:
     // testing/libfuzzer/getting_started.md#Disable-noisy-error-message-logging.
     logging::SetMinLogLevel(logging::LOG_FATAL);

net/http/http_proxy_connect_job_unittest.cc

@@ -61,10 +61,6 @@ class HttpProxyConnectJobTest : public ::testing::TestWithParam<HttpProxyType>,
             base::test::ScopedTaskEnvironment::NowSource::
                 MAIN_THREAD_MOCK_TIME),
         field_trial_list_(nullptr) {
-    // Set an initial delay to ensure that calls to TimeTicks::Now() do not
-    // return a null value.
-    FastForwardBy(base::TimeDelta::FromSeconds(1));
-
     // Used a mock HostResolver that does not have a cache.
     session_deps_.host_resolver = std::make_unique<MockHostResolver>();
 

net/socket/socks_connect_job_unittest.cc

@@ -65,11 +65,7 @@ class SOCKSConnectJobTest : public testing::Test,
             nullptr /* socket_performance_watcher_factory */,
             nullptr /* network_quality_estimator */,
             &net_log_,
-            nullptr /* websocket_endpoint_lock_manager */) {
-    // Set an initial delay to ensure that the first call to TimeTicks::Now()
-    // before incrementing the counter does not return a null value.
-    FastForwardBy(base::TimeDelta::FromSeconds(1));
-  }
+            nullptr /* websocket_endpoint_lock_manager */) {}
 
   ~SOCKSConnectJobTest() override {}
 

net/socket/ssl_connect_job_unittest.cc

@@ -112,10 +112,6 @@ class SSLConnectJobTest : public WithScopedTaskEnvironment,
                                       NetworkIsolationKey())),
         common_connect_job_params_(session_->CreateCommonConnectJobParams()) {
     ssl_config_service_->GetSSLConfig(&ssl_config_);
-
-    // Set an initial delay to ensure that the first call to TimeTicks::Now()
-    // before incrementing the counter does not return a null value.
-    FastForwardBy(base::TimeDelta::FromSeconds(1));
   }
 
   ~SSLConnectJobTest() override = default;

net/socket/transport_client_socket_pool_unittest.cc

@@ -2176,11 +2176,7 @@ class TransportClientSocketPoolMockNowSourceTest
       : TransportClientSocketPoolTest(
             base::test::ScopedTaskEnvironment::MainThreadType::IO_MOCK_TIME,
             base::test::ScopedTaskEnvironment::NowSource::
-                MAIN_THREAD_MOCK_TIME) {
-    // Forward the clock by a non-zero amount to avoid triggering DCHECKs that
-    // verify that certain timestamps are non-null.
-    FastForwardBy(base::TimeDelta::FromSeconds(1));
-  }
+                MAIN_THREAD_MOCK_TIME) {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TransportClientSocketPoolMockNowSourceTest);

net/socket/transport_connect_job_unittest.cc

@@ -54,11 +54,7 @@ class TransportConnectJobTest : public WithScopedTaskEnvironment,
             nullptr /* socket_performance_watcher_factory */,
             nullptr /* network_quality_estimator */,
             &net_log_,
-            nullptr /* websocket_endpoint_lock_manager */) {
-    // Set an initial delay to ensure that calls to TimeTicks::Now() do not
-    // return a null value.
-    FastForwardBy(base::TimeDelta::FromSeconds(1));
-  }
+            nullptr /* websocket_endpoint_lock_manager */) {}
 
   ~TransportConnectJobTest() override {}
 

third_party/blink/renderer/platform/scheduler/common/scheduler_helper_unittest.cc

@@ -54,8 +54,6 @@ class SchedulerHelperTest : public testing::Test {
             base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
             base::test::ScopedTaskEnvironment::ThreadPoolExecutionMode::
                 QUEUED) {
-    // Null clock triggers some assertions.
-    task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
     sequence_manager_ = base::sequence_manager::SequenceManagerForTest::Create(
         nullptr, task_environment_.GetMainThreadTaskRunner(),
         task_environment_.GetMockTickClock());

third_party/blink/renderer/platform/scheduler/main_thread/deadline_task_runner_unittest.cc

@@ -22,8 +22,6 @@ class DeadlineTaskRunnerTest : public testing::Test {
             base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
             base::test::ScopedTaskEnvironment::ThreadPoolExecutionMode::
                 QUEUED) {
-    // Null clock might trigger some assertions.
-    task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
   }
   ~DeadlineTaskRunnerTest() override = default;
 

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl_unittest.cc

@@ -62,10 +62,7 @@ class FrameSchedulerImplTest : public testing::Test {
       : task_environment_(
             base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
             base::test::ScopedTaskEnvironment::ThreadPoolExecutionMode::
-                QUEUED) {
-    // Null clock might trigger some assertions.
-    task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
-  }
+                QUEUED) {}
 
   FrameSchedulerImplTest(std::vector<base::Feature> features_to_enable,
                          std::vector<base::Feature> features_to_disable)

third_party/blink/renderer/platform/scheduler/main_thread/idle_time_estimator_unittest.cc

@@ -40,8 +40,6 @@ class IdleTimeEstimatorTest : public testing::Test {
             base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
             base::test::ScopedTaskEnvironment::ThreadPoolExecutionMode::QUEUED),
         frame_length_(base::TimeDelta::FromMilliseconds(16)) {
-    // Null clock might trigger some assertions.
-    task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
   }
 
   ~IdleTimeEstimatorTest() override = default;