Loosen FirstMeaningfulPaint UMA network-idle heuristics

third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp

Index: third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
diff --git a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
index 9fca4136977ee48f1cbc383fcf7596b21f0748de..c544c7ad14ef81fd575fbe9cc04b61b1bbb8ea24 100644
--- a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
+++ b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
@@ -7,6 +7,7 @@
 #include "core/css/FontFaceSet.h"
 #include "core/dom/TaskRunnerHelper.h"
 #include "core/paint/PaintTiming.h"
+#include "platform/Histogram.h"
 #include "platform/instrumentation/tracing/TraceEvent.h"
 #include "platform/loader/fetch/ResourceFetcher.h"
 
@@ -18,9 +19,10 @@ namespace {
 // Meaningful Paint.
 const int kBlankCharactersThreshold = 200;
 
-// FirstMeaningfulPaintDetector stops observing layouts and reports First
-// Meaningful Paint when this duration passed from last network activity.
-const double kSecondsWithoutNetworkActivityThreshold = 0.5;
+// The page is n-quiet if there are no more than n active network requests for
+// this duration of time.
+const double kNetwork2QuietWindowSeconds = 3;
+const double kNetwork0QuietWindowSeconds = 0.5;
 
 }  // namespace
 
@@ -33,10 +35,14 @@ FirstMeaningfulPaintDetector::FirstMeaningfulPaintDetector(
     PaintTiming* paintTiming,
     Document& document)
     : m_paintTiming(paintTiming),
