[FMP] Clean up renderer timestamp from FirstMeaningfulPaintDetector

The current implementation has switched to use until-GPU-swap timestamp, but the
code base still keep the renderer timestamp. As the renderer timestamp is no
longer used. We should remove it.

The CL cleans up the renderer timestamp from paint-timing and
first-meaningful-paint-detector.

But this CL has leftover work, as we still have first_meaningful_paint2_quiet_
and first_meaningful_paint2_quiet_swap. Intuitively we would remove
first_meaningful_paint2_quiet_ and keep first_meaningful_paint2_quiet_swap.
But first_meaningful_paint2_quiet_ is still playing a role in the algorithm,
which therefore makes the clean up a bit tricker. We will focus on this issue
in a later patch.

Bug: 747484
Change-Id: I8bdb5f4551fe823ad56fd26d735e37e626006c61
Reviewed-on: https://chromium-review.googlesource.com/c/1355258Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Liquan (Max) Gǔ <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613374}

third_party/blink/renderer/core/paint/first_meaningful_paint_detector.cc

@@ -152,8 +152,7 @@ void FirstMeaningfulPaintDetector::OnNetwork2Quiet() {
     if (defer_first_meaningful_paint_ == kDoNotDefer) {
       // Report FirstMeaningfulPaint when the page reached network 2-quiet if
       // we aren't waiting for a swap timestamp.
-      SetFirstMeaningfulPaint(first_meaningful_paint2_quiet_,
-                              first_meaningful_paint2_quiet_swap);
+      SetFirstMeaningfulPaint(first_meaningful_paint2_quiet_swap);
     }
   }
   ReportHistograms();
@@ -248,8 +247,7 @@ void FirstMeaningfulPaintDetector::ReportSwapTime(
   if (defer_first_meaningful_paint_ == kDeferOutstandingSwapPromises &&
       outstanding_swap_promise_count_ == 0) {
     DCHECK(!first_meaningful_paint2_quiet_.is_null());
-    SetFirstMeaningfulPaint(first_meaningful_paint2_quiet_,
-                            provisional_first_meaningful_paint_swap_);
+    SetFirstMeaningfulPaint(provisional_first_meaningful_paint_swap_);
   }
 }
 
@@ -257,11 +255,10 @@ void FirstMeaningfulPaintDetector::NotifyFirstContentfulPaint(
     TimeTicks swap_stamp) {
   if (defer_first_meaningful_paint_ != kDeferFirstContentfulPaintNotSet)
     return;
-  SetFirstMeaningfulPaint(first_meaningful_paint2_quiet_, swap_stamp);
+  SetFirstMeaningfulPaint(swap_stamp);
 }
 
 void FirstMeaningfulPaintDetector::SetFirstMeaningfulPaint(
-    TimeTicks stamp,
     TimeTicks swap_stamp) {
   DCHECK(paint_timing_->FirstMeaningfulPaint().is_null());
   DCHECK(!swap_stamp.is_null());
@@ -276,8 +273,7 @@ void FirstMeaningfulPaintDetector::SetFirstMeaningfulPaint(
   paint_timing_->SetFirstMeaningfulPaintCandidate(swap_stamp);
 
   paint_timing_->SetFirstMeaningfulPaint(
-      stamp, swap_stamp,
-      had_user_input_before_provisional_first_meaningful_paint_);
+      swap_stamp, had_user_input_before_provisional_first_meaningful_paint_);
 }
 
 void FirstMeaningfulPaintDetector::Trace(blink::Visitor* visitor) {

third_party/blink/renderer/core/paint/first_meaningful_paint_detector.h

@@ -61,7 +61,7 @@ class CORE_EXPORT FirstMeaningfulPaintDetector
   int ActiveConnections();
   void ReportHistograms();
   void RegisterNotifySwapTime(PaintEvent);
-  void SetFirstMeaningfulPaint(TimeTicks stamp, TimeTicks swap_stamp);
+  void SetFirstMeaningfulPaint(TimeTicks swap_stamp);
 
   bool next_paint_is_meaningful_ = false;
   HadUserInput had_user_input_ = kNoUserInput;

third_party/blink/renderer/core/paint/first_meaningful_paint_detector_test.cc

@@ -109,7 +109,6 @@ TEST_F(FirstMeaningfulPaintDetectorTest, NoFirstPaint) {
   SimulateLayoutAndPaint(1);
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 0U);
   SimulateNetworkStable();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
 }
 
