Adding UseCounter specific for extensions

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

Index: third_party/WebKit/Source/core/frame/UseCounterTest.cpp
diff --git a/third_party/WebKit/Source/core/frame/UseCounterTest.cpp b/third_party/WebKit/Source/core/frame/UseCounterTest.cpp
index f8c491e322df036683e25c11a0283017b024ee92..dc9c85a4cc0a5aaeaa6a42f1e2afde8a85e53ae0 100644
--- a/third_party/WebKit/Source/core/frame/UseCounterTest.cpp
+++ b/third_party/WebKit/Source/core/frame/UseCounterTest.cpp
@@ -16,6 +16,8 @@ const char* const kFeaturesHistogramName = "Blink.UseCounter.Features";
 const char* const kCSSHistogramName = "Blink.UseCounter.CSSProperties";
 const char* const kAnimatedCSSHistogramName =
     "Blink.UseCounter.AnimatedCSSProperties";
+const char* const kExtensionFeaturesHistogramName =
+    "Blink.UseCounter.Extensions.Features";
 
 const char* const kSVGFeaturesHistogramName =
     "Blink.UseCounter.SVGImage.Features";
@@ -27,6 +29,13 @@ const char* const kSVGAnimatedCSSHistogramName =
 const char* const kLegacyFeaturesHistogramName = "WebCore.FeatureObserver";
 const char* const kLegacyCSSHistogramName =
     "WebCore.FeatureObserver.CSSProperties";
