Adding UseCounter specific for extensions

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

 // Copyright 2015 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/frame/Deprecation.h"
 #include "core/frame/UseCounter.h"
 #include "core/page/Page.h"
 #include "core/testing/DummyPageHolder.h"
 #include "platform/testing/HistogramTester.h"
 #include "platform/testing/URLTestHelpers.h"
 #include "platform/weborigin/KURL.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 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";
 const char* const kSVGCSSHistogramName =
     "Blink.UseCounter.SVGImage.CSSProperties";
 const char* const kSVGAnimatedCSSHistogramName =
     "Blink.UseCounter.SVGImage.AnimatedCSSProperties";
 
 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 {
 
 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,
                         std::function<void(T)> count,
                         std::function<int(T)> histogram_map,
                         std::function<void(KURL)> did_commit_load,
                         const std::string& url,
                         int page_visit_bucket) {
   HistogramTester histogram_tester;
 
@@ skipping 54 matching lines @@
   did_commit_load(URLTestHelpers::ToKURL(url));
   if (!legacy_histogram.empty()) {
     histogram_tester.ExpectBucketCount(legacy_histogram, histogram_map(item),
                                        2);
     histogram_tester.ExpectBucketCount(legacy_histogram,
                                        histogram_map(second_item), 1);
     histogram_tester.ExpectBucketCount(legacy_histogram, page_visit_bucket, 2);
     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);
       },
       [&](UseCounter::Feature feature) {
         use_counter.RecordMeasurement(feature);
       },
       [](UseCounter::Feature feature) -> int { return feature; },
       [&](KURL kurl) { use_counter.DidCommitLoad(kurl); },
       "https://dummysite.com/", UseCounter::kPageVisits);
 }
 
 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);
       },
       [&](CSSPropertyID property) {
         use_counter.Count(kHTMLStandardMode, property);
       },
       [](CSSPropertyID property) -> int {
         return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
       },
       [&](KURL kurl) { use_counter.DidCommitLoad(kurl); },
       "https://dummysite.com/", 1 /* page visit bucket */);
 }
 
 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);
       },
       [&](CSSPropertyID property) { use_counter.CountAnimatedCSS(property); },
       [](CSSPropertyID property) -> int {
         return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
       },
       [&](KURL kurl) { use_counter.DidCommitLoad(kurl); },
       "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 {
         return use_counter.HasRecordedMeasurement(feature);
       },
       [&](UseCounter::Feature feature) {
         use_counter.RecordMeasurement(feature);
       },
       [](UseCounter::Feature feature) -> int { return feature; },
       [&](KURL kurl) { use_counter.DidCommitLoad(kurl); }, "about:blank",
       // In practice SVGs always appear to be loaded with an about:blank URL
       UseCounter::kPageVisits);
 }
 
 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);
       },
       [&](CSSPropertyID property) {
         use_counter.Count(kHTMLStandardMode, property);
       },
       [](CSSPropertyID property) -> int {
         return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
       },
       [&](KURL kurl) { use_counter.DidCommitLoad(kurl); }, "about:blank",
       // In practice SVGs always appear to be loaded with an about:blank URL
       1 /* page visit bucket */);
 }
 
 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);
       },
       [&](CSSPropertyID property) { use_counter.CountAnimatedCSS(property); },
       [](CSSPropertyID property) -> int {
         return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
       },
       [&](KURL kurl) { use_counter.DidCommitLoad(kurl); }, "about:blank",
       // In practice SVGs always appear to be loaded with an about:blank URL
       1 /* page visit bucket */);
@@ skipping 193 matching lines @@
 
   deprecation_.UnmuteForInspector();
   Deprecation::WarnOnDeprecatedProperties(GetFrame(), property);
   // TODO: use the actually deprecated property to get a deprecation message.
   EXPECT_FALSE(deprecation_.IsSuppressed(property));
   Deprecation::CountDeprecation(GetFrame(), feature);
   EXPECT_TRUE(use_counter_.HasRecordedMeasurement(feature));
 }
 
 }  // namespace blink