@@ -119,14 +118,8 @@ TEST_F(FirstMeaningfulPaintDetectorTest, OneLayout) {
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 1U);
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
   TimeTicks after_paint = AdvanceClockAndGetTime();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   SimulateNetworkStable();
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(),
-            GetPaintTiming().FirstPaintRendered());
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
-  EXPECT_LT(GetPaintTiming().FirstMeaningfulPaintRendered(), after_paint);
   EXPECT_LT(GetPaintTiming().FirstMeaningfulPaint(), after_paint);
 }
 
@@ -141,12 +134,8 @@ TEST_F(FirstMeaningfulPaintDetectorTest, TwoLayoutsSignificantSecond) {
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
   TimeTicks after_layout2 = AdvanceClockAndGetTime();
   SimulateNetworkStable();
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(), after_layout1);
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), after_layout1);
-  EXPECT_LT(GetPaintTiming().FirstMeaningfulPaintRendered(), after_layout2);
   EXPECT_LT(GetPaintTiming().FirstMeaningfulPaint(), after_layout2);
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest, TwoLayoutsSignificantFirst) {
@@ -158,11 +147,8 @@ TEST_F(FirstMeaningfulPaintDetectorTest, TwoLayoutsSignificantFirst) {
   SimulateLayoutAndPaint(1);
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 0U);
   SimulateNetworkStable();
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(),
-            GetPaintTiming().FirstPaintRendered());
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
             GetPaintTiming().FirstPaintRendered());
-  EXPECT_LT(GetPaintTiming().FirstMeaningfulPaintRendered(), after_layout1);
   EXPECT_LT(GetPaintTiming().FirstMeaningfulPaint(), after_layout1);
 }
 
@@ -215,14 +201,10 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 1U);
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
   SimulateNetworkStable();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   MarkFirstContentfulPaintAndClearSwapPromise();
   SimulateNetworkStable();
-  EXPECT_NE(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_NE(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest,
@@ -234,12 +216,8 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   platform_->AdvanceClock(TimeDelta::FromMilliseconds(1));
   MarkFirstContentfulPaintAndClearSwapPromise();
   SimulateNetworkStable();
-  EXPECT_GE(GetPaintTiming().FirstMeaningfulPaintRendered(),
-            GetPaintTiming().FirstContentfulPaintRendered());
   EXPECT_GE(GetPaintTiming().FirstMeaningfulPaint(),
             GetPaintTiming().FirstContentfulPaint());
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest, Network2QuietThen0Quiet) {
@@ -257,13 +235,9 @@ TEST_F(FirstMeaningfulPaintDetectorTest, Network2QuietThen0Quiet) {
   SimulateNetwork0Quiet();
 
   // The first paint is FirstMeaningfulPaint.
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
-  EXPECT_LT(GetPaintTiming().FirstMeaningfulPaintRendered(), after_first_paint);
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), after_first_paint);
   EXPECT_LT(GetPaintTiming().FirstMeaningfulPaint(), after_first_paint_swap);
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest, Network0QuietThen2Quiet) {
@@ -282,15 +256,8 @@ TEST_F(FirstMeaningfulPaintDetectorTest, Network0QuietThen2Quiet) {
   SimulateNetwork2Quiet();
 
   // The second paint is FirstMeaningfulPaint.
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(), after_first_paint);
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), after_first_paint);
-  EXPECT_LT(GetPaintTiming().FirstMeaningfulPaintRendered(),
-            after_second_paint);
   EXPECT_LT(GetPaintTiming().FirstMeaningfulPaint(), after_second_paint);
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest,
@@ -301,7 +268,6 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 1U);
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
   SimulateNetworkStable();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
 }
 
@@ -312,10 +278,7 @@ TEST_F(FirstMeaningfulPaintDetectorTest, UserInteractionBeforeFirstPaint) {
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 1U);
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
   SimulateNetworkStable();
-  EXPECT_NE(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_NE(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest,
@@ -324,13 +287,9 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   SimulateLayoutAndPaint(10);
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 1U);
   SimulateNetworkStable();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
-  EXPECT_NE(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_NE(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest,
@@ -343,22 +302,17 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 2U);
   // Having outstanding swap promises should defer setting FMP.
   SimulateNetworkStable();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   // Clearing the first swap promise should have no effect on FMP.
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 1U);
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   TimeTicks after_first_swap = AdvanceClockAndGetTime();
   // Clearing the last outstanding swap promise should set FMP.
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 0U);
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), after_first_swap);
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest,
@@ -367,25 +321,16 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   SimulateLayoutAndPaint(10);
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 1U);
   ClearProvisionalFirstMeaningfulPaintSwapPromise();