+
+// Sorted list of all histograms to be tested.
+const std::vector<std::string>& all_histograms = {
+    kAnimatedCSSHistogramName,       kCSSHistogramName,
+    kExtensionFeaturesHistogramName, kFeaturesHistogramName,
+    kSVGAnimatedCSSHistogramName,    kSVGCSSHistogramName,
+    kSVGFeaturesHistogramName};
 }
 
 namespace blink {
@@ -34,7 +43,7 @@ namespace blink {
 template <typename T>
 void HistogramBasicTest(const std::string& histogram,
                         const std::string& legacy_histogram,
-                        const std::vector<std::string>& unaffected_histograms,
+                        const std::vector<std::string>& affected_histograms,

[Rick Byers, 2017/05/01 21:19:20]

I was thinking that the only affected histograms would be the two above, and so
you wouldn't need to list out the others at all... But I guess that was naive. 
Some ideas below.

[lunalu1, 2017/05/04 15:33:56]

That was my initial thought. Although everything failed. So I tried to do the
set difference thing.

                         T item,
                         T second_item,
                         std::function<bool(T)> counted,
@@ -109,17 +118,20 @@ void HistogramBasicTest(const std::string& histogram,
     histogram_tester.ExpectTotalCount(legacy_histogram, 5);
   }
 
-  for (size_t i = 0; i < unaffected_histograms.size(); ++i) {
-    histogram_tester.ExpectTotalCount(unaffected_histograms[i], 0);
-  }
+  std::vector<std::string> unaffected_histograms(all_histograms.size() -
+                                                 affected_histograms.size());
+  std::set_difference(all_histograms.begin(), all_histograms.end(),

[Rick Byers, 2017/05/01 21:19:20]

can you also remove "histogram" and "legacy_histogram" from the unaffected set
so that they don't need to be explicitly listed as "affected"?

[lunalu1, 2017/05/04 15:33:56]

Yes. I thought about that. But I decided explicitly listing it would be more
clear of what we were trying to do. But I guess that was unnecessary. 

+                      affected_histograms.begin(), affected_histograms.end(),
+                      unaffected_histograms.begin());
+  for (const auto unaffected_histogram : unaffected_histograms)
+    histogram_tester.ExpectTotalCount(unaffected_histogram, 0);
 }
 
 TEST(UseCounterTest, RecordingFeatures) {
   UseCounter use_counter;
   HistogramBasicTest<UseCounter::Feature>(
       kFeaturesHistogramName, kLegacyFeaturesHistogramName,
-      {kSVGFeaturesHistogramName, kSVGCSSHistogramName,
-       kSVGAnimatedCSSHistogramName},
+      {kAnimatedCSSHistogramName, kCSSHistogramName, kFeaturesHistogramName},

[Rick Byers, 2017/05/01 21:19:20]

I assume you need to specify the CSS histograms here because just their page
visits bucket gets bumped by the 'didCommitLoad', right?  That's a shame.  It
would be nice to validate that they're otherwise unmodified.

If that's the only case, then perhaps we could take a list of histograms which
should have only a "page visits" bucket?  To do that we'd have to know which
bucket is the page visits one, but it should be easy to replace the final
argument to the function with a single "getPageVisitsBucketforHistogram"
function which can return the bucket number given any histogram name (basically
just return 1 if 'CSS' is in the name, and kPageVisits otherwise).  WDYT?

[Rick Byers, 2017/05/01 21:19:20]

Why does this pass without kLegacyFeaturesHistogramName being listed explicitly?
 It's affected too, right?

[lunalu1, 2017/05/04 15:33:56]

That makes sense. I assume "getPageVisitsBucketforHistogram" would be only used
for testing? Also, so far, only used for CSS testing? Do we
expect more things like CSS would happen in the future? Cause 
otherwise it just seems like we are using one trick for one particular 
test. But I don't know.

[lunalu1, 2017/05/04 15:33:56]

I didn't include the legacy histograms in all_histograms. I guess I should have.

       UseCounter::kFetch, UseCounter::kFetchBodyStream,
       [&](UseCounter::Feature feature) -> bool {
         return use_counter.HasRecordedMeasurement(feature);
@@ -136,8 +148,7 @@ TEST(UseCounterTest, RecordingCSSProperties) {
   UseCounter use_counter;
   HistogramBasicTest<CSSPropertyID>(
       kCSSHistogramName, kLegacyCSSHistogramName,
-      {kSVGFeaturesHistogramName, kSVGCSSHistogramName,
-       kSVGAnimatedCSSHistogramName},
+      {kAnimatedCSSHistogramName, kCSSHistogramName, kFeaturesHistogramName},
       CSSPropertyFont, CSSPropertyZoom,
       [&](CSSPropertyID property) -> bool {
         return use_counter.IsCounted(property);
@@ -156,8 +167,9 @@ TEST(UseCounterTest, RecordingAnimatedCSSProperties) {
   UseCounter use_counter;
   HistogramBasicTest<CSSPropertyID>(
       kAnimatedCSSHistogramName, "",
-      {kSVGCSSHistogramName, kSVGAnimatedCSSHistogramName}, CSSPropertyOpacity,
-      CSSPropertyVariable,
+      {kAnimatedCSSHistogramName, kCSSHistogramName, kFeaturesHistogramName,
+       kSVGFeaturesHistogramName},

[Rick Byers, 2017/05/01 21:19:20]

Is kSVGFeaturesHistogramName really affected?  I don't think it should be.

[lunalu1, 2017/05/04 15:33:56]

You are so right. My bad! I just did an exclusive set of the
unaffected_histograms set for each test. Apologies for not taking many thoughts
into this.

+      CSSPropertyOpacity, CSSPropertyVariable,
       [&](CSSPropertyID property) -> bool {
         return use_counter.IsCountedAnimatedCSS(property);
       },
@@ -169,11 +181,29 @@ TEST(UseCounterTest, RecordingAnimatedCSSProperties) {
       "https://dummysite.com/", 1 /* page visit bucket */);
 }
 
+TEST(UseCounterTest, RecordingExtensions) {
+  UseCounter use_counter(UseCounter::kExtensionContext);
+  HistogramBasicTest<UseCounter::Feature>(
+      kExtensionFeaturesHistogramName, kLegacyFeaturesHistogramName,
+      {kExtensionFeaturesHistogramName}, UseCounter::kFetch,
+      UseCounter::kFetchBodyStream,
+      [&](UseCounter::Feature feature) -> bool {
+        return use_counter.HasRecordedMeasurement(feature);
+      },
+      [&](UseCounter::Feature feature) {
+        use_counter.RecordMeasurement(feature);
+      },
+      [](UseCounter::Feature feature) -> int { return feature; },
+      [&](KURL kurl) { use_counter.DidCommitLoad(kurl); },
+      "chrome-extension://dummysite", UseCounter::kPageVisits);
+}
+
 TEST(UseCounterTest, SVGImageContextFeatures) {
   UseCounter use_counter(UseCounter::kSVGImageContext);
   HistogramBasicTest<UseCounter::Feature>(
       kSVGFeaturesHistogramName, kLegacyFeaturesHistogramName,
-      {kFeaturesHistogramName, kCSSHistogramName, kAnimatedCSSHistogramName},
+      {kSVGAnimatedCSSHistogramName, kSVGCSSHistogramName,
+       kSVGFeaturesHistogramName},
       UseCounter::kSVGSMILAdditiveAnimation,
       UseCounter::kSVGSMILAnimationElementTiming,
       [&](UseCounter::Feature feature) -> bool {
@@ -192,7 +222,8 @@ TEST(UseCounterTest, SVGImageContextCSSProperties) {
   UseCounter use_counter(UseCounter::kSVGImageContext);
   HistogramBasicTest<CSSPropertyID>(
       kSVGCSSHistogramName, kLegacyCSSHistogramName,
-      {kFeaturesHistogramName, kCSSHistogramName, kAnimatedCSSHistogramName},
+      {kSVGAnimatedCSSHistogramName, kSVGCSSHistogramName,
+       kSVGFeaturesHistogramName},
       CSSPropertyFont, CSSPropertyZoom,
       [&](CSSPropertyID property) -> bool {
         return use_counter.IsCounted(property);
@@ -212,8 +243,9 @@ TEST(UseCounterTest, SVGImageContextAnimatedCSSProperties) {
   UseCounter use_counter(UseCounter::kSVGImageContext);
   HistogramBasicTest<CSSPropertyID>(
       kSVGAnimatedCSSHistogramName, "",
-      {kCSSHistogramName, kAnimatedCSSHistogramName}, CSSPropertyOpacity,
-      CSSPropertyVariable,
+      {kFeaturesHistogramName, kSVGAnimatedCSSHistogramName,
+       kSVGCSSHistogramName, kSVGFeaturesHistogramName},
+      CSSPropertyOpacity, CSSPropertyVariable,
       [&](CSSPropertyID property) -> bool {
         return use_counter.IsCountedAnimatedCSS(property);
       },