Minor UseCounter clean-ups

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 24 matching lines @@
 #include "core/frame/LocalFrame.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "core/workers/WorkerGlobalScope.h"
 #include "platform/Histogram.h"
 #include "platform/TraceEvent.h"
 
 namespace blink {
 
 static int totalPagesMeasuredCSSSampleId() { return 1; }
 
-int UseCounter::mapCSSPropertyIdToCSSSampleIdForHistogram(int id)
+int UseCounter::mapCSSPropertyIdToCSSSampleIdForHistogram(CSSPropertyID cssPropertyID)
 {
-    CSSPropertyID cssPropertyID = static_cast<CSSPropertyID>(id);
-
     switch (cssPropertyID) {
     // Begin at 2, because 1 is reserved for totalPagesMeasuredCSSSampleId.
     case CSSPropertyColor: return 2;
     case CSSPropertyDirection: return 3;
     case CSSPropertyDisplay: return 4;
     case CSSPropertyFont: return 5;
     case CSSPropertyFontFamily: return 6;
     case CSSPropertyFontSize: return 7;
     case CSSPropertyFontStyle: return 8;
     case CSSPropertyFontVariant: return 9;
@@ skipping 539 matching lines @@
 void UseCounter::unmuteForInspector()
 {
     m_muteCount--;
 }
 
 void UseCounter::recordMeasurement(Feature feature)
 {
     if (m_muteCount)
         return;
 
-    if (!m_countBits.hasRecordedMeasurement(feature)) {
+    DCHECK(feature != PageDestruction); // PageDestruction is reserved as a scaling factor.
+    DCHECK(feature < NumberOfFeatures);
+
+    if (!m_featureBits.quickGet(feature)) {
         TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.feature_usage"), "FeatureFirstUsed", "feature", feature);
+        m_featureBits.quickSet(feature);
     }
-    m_countBits.recordMeasurement(feature);
+}
+
+bool UseCounter::hasRecordedMeasurement(Feature feature) const
+{
+    if (m_muteCount)
+        return false;
+
+    DCHECK(feature != PageDestruction); // 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)
 {
-    m_CSSFeatureBits.ensureSize(lastUnresolvedCSSProperty + 1);
-    m_CSSFeatureBits.clearAll();
 }
 
 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::CountBits::updateMeasurements()
+void UseCounter::updateMeasurements()
 {
     EnumerationHistogram& featureHistogram = featureObserverHistogram();
-    for (unsigned i = 0; i < NumberOfFeatures; ++i) {
-        if (m_bits.quickGet(i))
+    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_bits.clearAll();
-}
-
-void UseCounter::updateMeasurements()
-{
-    featureObserverHistogram().count(PageVisits);
-    m_countBits.updateMeasurements();
+    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 (int i = firstCSSProperty; i <= lastUnresolvedCSSProperty; ++i) {
+    for (size_t i = firstCSSProperty; i <= lastUnresolvedCSSProperty; ++i) {
         if (m_CSSFeatureBits.quickGet(i)) {
-            int cssSampleId = mapCSSPropertyIdToCSSSampleIdForHistogram(i);
+            int cssSampleId = mapCSSPropertyIdToCSSSampleIdForHistogram(static_cast<CSSPropertyID>(i));
             cssPropertiesHistogram.count(cssSampleId);
             needsPagesMeasuredUpdate = true;
         }
     }
 
     if (needsPagesMeasuredUpdate)
         cssPropertiesHistogram.count(totalPagesMeasuredCSSSampleId());
 
     m_CSSFeatureBits.clearAll();
 }
 
 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();
 }
 
 void UseCounter::count(const Frame* frame, Feature feature)
 {
     if (!frame)
         return;
     FrameHost* host = frame->host();
     if (!host)
         return;
@@ skipping 15 matching lines @@
     if (!host)
         return false;
     return host->useCounter().hasRecordedMeasurement(feature);
 }
 
 bool UseCounter::isCounted(CSSPropertyID unresolvedProperty)
 {
     return m_CSSFeatureBits.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;
 
     CSSPropertyID unresolvedProperty = unresolvedCSSPropertyID(string);
@@ skipping 37 matching lines @@
 
 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)
 {
-    ASSERT(feature >= firstCSSProperty);
-    ASSERT(feature <= lastUnresolvedCSSProperty);
+    DCHECK(feature >= firstCSSProperty);
+    DCHECK(feature <= 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);
     }
-    m_CSSFeatureBits.quickSet(feature);
 }
 
 void UseCounter::count(Feature feature)
 {
-    ASSERT(Deprecation::deprecationMessage(feature).isEmpty());
+    DCHECK(Deprecation::deprecationMessage(feature).isEmpty());
     recordMeasurement(feature);
 }
 
 UseCounter* UseCounter::getFrom(const Document* document)
 {
     if (document && document->frameHost())
         return &document->frameHost()->useCounter();
     return 0;
 }
 
 UseCounter* UseCounter::getFrom(const CSSStyleSheet* sheet)
 {
     if (sheet)
         return getFrom(sheet->contents());
     return 0;
 }
 
 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;
 }
 
 } // namespace blink