Add new UseCounter metric

third_party/WebKit/Source/core/frame/UseCounter.cpp

 /*
  * Copyright (C) 2012 Google, Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 564 matching lines @@
         return 0;
     }
 
     ASSERT_NOT_REACHED();
     return 0;
 }
 
 // Make sure update_use_counter_css.py was run which updates histograms.xml.
 static int maximumCSSSampleId() { return 539; }
 
-static EnumerationHistogram& featureObserverHistogram()
+static EnumerationHistogram& useCounterHistogram()

[dtapuska, 2016/09/09 18:30:19]

shouldn't these statics really be in an anonymous namespace?

[Rick Byers, 2016/09/09 19:31:37]

I don't think our coding style says to prefer one over the other (it's
equivalent in these cases AFAIK), but it's certainly more modern.  Done.  This
required some shuffling though.

 {
-    DEFINE_STATIC_LOCAL(EnumerationHistogram, histogram, ("WebCore.FeatureObserver", UseCounter::NumberOfFeatures));
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, histogram, ("WebCore.UseCounter.Features", UseCounter::NumberOfFeatures));
     return histogram;

[dtapuska, 2016/09/09 18:30:19]

Perhaps we should use temporary names because we haven't solved the SVG Image
problem yet. And we want to verify that this data is going to be more accurate
fist. Then solve the SVG Image problem. And then flip the name to the sensible
name.

[Rick Byers, 2016/09/09 19:31:36]

Done.

 }
 
+static EnumerationHistogram& CSSUseCounterHistogram()
+{
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, histogram, ("WebCore.UseCounter.CSSProperties", maximumCSSSampleId()));
+    return histogram;
+}
+
+UseCounter::UseCounter()
+    : m_muteCount(0)
+    , m_featuresRecorded(NumberOfFeatures)
+    , m_CSSRecorded(lastUnresolvedCSSProperty + 1)
+{
+}
+
 void UseCounter::muteForInspector()
 {
     m_muteCount++;
 }
 
 void UseCounter::unmuteForInspector()
 {
     m_muteCount--;
 }
 
 void UseCounter::recordMeasurement(Feature feature)
 {
     if (m_muteCount)
         return;
 
-    DCHECK(feature != PageDestruction); // PageDestruction is reserved as a scaling factor.
+    DCHECK(feature != OBSOLETE_PageDestruction && feature != PageVisits); // PageDestruction is reserved as a scaling factor.
     DCHECK(feature < NumberOfFeatures);
 
-    if (!m_featureBits.quickGet(feature)) {
+    if (!m_featuresRecorded.quickGet(feature)) {
+        // Note that HTTPArchive tooling looks specifically for this event - see https://github.com/HTTPArchive/httparchive/issues/59
         TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.feature_usage"), "FeatureFirstUsed", "feature", feature);
-        m_featureBits.quickSet(feature);
+        useCounterHistogram().count(feature);
+        m_featuresRecorded.quickSet(feature);
     }
+    m_legacyCounter.countFeature(feature);

[dtapuska, 2016/09/09 18:30:19]

Can this not go up into the conditional?

[Rick Byers, 2016/09/09 19:31:37]

No, the legacy counter has it's own set of recorded bits.  I think it's
equivalent today but wont be when I fix the SVG issue.

 }
 
 bool UseCounter::hasRecordedMeasurement(Feature feature) const
 {
     if (m_muteCount)
         return false;
 
-    DCHECK(feature != PageDestruction); // PageDestruction is reserved as a scaling factor.
+    DCHECK(feature != OBSOLETE_PageDestruction && feature != PageVisits); // PageDestruction is reserved as a scaling factor.
     DCHECK(feature < NumberOfFeatures);
 
-    return m_featureBits.quickGet(feature);
-}
-
-UseCounter::UseCounter()
-    : m_muteCount(0)
-    , m_featureBits(NumberOfFeatures)
-    , m_CSSFeatureBits(lastUnresolvedCSSProperty + 1)
-{
-}
-
-UseCounter::~UseCounter()
-{
-    // We always log PageDestruction so that we have a scale for the rest of the features.
-    // TODO(rbyers): This is flawed due to renderer fast shutdown - crbug.com/597963
-    featureObserverHistogram().count(PageDestruction);
-
-    updateMeasurements();
-}
-
-void UseCounter::updateMeasurements()
-{
-    EnumerationHistogram& featureHistogram = featureObserverHistogram();
-    featureHistogram.count(PageVisits);
-    for (size_t i = 0; i < NumberOfFeatures; ++i) {
-        if (m_featureBits.quickGet(i))
-            featureHistogram.count(i);
-    }
-    // Clearing count bits is timing sensitive.
-    m_featureBits.clearAll();
-
-    // FIXME: Sometimes this function is called more than once per page. The following
-    //        bool guards against incrementing the page count when there are no CSS
-    //        bits set. https://crbug.com/236262.
-    DEFINE_STATIC_LOCAL(EnumerationHistogram, cssPropertiesHistogram, ("WebCore.FeatureObserver.CSSProperties", maximumCSSSampleId()));
-    bool needsPagesMeasuredUpdate = false;
-    for (size_t i = firstCSSProperty; i <= lastUnresolvedCSSProperty; ++i) {
-        if (m_CSSFeatureBits.quickGet(i)) {
-            int cssSampleId = mapCSSPropertyIdToCSSSampleIdForHistogram(static_cast<CSSPropertyID>(i));
-            cssPropertiesHistogram.count(cssSampleId);
-            needsPagesMeasuredUpdate = true;
-        }
-    }
-
-    if (needsPagesMeasuredUpdate)
-        cssPropertiesHistogram.count(totalPagesMeasuredCSSSampleId());
-
-    m_CSSFeatureBits.clearAll();
+    return m_featuresRecorded.quickGet(feature);
 }
 
 void UseCounter::didCommitLoad()
 {
     // TODO(rbyers): This gets invoked more than expected.  crbug.com/236262
     // Eg. every SVGImage has it's own Page instance, they should probably all be delegating
     // their UseCounter to the containing Page.
-    updateMeasurements();
+    m_legacyCounter.updateMeasurements();
+
+    // TODO: When exactly do we want to do this?
+    m_featuresRecorded.clearAll();
+    useCounterHistogram().count(PageVisits);
+    m_CSSRecorded.clearAll();
+    CSSUseCounterHistogram().count(totalPagesMeasuredCSSSampleId());
 }
 
 void UseCounter::count(const Frame* frame, Feature feature)
 {
     if (!frame)
         return;
     FrameHost* host = frame->host();
     if (!host)
         return;
 
@@ skipping 11 matching lines @@
     if (!frame)
         return false;
     FrameHost* host = frame->host();
     if (!host)
         return false;
     return host->useCounter().hasRecordedMeasurement(feature);
 }
 
 bool UseCounter::isCounted(CSSPropertyID unresolvedProperty)
 {
-    return m_CSSFeatureBits.quickGet(unresolvedProperty);
+    return m_CSSRecorded.quickGet(unresolvedProperty);
 }
 
 bool UseCounter::isCounted(Document& document, const String& string)
 {
     Frame* frame = document.frame();
     if (!frame)
         return false;
     FrameHost* host = frame->host();
     if (!host)
         return false;
@@ skipping 37 matching lines @@
     UseCounter::count(context, feature);
 }
 
 void UseCounter::countCrossOriginIframe(const Document& document, Feature feature)
 {
     LocalFrame* frame = document.frame();
     if (frame && frame->isCrossOriginSubframe())
         count(frame, feature);
 }
 
-void UseCounter::count(CSSParserMode cssParserMode, CSSPropertyID feature)
+void UseCounter::count(CSSParserMode cssParserMode, CSSPropertyID property)
 {
-    DCHECK(feature >= firstCSSProperty);
-    DCHECK(feature <= lastUnresolvedCSSProperty);
+    DCHECK(property >= firstCSSProperty);
+    DCHECK(property <= lastUnresolvedCSSProperty);
 
     if (!isUseCounterEnabledForMode(cssParserMode) || m_muteCount)
         return;
 
-    if (!m_CSSFeatureBits.quickGet(feature)) {
-        TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.feature_usage"), "CSSFeatureFirstUsed", "feature", feature);
-        m_CSSFeatureBits.quickSet(feature);
+    if (!m_CSSRecorded.quickGet(property)) {
+        // Note that HTTPArchive tooling looks specifically for this event - see https://github.com/HTTPArchive/httparchive/issues/59
+        TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.feature_usage"), "CSSFeatureFirstUsed", "feature", property);
+        CSSUseCounterHistogram().count(mapCSSPropertyIdToCSSSampleIdForHistogram(property));
+        m_CSSRecorded.quickSet(property);
     }
+    m_legacyCounter.countCSS(property);

[dtapuska, 2016/09/09 18:30:19]

likewise why isn't this inside the else?

[Rick Byers, 2016/09/09 19:31:36]

I added some comments to the declarations to make it clear that the semantics of
the legacy and non-legacy bits are independent.   Better?

 }
 
 void UseCounter::count(Feature feature)
 {
     DCHECK(Deprecation::deprecationMessage(feature).isEmpty());
     recordMeasurement(feature);
 }
 
 UseCounter* UseCounter::getFrom(const Document* document)
 {
@@ skipping 11 matching lines @@
 
 UseCounter* UseCounter::getFrom(const StyleSheetContents* sheetContents)
 {
     // FIXME: We may want to handle stylesheets that have multiple owners
     //        https://crbug.com/242125
     if (sheetContents && sheetContents->hasSingleOwnerNode())
         return getFrom(sheetContents->singleOwnerDocument());
     return 0;
 }
 
+/*
+ *
+ * LEGACY metrics support - WebCore.FeatureObserver is to be superceded by WebCore.UseCounter
+ *
+ */
+
+static EnumerationHistogram& featureObserverHistogram()
+{
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, histogram, ("WebCore.FeatureObserver", UseCounter::NumberOfFeatures));
+    return histogram;
+}
+
+UseCounter::LegacyCounter::LegacyCounter()
+    : m_featureBits(NumberOfFeatures)
+    , m_CSSBits(lastUnresolvedCSSProperty + 1)
+{
+}
+
+UseCounter::LegacyCounter::~LegacyCounter()
+{
+    // PageDestruction was intended to be used as a scale, but it's broken (due to fast shutdown).
+    // See https://crbug.com/597963.
+    featureObserverHistogram().count(OBSOLETE_PageDestruction);
+    updateMeasurements();
+}
+
+void UseCounter::LegacyCounter::countFeature(Feature feature)
+{
+    m_featureBits.quickSet(feature);
+}
+
+void UseCounter::LegacyCounter::countCSS(CSSPropertyID property)
+{
+    m_CSSBits.quickSet(property);
+}
+
+void UseCounter::LegacyCounter::updateMeasurements()
+{
+    EnumerationHistogram& featureHistogram = featureObserverHistogram();
+    featureHistogram.count(PageVisits);
+    for (size_t i = 0; i < NumberOfFeatures; ++i) {
+        if (m_featureBits.quickGet(i))
+            featureHistogram.count(i);
+    }
+    // Clearing count bits is timing sensitive.
+    m_featureBits.clearAll();
+
+    // FIXME: Sometimes this function is called more than once per page. The following
+    //        bool guards against incrementing the page count when there are no CSS
+    //        bits set. https://crbug.com/236262.
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, cssPropertiesHistogram, ("WebCore.FeatureObserver.CSSProperties", maximumCSSSampleId()));
+    bool needsPagesMeasuredUpdate = false;
+    for (size_t i = firstCSSProperty; i <= lastUnresolvedCSSProperty; ++i) {
+        if (m_CSSBits.quickGet(i)) {
+            int cssSampleId = mapCSSPropertyIdToCSSSampleIdForHistogram(static_cast<CSSPropertyID>(i));
+            cssPropertiesHistogram.count(cssSampleId);
+            needsPagesMeasuredUpdate = true;
+        }
+    }
+
+    if (needsPagesMeasuredUpdate)
+        cssPropertiesHistogram.count(totalPagesMeasuredCSSSampleId());
+
+    m_CSSBits.clearAll();
+}
+
+
 } // namespace blink