FirstMeaningfulPaint UMA

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

+// Copyright 2016 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 "core/paint/FirstMeaningfulPaintDetector.h"
+
+#include "core/css/FontFaceSet.h"
+#include "core/fetch/ResourceFetcher.h"
+#include "core/paint/PaintTiming.h"
+
+namespace blink {
+
+namespace {
+
+// Web fonts that laid out more than this number of characters block First
+// 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 kTimeWithoutNetworkActivity = 2.0;

[Charlie Harrison, 2016/08/09 16:27:32]

Hm, this name isn't quite clear. The name as-is makes me think it is counting
the current time without network activity, rather than establishing a threshold.
Could we s/Time/Seconds/ and possibly add "Threshold" on the end?

[Kunihiko Sakamoto, 2016/08/10 02:33:32]

Thanks for the suggestion. Done.

+
+} // namespace
+
+FirstMeaningfulPaintDetector& FirstMeaningfulPaintDetector::from(Document& document)
+{
+    return PaintTiming::from(document).firstMeaningfulPaintDetector();
+}
+
+FirstMeaningfulPaintDetector::FirstMeaningfulPaintDetector(PaintTiming* paintTiming)
+    : m_paintTiming(paintTiming)
+    , m_networkStableTimer(this, &FirstMeaningfulPaintDetector::networkStableTimerFired)
+{
+}
+
+Document* FirstMeaningfulPaintDetector::document()
+{
+    return m_paintTiming->document();
+}
+
+void FirstMeaningfulPaintDetector::computeLayoutSignificance(

[Charlie Harrison, 2016/08/09 16:27:32]

It would help to document this method with justification, etc.

[kouhei (in TOK), 2016/08/10 01:58:13]

markNextPaintAsMeaningfulIfNeeded?

[Kunihiko Sakamoto, 2016/08/10 02:33:32]

Done.

[Kunihiko Sakamoto, 2016/08/10 02:33:32]

Yeah, that's a better description. Done.

+    const LayoutObjectCounter& counter, int contentsHeightBeforeLayout,
+    int contentsHeightAfterLayout, int visibleHeight)
+{
+    if (m_state == Reported)
+        return;
+
+    unsigned delta = counter.count() - m_prevLayoutObjectCount;
+    m_prevLayoutObjectCount = counter.count();
+
+    double ratioBefore = std::max(1.0, static_cast<double>(contentsHeightBeforeLayout) / visibleHeight);

[Charlie Harrison, 2016/08/09 16:27:32]

Can visibleHeight ever be 0? If not can you add a DCHECK (possibly with an
explanation)?

[Kunihiko Sakamoto, 2016/08/10 02:33:31]

Good point, actually it can be 0.
Added an early return, since layout significance in such a case is considered to
be 0.

+    double ratioAfter = std::max(1.0, static_cast<double>(contentsHeightAfterLayout) / visibleHeight);
+    double significance = delta / ((ratioBefore + ratioAfter) / 2);
+
+    int approximateBlankCharacterCount = FontFaceSet::approximateBlankCharacterCount(*document());
+    if (approximateBlankCharacterCount > kBlankCharactersThreshold) {
+        m_accumulatedSignificanceWhileHavingBlankText += significance;
+    } else {
+        significance += m_accumulatedSignificanceWhileHavingBlankText;
+        m_accumulatedSignificanceWhileHavingBlankText = 0;
+        if (significance > m_maxSignificanceSoFar) {
+            m_state = NextPaintIsMeaningful;
+            m_maxSignificanceSoFar = significance;
+        }
+    }
+}
+
+void FirstMeaningfulPaintDetector::notifyPaint()
+{
+    if (m_state != NextPaintIsMeaningful)
+        return;
+
+    // Skip document background-only paints.
+    if (m_paintTiming->firstPaint() == 0.0)
+        return;
+
+    m_provisionalFirstMeaningfulPaint = monotonicallyIncreasingTime();
+    m_state = NextPaintIsNotMeaningful;
+}
+
+void FirstMeaningfulPaintDetector::checkNetworkStable()
+{
+    if (m_state == Reported || !document() || document()->fetcher()->hasPendingRequest())

[kouhei (in TOK), 2016/08/10 01:58:13]

Move !document() -> DCHECK(document());

[Kunihiko Sakamoto, 2016/08/10 02:33:32]

Done.

+        return;
+
+    if (m_networkStableTimer.isActive())

[kouhei (in TOK), 2016/08/10 01:58:13]

if (!m_networkStableTimer.isActive())
    m_networkStableTimer.startOneShot(kTimeWithoutNetworkActivity,
BLINK_FROM_HERE);

[Kunihiko Sakamoto, 2016/08/10 02:33:31]

We need to update the timer to fire after 2 secs after this point.

It seems startOneShot() cancels active one if exists, so I've just removed this
conditional.

+        m_networkStableTimer.stop();
+    m_networkStableTimer.startOneShot(kTimeWithoutNetworkActivity, BLINK_FROM_HERE);
+}
+
+void FirstMeaningfulPaintDetector::networkStableTimerFired(TimerBase*)
+{
+    if (m_state == Reported || !document() || document()->fetcher()->hasPendingRequest())
+        return;
+
+    if (m_provisionalFirstMeaningfulPaint)
+        m_paintTiming->setFirstMeaningfulPaint(m_provisionalFirstMeaningfulPaint);
+    m_state = Reported;
+}
+
+DEFINE_TRACE(FirstMeaningfulPaintDetector)
+{
+    visitor->trace(m_paintTiming);
+}
+
+} // namespace blink