-  TimeTicks after_first_meaningful_paint_candidate = AdvanceClockAndGetTime();
   platform_->AdvanceClock(TimeDelta::FromMilliseconds(1));
   GetPaintTiming().MarkFirstContentfulPaint();
   // FCP > FMP candidate, but still waiting for FCP swap.
   SimulateNetworkStable();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   // Trigger notifying the detector about the FCP swap.
   ClearFirstContentfulPaintSwapPromise();
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(),
-            GetPaintTiming().FirstContentfulPaintRendered());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(),
             GetPaintTiming().FirstContentfulPaint());
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(),
-            after_first_meaningful_paint_candidate);
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(),
-            GetPaintTiming().FirstMeaningfulPaintRendered());
 }
 
 TEST_F(FirstMeaningfulPaintDetectorTest,
@@ -399,7 +344,6 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   TimeTicks pre_stable_timestamp = AdvanceClockAndGetTime();
   platform_->AdvanceClock(TimeDelta::FromMilliseconds(1));
   SimulateNetwork2Quiet();
-  EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
 
   // Force another FMP candidate while there is a pending swap promise and the
@@ -413,11 +357,8 @@ TEST_F(FirstMeaningfulPaintDetectorTest,
   // non-swap timestamp is used.
   ClearProvisionalFirstMeaningfulPaintSwapPromise(pre_stable_timestamp);
   EXPECT_EQ(OutstandingDetectorSwapPromiseCount(), 0U);
-  EXPECT_GT(GetPaintTiming().FirstMeaningfulPaintRendered(), TimeTicks());
   EXPECT_GT(GetPaintTiming().FirstMeaningfulPaint(), TimeTicks());
   EXPECT_EQ(GetPaintTiming().FirstMeaningfulPaint(), pre_stable_timestamp);
-  EXPECT_LT(GetPaintTiming().FirstMeaningfulPaintRendered(),
-            pre_stable_timestamp);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/paint/paint_timing.cc

@@ -98,12 +98,9 @@ void PaintTiming::SetFirstMeaningfulPaintCandidate(TimeTicks timestamp) {
 }
 
 void PaintTiming::SetFirstMeaningfulPaint(
-    TimeTicks stamp,
     TimeTicks swap_stamp,
     FirstMeaningfulPaintDetector::HadUserInput had_input) {
-  DCHECK(first_meaningful_paint_.is_null());
   DCHECK(first_meaningful_paint_swap_.is_null());
-  DCHECK(!stamp.is_null());
   DCHECK(!swap_stamp.is_null());
 
   TRACE_EVENT_MARK_WITH_TIMESTAMP2(
@@ -119,7 +116,6 @@ void PaintTiming::SetFirstMeaningfulPaint(
   // Notify FMP for UMA only if there's no user input before FMP, so that layout
   // changes caused by user interactions wouldn't be considered as FMP.
   if (had_input == FirstMeaningfulPaintDetector::kNoUserInput) {
-    first_meaningful_paint_ = stamp;
     first_meaningful_paint_swap_ = swap_stamp;
     NotifyPaintTimingChanged();
   }

third_party/blink/renderer/core/paint/paint_timing.h

@@ -57,7 +57,6 @@ class CORE_EXPORT PaintTiming final
 
   void SetFirstMeaningfulPaintCandidate(TimeTicks timestamp);
   void SetFirstMeaningfulPaint(
-      TimeTicks stamp,
       TimeTicks swap_stamp,
       FirstMeaningfulPaintDetector::HadUserInput had_input);
   void NotifyPaint(bool is_first_paint, bool text_painted, bool image_painted);
@@ -146,10 +145,6 @@ class CORE_EXPORT PaintTiming final
     return first_contentful_paint_;
   }
 
-  TimeTicks FirstMeaningfulPaintRendered() const {
-    return first_meaningful_paint_;
-  }
-
   // TODO(crbug/738235): Non first_*_swap_ variables are only being tracked to
   // compute deltas for reporting histograms and should be removed once we
   // confirm the deltas and discrepancies look reasonable.
@@ -161,7 +156,6 @@ class CORE_EXPORT PaintTiming final
   TimeTicks first_image_paint_swap_;
   TimeTicks first_contentful_paint_;
   TimeTicks first_contentful_paint_swap_;
-  TimeTicks first_meaningful_paint_;
   TimeTicks first_meaningful_paint_swap_;
   TimeTicks first_meaningful_paint_candidate_;