Exclude initially out-of-view frames for paint timing (behind feature)

Behind kPaintTimingNoOutOfViewFrames feature (disabled by default for
now), paint timing is excluded for frames that are initially
out-of-view, to make related metrics reflect the performance, instead
of depending on paint implementation (e.g. how we paint out-of-view
contents) and arbitrary user actions (e.g. when the user scrolls
out-of-view contents near/into view).

Bug: 1236136
Change-Id: I316f0d0c7e8f77545fe564570d21f37e272f7660
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3072009
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#908961}

third_party/blink/common/features.cc

@@ -1055,6 +1055,13 @@ const base::Feature kDesktopPWAsSubApps{"DesktopPWAsSubApps",
 const base::Feature kReportAllJavascriptFrameworks{
     "ReportAllJavaScriptFrameworks", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Whether to exclude out-of-view frames for paint timing, to make paint timing
+// actually measure user experience instead of depending on paint implementation
+// (e.g. whether to paint out-of-view frames) and arbitrary user actions (e.g.
+// scrolling a never-painted frame into view).
+const base::Feature kPaintTimingNoOutOfViewFrames{
+    "PaintTimingNoOutOfViewFrames", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Suppresses console errors for CORS problems which report an associated
 // inspector issue anyway.
 const base::Feature kCORSErrorsIssueOnly{"CORSErrorsIssueOnly",

third_party/blink/public/common/features.h

@@ -452,6 +452,12 @@ BLINK_COMMON_EXPORT extern const base::Feature kDesktopPWAsSubApps;
 // detect the properties and attributes required.
 BLINK_COMMON_EXPORT extern const base::Feature kReportAllJavascriptFrameworks;
 
+// Whether to exclude out-of-view frames for paint timing, to make paint timing
+// actually measure user experience instead of depending on paint implementation
+// (e.g. whether to paint out-of-view frames) and arbitrary user actions (e.g.
+// scrolling a frame into view making it eligible for paint).
+BLINK_COMMON_EXPORT extern const base::Feature kPaintTimingNoOutOfViewFrames;
+
 // Suppresses console errors for CORS problems which report an associated
 // inspector issue anyway.
 BLINK_COMMON_EXPORT extern const base::Feature kCORSErrorsIssueOnly;

third_party/blink/renderer/core/BUILD.gn

@@ -1529,6 +1529,7 @@ source_set("unit_tests") {
     "paint/paint_property_tree_builder_test.h",
     "paint/paint_property_tree_printer_test.cc",
     "paint/paint_property_tree_update_tests.cc",
+    "paint/paint_timing_test.cc",
     "paint/paint_timing_test_helper.h",
     "paint/pre_paint_tree_walk_test.cc",
     "paint/scrollable_area_painter_test.cc",

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

@@ -4384,6 +4384,15 @@ void LocalFrameView::CrossOriginToParentFrameChanged() {
   }
 }
 
+void LocalFrameView::SetViewportIntersection(
+    const mojom::blink::ViewportIntersectionState& intersection_state) {
+  if (frame_ && frame_->GetDocument()) {
+    PaintTiming::From(*frame_->GetDocument())
+        .SetFrameIsOutOfView(
+            intersection_state.viewport_intersection.IsEmpty());
+  }
+}
+
 void LocalFrameView::VisibilityForThrottlingChanged() {
   if (FrameScheduler* frame_scheduler = frame_->GetFrameScheduler()) {
     // TODO(szager): Per crbug.com/994443, maybe this should be:

third_party/blink/renderer/core/frame/local_frame_view.h

@@ -749,7 +749,7 @@ class CORE_EXPORT LocalFrameView final
   void ParentVisibleChanged() override;
   void NotifyFrameRectsChangedIfNeeded();
   void SetViewportIntersection(const mojom::blink::ViewportIntersectionState&
-                                   intersection_state) override {}
+                                   intersection_state) override;
   void VisibilityForThrottlingChanged() override;
   bool LifecycleUpdatesThrottled() const override {
     return lifecycle_updates_throttled_;

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

@@ -9,6 +9,7 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/time/default_tick_clock.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/dom/frame_request_callback_collection.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
@@ -215,6 +216,8 @@ void PaintTiming::Trace(Visitor* visitor) const {
 PaintTiming::PaintTiming(Document& document)
     : Supplement<Document>(document),
       fmp_detector_(MakeGarbageCollected<FirstMeaningfulPaintDetector>(this)),
+      excludes_out_of_view_frame_(base::FeatureList::IsEnabled(
+          features::kPaintTimingNoOutOfViewFrames)),
       clock_(base::DefaultTickClock::GetInstance()) {}
 
 LocalFrame* PaintTiming::GetFrame() const {

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

@@ -86,7 +86,9 @@ class CORE_EXPORT PaintTiming final : public GarbageCollected<PaintTiming>,
 
   // FirstPaint returns the first time that anything was painted for the
   // current document.
-  base::TimeTicks FirstPaint() const { return first_paint_presentation_; }
+  base::TimeTicks FirstPaint() const {
+    return ExcludeOutOfViewFrame(first_paint_presentation_);
+  }
 
   // Times when the first paint happens after the page is restored from the
   // back-forward cache. If the element value is zero time tick, the first paint
@@ -104,37 +106,37 @@ class CORE_EXPORT PaintTiming final : public GarbageCollected<PaintTiming>,
   // painted. For instance, the first time that text or image content was
   // painted.
   base::TimeTicks FirstContentfulPaint() const {
-    return first_contentful_paint_presentation_;
+    return ExcludeOutOfViewFrame(first_contentful_paint_presentation_);
   }
 
   // FirstImagePaint returns the first time that image content was painted.
   base::TimeTicks FirstImagePaint() const {
-    return first_image_paint_presentation_;
+    return ExcludeOutOfViewFrame(first_image_paint_presentation_);
   }
 
   // FirstEligibleToPaint returns the first time that the frame is not
   // throttled and is eligible to paint. A null value indicates throttling.
   base::TimeTicks FirstEligibleToPaint() const {
-    return first_eligible_to_paint_;
+    return ExcludeOutOfViewFrame(first_eligible_to_paint_);
   }
 
   // FirstMeaningfulPaint returns the first time that page's primary content
   // was painted.
   base::TimeTicks FirstMeaningfulPaint() const {
-    return first_meaningful_paint_presentation_;
+    return ExcludeOutOfViewFrame(first_meaningful_paint_presentation_);
   }
 
   // The time that the first paint happened after a portal activation.
   base::TimeTicks LastPortalActivatedPaint() const {
-    return last_portal_activated_presentation_;
+    return ExcludeOutOfViewFrame(last_portal_activated_presentation_);
   }
 
   // FirstMeaningfulPaintCandidate indicates the first time we considered a
   // paint to qualify as the potentially first meaningful paint. Unlike
   // firstMeaningfulPaint, this signal is available in real time, but it may be
   // an optimistic (i.e., too early) estimate.
   base::TimeTicks FirstMeaningfulPaintCandidate() const {
-    return first_meaningful_paint_candidate_;
+    return ExcludeOutOfViewFrame(first_meaningful_paint_candidate_);
   }
 
   FirstMeaningfulPaintDetector& GetFirstMeaningfulPaintDetector() {
@@ -155,6 +157,13 @@ class CORE_EXPORT PaintTiming final : public GarbageCollected<PaintTiming>,
 
   void OnRestoredFromBackForwardCache();
 
+  void SetFrameIsOutOfView(bool out_of_view) {
+    if (!frame_initial_out_of_view_is_set_) {
+      frame_is_initially_out_of_view_ = out_of_view;
+      frame_initial_out_of_view_is_set_ = true;
+    }
+  }
+
   void Trace(Visitor*) const override;
 
  private:
@@ -197,10 +206,17 @@ class CORE_EXPORT PaintTiming final : public GarbageCollected<PaintTiming>,
   void RegisterNotifyFirstPaintAfterBackForwardCacheRestorePresentationTime(
       wtf_size_t index);
 
-  base::TimeTicks FirstPaintRendered() const { return first_paint_; }
-
+  base::TimeTicks FirstPaintRendered() const {
+    return ExcludeOutOfViewFrame(first_paint_);
+  }
   base::TimeTicks FirstContentfulPaintRendered() const {
-    return first_contentful_paint_;
+    return ExcludeOutOfViewFrame(first_contentful_paint_);
+  }
+
+  base::TimeTicks ExcludeOutOfViewFrame(base::TimeTicks time) const {
+    return excludes_out_of_view_frame_ && frame_is_initially_out_of_view_
+               ? base::TimeTicks()
+               : time;
   }
 
   // TODO(crbug/738235): Non first_*_presentation_ variables are only being
@@ -228,6 +244,10 @@ class CORE_EXPORT PaintTiming final : public GarbageCollected<PaintTiming>,
   // is restored from the back-forward cache.
   int raf_after_bfcache_restore_measurement_callback_id_ = 0;
 
+  bool excludes_out_of_view_frame_ = false;
+  bool frame_is_initially_out_of_view_ = true;
+  bool frame_initial_out_of_view_is_set_ = false;
+
   const base::TickClock* clock_;
 
   FRIEND_TEST_ALL_PREFIXES(FirstMeaningfulPaintDetectorTest,

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

@@ -0,0 +1,88 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/paint/paint_timing.h"
+
+#include "base/test/scoped_feature_list.h"
+#include "third_party/blink/public/common/features.h"
+#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
+
+namespace blink {
+
+class PaintTimingTest : public RenderingTest {
+ public:
+  PaintTimingTest()
+      : RenderingTest(MakeGarbageCollected<SingleChildLocalFrameClient>()) {}
+};
+
+TEST_F(PaintTimingTest, InViewFrame) {
+  base::test::ScopedFeatureList features;
+  features.InitWithFeatureState(features::kPaintTimingNoOutOfViewFrames, true);
+
+  SetBodyInnerHTML("<iframe></iframe>");
+  SetChildFrameHTML("ABC");
+  UpdateAllLifecyclePhasesForTest();
+
+  PaintTiming& paint_timing = PaintTiming::From(ChildDocument());
+  paint_timing.ReportPresentationTime(PaintEvent::kFirstContentfulPaint,
+                                      WebSwapResult::kDidSwap,
+                                      base::TimeTicks::Now());
+  EXPECT_FALSE(paint_timing.FirstContentfulPaint().is_null());
+}
+
+TEST_F(PaintTimingTest, ExcludeOutOfViewFrame) {
+  base::test::ScopedFeatureList features;
+  features.InitWithFeatureState(features::kPaintTimingNoOutOfViewFrames, true);
+
+  SetBodyInnerHTML(
+      "<iframe style='position: absolute; top: 20000px'></iframe>");
+  SetChildFrameHTML("ABC");
+  UpdateAllLifecyclePhasesForTest();
+
+  // Should not report FCP for the out-of-view frame.
+  PaintTiming& paint_timing = PaintTiming::From(ChildDocument());
+  paint_timing.ReportPresentationTime(PaintEvent::kFirstContentfulPaint,
+                                      WebSwapResult::kDidSwap,
+                                      base::TimeTicks::Now());
+  EXPECT_TRUE(paint_timing.FirstContentfulPaint().is_null());
+}
+
+TEST_F(PaintTimingTest, ExcludeOutOfViewFrameScrolledIntoView) {
+  base::test::ScopedFeatureList features;
+  features.InitWithFeatureState(features::kPaintTimingNoOutOfViewFrames, true);
+
+  SetBodyInnerHTML(
+      "<iframe style='position: absolute; top: 20000px'></iframe>");
+  SetChildFrameHTML("ABC");
+  UpdateAllLifecyclePhasesForTest();
+
+  // Should not report FCP even if the iframe scrolls into view and get painted
+  // because the iframe is initially out-of-view.
+  GetDocument().View()->LayoutViewport()->SetScrollOffset(
+      ScrollOffset(0, 20000), mojom::blink::ScrollType::kProgrammatic);
+  UpdateAllLifecyclePhasesForTest();
+  PaintTiming& paint_timing = PaintTiming::From(ChildDocument());
+  paint_timing.ReportPresentationTime(PaintEvent::kFirstContentfulPaint,
+                                      WebSwapResult::kDidSwap,
+                                      base::TimeTicks::Now());
+  EXPECT_TRUE(paint_timing.FirstContentfulPaint().is_null());
+}
+
+TEST_F(PaintTimingTest, IncludeOutOfViewFrame) {
+  base::test::ScopedFeatureList features;
+  features.InitWithFeatureState(features::kPaintTimingNoOutOfViewFrames, false);
+
+  SetBodyInnerHTML(
+      "<iframe style='position: absolute; top: 20000px'></iframe>");
+  SetChildFrameHTML("ABC");
+  UpdateAllLifecyclePhasesForTest();
+
+  PaintTiming& paint_timing = PaintTiming::From(ChildDocument());
+  paint_timing.ReportPresentationTime(PaintEvent::kFirstContentfulPaint,
+                                      WebSwapResult::kDidSwap,
+                                      base::TimeTicks::Now());
+  EXPECT_FALSE(paint_timing.FirstContentfulPaint().is_null());
+}
+
+}  // namespace blink