UseCounter: Introduce UseCounter::Observer for layout tests

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 1125 matching lines @@
     return false;
 
   DCHECK(feature != OBSOLETE_PageDestruction &&
          feature !=
              PageVisits);  // PageDestruction is reserved as a scaling factor.
   DCHECK(feature < NumberOfFeatures);
 
   return m_featuresRecorded.quickGet(feature);
 }
 
+DEFINE_TRACE(UseCounter) {
+  visitor->trace(m_observers);
+}
+
 void UseCounter::didCommitLoad(KURL url) {
   m_legacyCounter.updateMeasurements();
 
   // Reset state from previous load.
   m_disableReporting = false;
 
   // Use the protocol of the document being loaded into the main frame to
   // decide whether this page is interesting from a metrics perspective.
   // Note that SVGImage cases always have an about:blank URL
   if (m_context == DefaultContext &&
@@ skipping 10 matching lines @@
 }
 
 void UseCounter::count(const Frame* frame, Feature feature) {
   if (!frame)
     return;
   FrameHost* host = frame->host();
   if (!host)
     return;
 
   host->useCounter().count(feature);
+  HeapHashSet<Member<Observer>> toBeRemoved;

[Rick Byers, 2017/02/11 15:01:51]

IIRC, this code path isn't triggered on the deprecation case.  If that's
correct, why is the test passing?  Is it just relying on the race and could
still be flaky?  I think you want to move this logic down into
recordMeasurement.

In fact, do you really want the semantics to be that the observer is invoked for
EVERY call to 'count'?  That can be a LOT of calls .  Perhaps the semantics
should be that it's invoked when the histogram is updated - i.e. the first time
a feature is used on a page.  That's much rarer and so less performance
sensitive.  So maybe you want to put this logic in a helper function that's
called at the same time as we generate the blink.feature_usage TRACE?

Regardless, this choice of observation semantics should be documented in a
comment in UseCounter.h.

[nhiroki, 2017/02/13 04:35:07]

Good point. This is because both countFeature() and countDeprecation() on
SharedWorkerGlobalScope get to countFeature() on Document.
Yes, it's preferable to call it only one time per feature. Moved the call into
recordMeasurement.
Added.

+  for (auto observer : host->useCounter().m_observers) {
+    if (observer->onCountFeature(feature))
+      toBeRemoved.insert(observer);
+  }
+  host->useCounter().m_observers.removeAll(toBeRemoved);
 }
 
 void UseCounter::count(const Document& document, Feature feature) {
   count(document.frame(), feature);
 }
 
 bool UseCounter::isCounted(Document& document, Feature feature) {
   Frame* frame = document.frame();
   if (!frame)
     return false;
   FrameHost* host = frame->host();
   if (!host)
     return false;
   return host->useCounter().hasRecordedMeasurement(feature);
 }
 
 bool UseCounter::isCounted(CSSPropertyID unresolvedProperty) {
   return m_CSSRecorded.quickGet(unresolvedProperty);
 }
 
+void UseCounter::addObserver(Document& document, Observer* observer) {
+  Frame* frame = document.frame();
+  if (!frame)
+    return;
+  FrameHost* host = frame->host();
+  if (!host)
+    return;
+  host->useCounter().m_observers.insert(observer);
+}
+
 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);
   if (unresolvedProperty == CSSPropertyInvalid)
@@ skipping 129 matching lines @@
     }
   }
 
   if (needsPagesMeasuredUpdate)
     cssPropertiesHistogram.count(totalPagesMeasuredCSSSampleId());
 
   m_CSSBits.clearAll();
 }
 
 }  // namespace blink