This is a pre-existing bug that is exacerbated by migrating to the SequecenManager. Currently the only safe time to override Now is when a renderer is created. 

I'm uncertain how serious the pre-existing bug is.  The only safe time to radically change the result of now is when the renderer is created via the --initial-virtual-time flag.  That does result in a  base:: ScopedTimeClockOverrides but that's really early on so it should be OK.

It's possible to enable virtual time after the renderer has started via DevTools but looking at MainThreadSchedulerImpl::EnableVirtualTime() and MainThreadSchedulerImpl::EnableVirtualTime(BaseTimeOverridePolicy policy) I don't think the race can happen, although future refactoring might change that.  Specifically MainThreadSchedulerImpl::EnableVirtualTime() requests BaseTimeOverridePolicy::OVERRIDE if main_thread_only().initial_virtual_time is not null, but that can only happen if either --initial-virtual-time was set or if MainThreadSchedulerImpl::EnableVirtualTime(BaseTimeOverridePolicy policy) was called.  Note MainThreadSchedulerImpl::EnableVirtualTime(BaseTimeOverridePolicy policy) bails out if virtual time was already enabled.

I think this means we're OK and time can only be overriden if --initial-virtual-time is set.

If we need to support overriding now after the renderer is started, we wither need to make the pointer atomic (which has a cost) or freeze other threads while we override time.

 



Calling EnableVirtualTime which overrides Now after the renderer has started is going to lead to racy results from Now, and may theoretically lead to crashes if the function pointer change requires multiple registers (I suspect we're OK here since it's .

Oops the last bit from #1 should have been deleted.

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

commit 9e50e07dbf60ecb29d8fe20ca4a9479c88d389ff
Author: Alex Clarke <alexclarke@chromium.org>
Date: Fri Nov 30 16:43:41 2018

LazilyDeallocatedDeque to use TimeTicksNowIgnoringOverride

This works around one issue with overriding now while there are other
SequenceManager threads around.  Specifically the MessageLoop didn't
sample Now unless there where delayed tasks, and the IO thread where
this problem crops up in tests does not use DelayedTasks.

The SequenceManager however does sample now due to heuristics in the
LazilyDeallocatedDeque.

Overriding time isn't thread safe and this patch does not change
that. Further work may be needded to make that safe,

Bug: 910524, 863341, 891670
Change-Id: I816247b8640f178e00cbd40ac85cd76e5139320a
Reviewed-on: https://chromium-review.googlesource.com/c/1356586
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612674}
[modify] https://crrev.com/9e50e07dbf60ecb29d8fe20ca4a9479c88d389ff/base/task/sequence_manager/lazily_deallocated_deque.h
[modify] https://crrev.com/9e50e07dbf60ecb29d8fe20ca4a9479c88d389ff/base/task/sequence_manager/task_queue_impl.h

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

commit d1c38112ee879acdb2ddea94d2c18074b9ecafa6
Author: Andrey Kosyakov <caseq@chromium.org>
Date: Sun Dec 16 23:11:31 2018

Suppress data race in ScopedTimeClockOverrides

Bug: 910524, 843734 
Change-Id: I23301797735349729135ea792f6ed4805bb5100c
Reviewed-on: https://chromium-review.googlesource.com/c/1378974
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617019}
[modify] https://crrev.com/d1c38112ee879acdb2ddea94d2c18074b9ecafa6/build/sanitizers/tsan_suppressions.cc