-      m_networkStableTimer(
+      m_network0QuietTimer(
           TaskRunnerHelper::get(TaskType::UnspecedTimer, &document),
           this,
-          &FirstMeaningfulPaintDetector::networkStableTimerFired) {}
+          &FirstMeaningfulPaintDetector::network0QuietTimerFired),
+      m_network2QuietTimer(
+          TaskRunnerHelper::get(TaskType::UnspecedTimer, &document),
+          this,
+          &FirstMeaningfulPaintDetector::network2QuietTimerFired) {}
 
 Document* FirstMeaningfulPaintDetector::document() {
   return m_paintTiming->supplementable();
@@ -52,7 +58,7 @@ void FirstMeaningfulPaintDetector::markNextPaintAsMeaningfulIfNeeded(
     int contentsHeightBeforeLayout,
     int contentsHeightAfterLayout,
     int visibleHeight) {
-  if (m_state == Reported)
+  if (m_network0QuietReached && m_network2QuietReached)
     return;
 
   unsigned delta = counter.count() - m_prevLayoutObjectCount;
@@ -77,14 +83,14 @@ void FirstMeaningfulPaintDetector::markNextPaintAsMeaningfulIfNeeded(
     significance += m_accumulatedSignificanceWhileHavingBlankText;
     m_accumulatedSignificanceWhileHavingBlankText = 0;
     if (significance > m_maxSignificanceSoFar) {
-      m_state = NextPaintIsMeaningful;
+      m_nextPaintIsMeaningful = true;
       m_maxSignificanceSoFar = significance;
     }
   }
 }
 
 void FirstMeaningfulPaintDetector::notifyPaint() {
-  if (m_state != NextPaintIsMeaningful)
+  if (!m_nextPaintIsMeaningful)
     return;
 
   // Skip document background-only paints.
@@ -92,7 +98,10 @@ void FirstMeaningfulPaintDetector::notifyPaint() {
     return;
 
   m_provisionalFirstMeaningfulPaint = monotonicallyIncreasingTime();
-  m_state = NextPaintIsNotMeaningful;
+  m_nextPaintIsMeaningful = false;
+
+  if (m_network2QuietReached)
+    return;
 
   TRACE_EVENT_MARK_WITH_TIMESTAMP1(
       "loading", "firstMeaningfulPaintCandidate",
@@ -107,28 +116,94 @@ void FirstMeaningfulPaintDetector::notifyPaint() {
   m_paintTiming->markFirstMeaningfulPaintCandidate();
 }
 
-void FirstMeaningfulPaintDetector::checkNetworkStable() {
+bool FirstMeaningfulPaintDetector::isNetworkQuiet(int maxActiveConnections) {
   DCHECK(document());
-  if (m_state == Reported || document()->fetcher()->hasPendingRequest())
+  if (!document()->hasFinishedParsing())
+    return false;
+  ResourceFetcher* fetcher = document()->fetcher();
+  return fetcher->blockingRequestCount() + fetcher->nonblockingRequestCount() <=
+         maxActiveConnections;
+}
+
+void FirstMeaningfulPaintDetector::checkNetworkStable() {
+  if (!m_network0QuietReached && isNetworkQuiet(0)) {
+    m_network0QuietTimer.startOneShot(kNetwork0QuietWindowSeconds,
+                                      BLINK_FROM_HERE);
+  }
+  if (!m_network2QuietReached && isNetworkQuiet(2)) {
+    m_network2QuietTimer.startOneShot(kNetwork2QuietWindowSeconds,
+                                      BLINK_FROM_HERE);
+  }
+}
+
+void FirstMeaningfulPaintDetector::network0QuietTimerFired(TimerBase*) {
+  if (!document() || m_network0QuietReached || !isNetworkQuiet(0) ||
+      !m_paintTiming->firstContentfulPaint())
     return;
+  m_network0QuietReached = true;
 
-  m_networkStableTimer.startOneShot(kSecondsWithoutNetworkActivityThreshold,
-                                    BLINK_FROM_HERE);
+  if (m_provisionalFirstMeaningfulPaint) {
+    // Enforce FirstContentfulPaint <= FirstMeaningfulPaint.
+    m_firstMeaningfulPaint0Quiet =
+        std::max(m_provisionalFirstMeaningfulPaint,
+                 m_paintTiming->firstContentfulPaint());
+  }
+  reportHistograms();
 }
 
-void FirstMeaningfulPaintDetector::networkStableTimerFired(TimerBase*) {
-  if (m_state == Reported || !document() ||
-      document()->fetcher()->hasPendingRequest() ||
+void FirstMeaningfulPaintDetector::network2QuietTimerFired(TimerBase*) {
+  if (!document() || m_network2QuietReached || !isNetworkQuiet(2) ||
       !m_paintTiming->firstContentfulPaint())
     return;
+  m_network2QuietReached = true;
 
   if (m_provisionalFirstMeaningfulPaint) {
     // Enforce FirstContentfulPaint <= FirstMeaningfulPaint.
-    double timestamp = std::max(m_provisionalFirstMeaningfulPaint,
-                                m_paintTiming->firstContentfulPaint());
-    m_paintTiming->setFirstMeaningfulPaint(timestamp);
+    m_firstMeaningfulPaint2Quiet =
+        std::max(m_provisionalFirstMeaningfulPaint,
+                 m_paintTiming->firstContentfulPaint());
+    // Report FirstMeaningfulPaint when the page reached network 2-quiet.
+    m_paintTiming->setFirstMeaningfulPaint(m_firstMeaningfulPaint2Quiet);
+  }
+  reportHistograms();
+}
+
+void FirstMeaningfulPaintDetector::reportHistograms() {
+  enum HadNetworkQuiet {

[Ilya Sherman, 2017/03/08 09:19:46]

Please document that this enum should be treated as append-only.

[Kunihiko Sakamoto, 2017/03/08 10:16:10]

Done.

+    HadNetwork0Quiet,
+    HadNetwork2Quiet,
+    HadNetworkQuietEnumMax
+  };
+  DEFINE_STATIC_LOCAL(
+      EnumerationHistogram, hadNetworkQuietHistogram,
+      ("FirstMeaningfulPaintDetector.HadNetworkQuiet", HadNetworkQuietEnumMax));
+
+  enum FMPOrderingEnum {

[Ilya Sherman, 2017/03/08 09:19:46]

And this one too =)

[Kunihiko Sakamoto, 2017/03/08 10:16:10]

Done.

+    FMP0QuietFirst,
+    FMP2QuietFirst,
+    FMP0QuietEqualFMP2Quiet,
+    FMPOrderingEnumMax
+  };
+  DEFINE_STATIC_LOCAL(
+      EnumerationHistogram, firstMeaningfulPaintOrderingHistogram,
+      ("FirstMeaningfulPaintDetector.FirstMeaningfulPaintOrdering",
+       FMPOrderingEnumMax));
+
+  if (m_firstMeaningfulPaint0Quiet && m_firstMeaningfulPaint2Quiet) {
+    int sample;
+    if (m_firstMeaningfulPaint2Quiet < m_firstMeaningfulPaint0Quiet) {
+      sample = FMP0QuietFirst;
+    } else if (m_firstMeaningfulPaint2Quiet > m_firstMeaningfulPaint0Quiet) {
+      sample = FMP2QuietFirst;
+    } else {
+      sample = FMP0QuietEqualFMP2Quiet;
+    }
+    firstMeaningfulPaintOrderingHistogram.count(sample);
+  } else if (m_firstMeaningfulPaint0Quiet) {
+    hadNetworkQuietHistogram.count(HadNetwork0Quiet);
+  } else if (m_firstMeaningfulPaint2Quiet) {
+    hadNetworkQuietHistogram.count(HadNetwork2Quiet);
   }
-  m_state = Reported;
 }
 
 DEFINE_TRACE(FirstMeaningfulPaintDetector) {