Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(299)

Unified Diff: third_party/WebKit/Source/core/frame/UseCounterTest.cpp

Issue 2796283005: Adding UseCounter specific for extensions (Closed)
Patch Set: Codereview: nit + refactored UseCounterTests unaffected_histograms => all_histograms / affected_his… Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « third_party/WebKit/Source/core/frame/UseCounter.cpp ('k') | tools/metrics/histograms/histograms.xml » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 w
lunalu1 2017/05/04 15:33:56 That was my initial thought. Although everything f
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_histog
lunalu1 2017/05/04 15:33:56 Yes. I thought about that. But I decided explicitl
+ 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 he
Rick Byers 2017/05/01 21:19:20 Why does this pass without kLegacyFeaturesHistogra
lunalu1 2017/05/04 15:33:56 That makes sense. I assume "getPageVisitsBucketfor
lunalu1 2017/05/04 15:33:56 I didn't include the legacy histograms in all_hist
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 d
lunalu1 2017/05/04 15:33:56 You are so right. My bad! I just did an exclusive
+ 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);
},
« no previous file with comments | « third_party/WebKit/Source/core/frame/UseCounter.cpp ('k') | tools/metrics/histograms/histograms.xml » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698