Bug 1518999 - Update ContentfulPaint algorithm to follow the spec r=emilio
authorSean Feng <sefeng@mozilla.com>
Tue, 27 Oct 2020 16:13:23 +0000
changeset 554728 47629f89cb6a8a8a663789855591a3d17466493b
parent 554727 3fbb53f6d6ef6ec9fc2099fa25abeadfd616b6ba
child 554729 18977625f8a4df6ed7c0bd4829ef1be2c6226c26
push id37898
push userabutkovits@mozilla.com
push dateWed, 28 Oct 2020 09:24:21 +0000
treeherdermozilla-central@83bf4fd3b1fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1518999
milestone84.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518999 - Update ContentfulPaint algorithm to follow the spec r=emilio This patch includes a couple of changes. 1) Notify contentful paint only during refresh driver ticks. 2) Not only the root document, sub document should also have their own contentful paint entry. 3) Consider invisible text as contentful as well. Differential Revision: https://phabricator.services.mozilla.com/D89498
layout/base/nsPresContext.cpp
layout/base/nsRefreshDriver.cpp
layout/base/nsRefreshDriver.h
layout/painting/nsDisplayList.cpp
testing/web-platform/tests/paint-timing/fcp-only/fcp-ensure-update-the-rendering-step.html
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -2419,20 +2419,36 @@ void nsPresContext::NotifyNonBlankPaint(
 void nsPresContext::NotifyContentfulPaint() {
   if (!mHadContentfulPaint) {
 #if defined(MOZ_WIDGET_ANDROID)
     (new AsyncEventDispatcher(mDocument, u"MozFirstContentfulPaint"_ns,
                               CanBubble::eYes, ChromeOnlyDispatch::eYes))
         ->PostDOMEvent();
 #endif
     mHadContentfulPaint = true;
-    if (IsRootContentDocument()) {
-      if (nsRootPresContext* rootPresContext = GetRootPresContext()) {
-        mFirstContentfulPaintTransactionId =
-            Some(rootPresContext->mRefreshDriver->LastTransactionId().Next());
+    if (nsRootPresContext* rootPresContext = GetRootPresContext()) {
+      mFirstContentfulPaintTransactionId =
+          Some(rootPresContext->mRefreshDriver->LastTransactionId().Next());
+      if (nsPIDOMWindowInner* innerWindow = mDocument->GetInnerWindow()) {
+        if (Performance* perf = innerWindow->GetPerformance()) {
+          TimeStamp nowTime =
+              rootPresContext->RefreshDriver()->MostRecentRefresh(
+                  /* aEnsureTimerStarted */ false);
+          MOZ_ASSERT(
+              !nowTime.IsNull(),
+              "Most recent refresh timestamp should exist since we are in "
+              "a refresh driver tick");
+          MOZ_ASSERT(rootPresContext->RefreshDriver()->IsInRefresh(),
+                     "We should only notify contentful paint during refresh "
+                     "driver ticks");
+          RefPtr<PerformancePaintTiming> paintTiming =
+              new PerformancePaintTiming(perf, u"first-contentful-paint"_ns,
+                                         nowTime);
+          perf->SetFCPTimingEntry(paintTiming);
+        }
       }
     }
   }
 }
 
 void nsPresContext::NotifyPaintStatusReset() {
   mHadNonBlankPaint = false;
   mHadContentfulPaint = false;
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -1239,20 +1239,20 @@ void nsRefreshDriver::AdvanceTimeAndRefr
 }
 
 void nsRefreshDriver::RestoreNormalRefresh() {
   mTestControllingRefreshes = false;
   EnsureTimerStarted(eAllowTimeToGoBackwards);
   mCompletedTransaction = mOutstandingTransactionId = mNextTransactionId;
 }
 
-TimeStamp nsRefreshDriver::MostRecentRefresh() const {
+TimeStamp nsRefreshDriver::MostRecentRefresh(bool aEnsureTimerStarted) const {
   // In case of stylo traversal, we have already activated the refresh driver in
   // RestyleManager::ProcessPendingRestyles().
-  if (!ServoStyleSet::IsInServoTraversal()) {
+  if (aEnsureTimerStarted && !ServoStyleSet::IsInServoTraversal()) {
     const_cast<nsRefreshDriver*>(this)->EnsureTimerStarted();
   }
 
   return mMostRecentRefresh;
 }
 
 void nsRefreshDriver::AddRefreshObserver(nsARefreshObserver* aObserver,
                                          FlushType aFlushType,
--- a/layout/base/nsRefreshDriver.h
+++ b/layout/base/nsRefreshDriver.h
@@ -74,17 +74,17 @@ class nsRefreshDriver final : public moz
 
   /**
    * Return the time of the most recent refresh.  This is intended to be
    * used by callers who want to start an animation now and want to know
    * what time to consider the start of the animation.  (This helps
    * ensure that multiple animations started during the same event off
    * the main event loop have the same start time.)
    */
-  mozilla::TimeStamp MostRecentRefresh() const;
+  mozilla::TimeStamp MostRecentRefresh(bool aEnsureTimerStarted = true) const;
 
   /**
    * Add / remove refresh observers.
    * RemoveRefreshObserver returns true if aObserver was found.
    *
    * The flush type affects:
    *   + the order in which the observers are notified (lowest flush
    *     type to highest, in order registered)
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -1146,20 +1146,24 @@ void nsDisplayListBuilder::LeavePresShel
   if (mIsPaintingToWindow && aPaintedContents) {
     nsPresContext* pc = aReferenceFrame->PresContext();
     if (!pc->HadNonBlankPaint()) {
       if (!CurrentPresShellState()->mIsBackgroundOnly &&
           DisplayListIsNonBlank(aPaintedContents)) {
         pc->NotifyNonBlankPaint();
       }
     }
-    if (!pc->HadContentfulPaint()) {
-      if (!CurrentPresShellState()->mIsBackgroundOnly &&
-          DisplayListIsContentful(aPaintedContents)) {
-        pc->NotifyContentfulPaint();
+    nsRootPresContext* rootPresContext = pc->GetRootPresContext();
+    if (!pc->HadContentfulPaint() && rootPresContext &&
+        rootPresContext->RefreshDriver()->IsInRefresh()) {
+      if (!CurrentPresShellState()->mIsBackgroundOnly) {
+        if (pc->HasEverBuiltInvisibleText() ||
+            DisplayListIsContentful(this, aPaintedContents)) {
+          pc->NotifyContentfulPaint();
+        }
       }
     }
   }
 
   ResetMarkedFramesForDisplayList(aReferenceFrame);
   mPresShellStates.RemoveLastElement();
 
   if (!mPresShellStates.IsEmpty()) {
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/paint-timing/fcp-only/fcp-ensure-update-the-rendering-step.html
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<head>
+    <title>
+      Ensure the timing is marked during the `update the rendering` step.
+    </title>
+</head>
+<style>
+    #main {
+        width: 100px;
+        height: 100px;
+        left: 0px;
+        position: relative;
+        top: 0;
+        background-image: url(../resources/circles.png);
+        opacity: 0;
+    }
+
+    #main.contentful {
+        opacity: 0.1;
+    }
+</style>
+<body>
+<script src="../resources/utils.js"></script>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="main"></div>
+<script>
+setup({"hide_test_state": true});
+async_test(function (t) {
+    assert_implements(window.PerformancePaintTiming, "Paint Timing isn't supported.");
+    let fired = false;
+    const main = document.getElementById('main');
+    let animationFrameStamps = [];
+    requestAnimationFrame(function frame(stamp) {
+        animationFrameStamps.unshift(stamp);
+        main.className = "contentful";
+        while (performance.now() - stamp <= 5) {
+          /* Busy-wait */
+        }
+        if(!fired)
+            requestAnimationFrame(frame);
+    });
+    new PerformanceObserver(t.step_func(list=>{
+        for (let entry of list.getEntries()) {
+            if (entry.name == "first-contentful-paint") {
+                fired = true;
+                assert_any(assert_approx_equals, entry.startTime, animationFrameStamps, 1, "One of the past requestAnimationFrame should have the same timestamp as paint entry");
+                t.done();
+            }
+        }
+    })).observe({type: "paint"});
+}, 'The first-contentful-paint timestamp should be same as the last RAF');
+</script>
+</body>
+</html>