Tentative fix for performance.timeOrigin

Currently, we use a static unified clock to be able to compute the
'time at which the global monotonic clock is zero'. This means that this
values survives even if a page is reloaded after going to sleep for a
while. This is a problem because the time relies on a delta between the
unix time and the monotonic time, which may change after going to sleep.
Hence this means that values after reload cannot be compared with the
value returned by performance.timeOrigin. This CL fixes this by removing
the class and computing this value separately upon instantiating each
Performance object. In particular this ensures that
performance.timeOrigin and performance.timing.navigationStart don't
become extremely different after going to sleep and then reloading a
page.

Bug: 955532
Change-Id: Iaed7e127cd1224a4d3f053225635f2947e074bb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2182670Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#768857}

third_party/blink/renderer/core/frame/dom_timer_test.cc

@@ -51,9 +51,7 @@ class DOMTimerTest : public RenderingTest {
     auto* mock_clock = test_task_runner->GetMockClock();
     auto* mock_tick_clock = test_task_runner->GetMockTickClock();
     auto now_ticks = test_task_runner->NowTicks();
-    unified_clock_ = std::make_unique<Performance::UnifiedClock>(
-        mock_clock, mock_tick_clock);
-    window_performance->SetClocksForTesting(unified_clock_.get());
+    window_performance->SetClocksForTesting(mock_clock, mock_tick_clock);
     window_performance->ResetTimeOriginForTesting(now_ticks);
     GetDocument().GetSettings()->SetScriptEnabled(true);
     auto* loader = GetDocument().Loader();
@@ -88,9 +86,6 @@ class DOMTimerTest : public RenderingTest {
         script, KURL(), SanitizeScriptErrors::kSanitize);
     platform()->RunUntilIdle();
   }
-
- private:
-  std::unique_ptr<Performance::UnifiedClock> unified_clock_;
 };
 
 const char* const kSetTimeout0ScriptText =

third_party/blink/renderer/core/timing/performance.cc

@@ -89,13 +89,6 @@ const SecurityOrigin* GetSecurityOrigin(ExecutionContext* context) {
   return nullptr;
 }
 
-const Performance::UnifiedClock* DefaultUnifiedClock() {
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(Performance::UnifiedClock, unified_clock,
-                                  (base::DefaultClock::GetInstance(),
-                                   base::DefaultTickClock::GetInstance()));
-  return &unified_clock;
-}
-
 bool IsMeasureOptionsEmpty(const PerformanceMeasureOptions& options) {
   return !options.hasDetail() && !options.hasEnd() && !options.hasStart() &&
          !options.hasDuration();
@@ -120,7 +113,7 @@ Performance::Performance(
       element_timing_buffer_max_size_(kDefaultElementTimingBufferSize),
       user_timing_(nullptr),
       time_origin_(time_origin),
-      unified_clock_(DefaultUnifiedClock()),
+      tick_clock_(base::DefaultTickClock::GetInstance()),
       observer_filter_options_(PerformanceEntry::kInvalid),
       task_runner_(std::move(task_runner)),
       deliver_observations_timer_(task_runner_,
@@ -129,7 +122,11 @@ Performance::Performance(
       resource_timing_buffer_full_timer_(
           task_runner_,
           this,
-          &Performance::FireResourceTimingBufferFull) {}
+          &Performance::FireResourceTimingBufferFull) {
+  unix_at_zero_monotonic_ = ConvertSecondsToDOMHighResTimeStamp(
+      base::DefaultClock::GetInstance()->Now().ToDoubleT() -
+      tick_clock_->NowTicks().since_origin().InSecondsF());
+}
 
 Performance::~Performance() = default;
 
@@ -207,7 +204,7 @@ ScriptPromise Performance::measureMemory(
 
 DOMHighResTimeStamp Performance::timeOrigin() const {
   DCHECK(!time_origin_.is_null());
-  return unified_clock_->GetUnixAtZeroMonotonic() +
+  return unix_at_zero_monotonic_ +
          ConvertTimeTicksToDOMHighResTimeStamp(time_origin_);
 }
 
@@ -1021,7 +1018,7 @@ base::TimeDelta Performance::MonotonicTimeToTimeDelta(
 }
 
 DOMHighResTimeStamp Performance::now() const {
-  return MonotonicTimeToDOMHighResTimeStamp(unified_clock_->NowTicks());
+  return MonotonicTimeToDOMHighResTimeStamp(tick_clock_->NowTicks());
 }
 
 // static
@@ -1067,25 +1064,13 @@ void Performance::Trace(Visitor* visitor) {
   EventTargetWithInlineData::Trace(visitor);
 }
 
-DOMHighResTimeStamp Performance::UnifiedClock::GetUnixAtZeroMonotonic() const {
-  // When a Performance object is first queried, use the current system time
-  // to calculate what the Unix time would be at the time the monotonic
-  // clock time was zero, assuming no manual changes to the system clock.
-  // This can be calculated as current_unix_time - current_monotonic_time.
-  if (UNLIKELY(!unix_at_zero_monotonic_)) {
-    unix_at_zero_monotonic_ = ConvertSecondsToDOMHighResTimeStamp(
-        clock_->Now().ToDoubleT() -
-        tick_clock_->NowTicks().since_origin().InSecondsF());
-  }
-  return unix_at_zero_monotonic_.value();
-}
-
-base::TimeTicks Performance::UnifiedClock::NowTicks() const {
-  return tick_clock_->NowTicks();
-}
-
-void Performance::SetClocksForTesting(const UnifiedClock* clock) {
-  unified_clock_ = clock;
+void Performance::SetClocksForTesting(const base::Clock* clock,
+                                      const base::TickClock* tick_clock) {
+  tick_clock_ = tick_clock;
+  // Recompute |unix_at_zero_monotonic_|.
+  unix_at_zero_monotonic_ = ConvertSecondsToDOMHighResTimeStamp(
+      clock->Now().ToDoubleT() -
+      tick_clock_->NowTicks().since_origin().InSecondsF());
 }
 
 void Performance::ResetTimeOriginForTesting(base::TimeTicks time_origin) {

third_party/blink/renderer/core/timing/performance.h

@@ -306,21 +306,9 @@ class CORE_EXPORT Performance : public EventTargetWithInlineData {
 
   void Trace(Visitor*) override;
 
-  class UnifiedClock {
-   public:
-    UnifiedClock(const base::Clock* clock, const base::TickClock* tick_clock)
-        : clock_(clock), tick_clock_(tick_clock) {}
-    DOMHighResTimeStamp GetUnixAtZeroMonotonic() const;
-    base::TimeTicks NowTicks() const;
-
-   private:
-    const base::Clock* clock_;
-    const base::TickClock* tick_clock_;
-    mutable base::Optional<DOMHighResTimeStamp> unix_at_zero_monotonic_;
-  };
-
   // The caller owns the |clock|.
-  void SetClocksForTesting(const UnifiedClock* clock);
+  void SetClocksForTesting(const base::Clock* clock,
+                           const base::TickClock* tick_clock);
   void ResetTimeOriginForTesting(base::TimeTicks time_origin);
 
  private:
@@ -387,7 +375,8 @@ class CORE_EXPORT Performance : public EventTargetWithInlineData {
   Member<PerformanceEventTiming> first_input_timing_;
 
   base::TimeTicks time_origin_;
-  const UnifiedClock* unified_clock_;
+  DOMHighResTimeStamp unix_at_zero_monotonic_;
+  const base::TickClock* tick_clock_;
 
   PerformanceEntryTypeMask observer_filter_options_;
   HeapLinkedHashSet<Member<PerformanceObserver>> observers_;

third_party/blink/renderer/core/timing/window_performance_test.cc

@@ -84,10 +84,8 @@ class WindowPerformanceTest : public testing::Test {
 
     LocalDOMWindow* window = LocalDOMWindow::From(GetScriptState());
     performance_ = DOMWindowPerformance::performance(*window);
-    unified_clock_ = std::make_unique<Performance::UnifiedClock>(
-        test_task_runner_->GetMockClock(),
-        test_task_runner_->GetMockTickClock());
-    performance_->SetClocksForTesting(unified_clock_.get());
+    performance_->SetClocksForTesting(test_task_runner_->GetMockClock(),
+                                      test_task_runner_->GetMockTickClock());
     performance_->time_origin_ = GetTimeOrigin();
   }
 
@@ -98,7 +96,6 @@ class WindowPerformanceTest : public testing::Test {
   Persistent<WindowPerformance> performance_;
   std::unique_ptr<DummyPageHolder> page_holder_;
   scoped_refptr<base::TestMockTimeTaskRunner> test_task_runner_;
-  std::unique_ptr<Performance::UnifiedClock> unified_clock_;
 };
 
 TEST_F(WindowPerformanceTest, LongTaskObserverInstrumentation) {