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

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

Issue 2796283005: Adding UseCounter specific for extensions (Closed)
Patch Set: Codereview: added tester ExpectTotalCountExcept to simplify UseCounter unit tests Created 3 years, 7 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
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..1dd55e610b2047706deafd7a29b6a6c6e8b35ded 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 =
Alexei Svitkine (slow) 2017/05/10 15:17:30 Nit: Use const char kSVGFeaturesHistogramName[] =
Nico 2017/05/10 15:38:20 Oh, good catch. To elaborate a bit on this: const
lunalu1 2017/05/10 19:39:39 Makes sense! Thanks for the insight =) Although i
"Blink.UseCounter.SVGImage.Features";
@@ -27,6 +29,20 @@ 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 = {
Nico 2017/05/09 19:36:46 1) This is just a vector, why is it a reference? 2
lunalu1 2017/05/10 19:39:39 I see. Thanks!
+ kAnimatedCSSHistogramName, kCSSHistogramName,
+ kExtensionFeaturesHistogramName, kFeaturesHistogramName,
+ kSVGAnimatedCSSHistogramName, kSVGCSSHistogramName,
+ kSVGFeaturesHistogramName};
+
+int GetPageVisitsBucketforHistogram(std::string histogram_name) {
Alexei Svitkine (slow) 2017/05/10 15:17:30 const std::string&
+ if (histogram_name.find("CSS") == std::string::npos)
+ return blink::UseCounter::kPageVisits;
+ // For CSS histograms, the page visits bucket should be 1.
+ return 1;
+}
}
Alexei Svitkine (slow) 2017/05/10 15:17:30 Nit: " // namespace" Also maybe add inner empty
lunalu1 2017/05/10 19:39:39 Sorry my bad.
namespace blink {
@@ -34,17 +50,17 @@ namespace blink {
template <typename T>
void HistogramBasicTest(const std::string& histogram,
const std::string& legacy_histogram,
- const std::vector<std::string>& unaffected_histograms,
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) {
+ const std::string& url) {
HistogramTester histogram_tester;
+ int page_visit_bucket = GetPageVisitsBucketforHistogram(histogram);
+
// Test recording a single (arbitrary) counter
EXPECT_FALSE(counted(item));
count(item);
@@ -109,18 +125,21 @@ 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);
+ for (const auto unaffected_histogram : all_histograms) {
Alexei Svitkine (slow) 2017/05/10 15:17:30 const auto& or const std::string& Otherwise, this
lunalu1 2017/05/10 19:39:39 Done.
+ if (unaffected_histogram == histogram)
+ continue;
+
+ histogram_tester.ExpectTotalCountExcept(
+ unaffected_histogram,
+ GetPageVisitsBucketforHistogram(unaffected_histogram), 0);
}
}
TEST(UseCounterTest, RecordingFeatures) {
UseCounter use_counter;
HistogramBasicTest<UseCounter::Feature>(
- kFeaturesHistogramName, kLegacyFeaturesHistogramName,
- {kSVGFeaturesHistogramName, kSVGCSSHistogramName,
- kSVGAnimatedCSSHistogramName},
- UseCounter::kFetch, UseCounter::kFetchBodyStream,
+ kFeaturesHistogramName, kLegacyFeaturesHistogramName, UseCounter::kFetch,
+ UseCounter::kFetchBodyStream,
[&](UseCounter::Feature feature) -> bool {
return use_counter.HasRecordedMeasurement(feature);
},
@@ -129,16 +148,14 @@ TEST(UseCounterTest, RecordingFeatures) {
},
[](UseCounter::Feature feature) -> int { return feature; },
[&](KURL kurl) { use_counter.DidCommitLoad(kurl); },
- "https://dummysite.com/", UseCounter::kPageVisits);
+ "https://dummysite.com/");
}
TEST(UseCounterTest, RecordingCSSProperties) {
UseCounter use_counter;
HistogramBasicTest<CSSPropertyID>(
- kCSSHistogramName, kLegacyCSSHistogramName,
- {kSVGFeaturesHistogramName, kSVGCSSHistogramName,
- kSVGAnimatedCSSHistogramName},
- CSSPropertyFont, CSSPropertyZoom,
+ kCSSHistogramName, kLegacyCSSHistogramName, CSSPropertyFont,
+ CSSPropertyZoom,
[&](CSSPropertyID property) -> bool {
return use_counter.IsCounted(property);
},
@@ -149,15 +166,13 @@ TEST(UseCounterTest, RecordingCSSProperties) {
return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
},
[&](KURL kurl) { use_counter.DidCommitLoad(kurl); },
- "https://dummysite.com/", 1 /* page visit bucket */);
+ "https://dummysite.com/");
}
TEST(UseCounterTest, RecordingAnimatedCSSProperties) {
UseCounter use_counter;
HistogramBasicTest<CSSPropertyID>(
- kAnimatedCSSHistogramName, "",
- {kSVGCSSHistogramName, kSVGAnimatedCSSHistogramName}, CSSPropertyOpacity,
- CSSPropertyVariable,
+ kAnimatedCSSHistogramName, "", CSSPropertyOpacity, CSSPropertyVariable,
[&](CSSPropertyID property) -> bool {
return use_counter.IsCountedAnimatedCSS(property);
},
@@ -166,14 +181,29 @@ TEST(UseCounterTest, RecordingAnimatedCSSProperties) {
return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
},
[&](KURL kurl) { use_counter.DidCommitLoad(kurl); },
- "https://dummysite.com/", 1 /* page visit bucket */);
+ "https://dummysite.com/");
+}
+
+TEST(UseCounterTest, RecordingExtensions) {
+ UseCounter use_counter(UseCounter::kExtensionContext);
+ HistogramBasicTest<UseCounter::Feature>(
+ kExtensionFeaturesHistogramName, kLegacyFeaturesHistogramName,
+ 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");
}
TEST(UseCounterTest, SVGImageContextFeatures) {
UseCounter use_counter(UseCounter::kSVGImageContext);
HistogramBasicTest<UseCounter::Feature>(
kSVGFeaturesHistogramName, kLegacyFeaturesHistogramName,
- {kFeaturesHistogramName, kCSSHistogramName, kAnimatedCSSHistogramName},
UseCounter::kSVGSMILAdditiveAnimation,
UseCounter::kSVGSMILAnimationElementTiming,
[&](UseCounter::Feature feature) -> bool {
@@ -183,17 +213,15 @@ TEST(UseCounterTest, SVGImageContextFeatures) {
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);
+ [&](KURL kurl) { use_counter.DidCommitLoad(kurl); }, "about:blank");
+ // In practice SVGs always appear to be loaded with an about:blank URL
Nico 2017/05/09 19:36:46 The comment looks strange here. Maybe do // In pr
lunalu1 2017/05/10 19:39:39 Done. I did the same for HttpsUrl and ExtensionUrl
}
TEST(UseCounterTest, SVGImageContextCSSProperties) {
UseCounter use_counter(UseCounter::kSVGImageContext);
HistogramBasicTest<CSSPropertyID>(
- kSVGCSSHistogramName, kLegacyCSSHistogramName,
- {kFeaturesHistogramName, kCSSHistogramName, kAnimatedCSSHistogramName},
- CSSPropertyFont, CSSPropertyZoom,
+ kSVGCSSHistogramName, kLegacyCSSHistogramName, CSSPropertyFont,
+ CSSPropertyZoom,
[&](CSSPropertyID property) -> bool {
return use_counter.IsCounted(property);
},
@@ -203,17 +231,15 @@ TEST(UseCounterTest, SVGImageContextCSSProperties) {
[](CSSPropertyID property) -> int {
return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
},
- [&](KURL kurl) { use_counter.DidCommitLoad(kurl); }, "about:blank",
+ [&](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,
+ kSVGAnimatedCSSHistogramName, "", CSSPropertyOpacity, CSSPropertyVariable,
[&](CSSPropertyID property) -> bool {
return use_counter.IsCountedAnimatedCSS(property);
},
@@ -221,9 +247,9 @@ TEST(UseCounterTest, SVGImageContextAnimatedCSSProperties) {
[](CSSPropertyID property) -> int {
return UseCounter::MapCSSPropertyIdToCSSSampleIdForHistogram(property);
},
- [&](KURL kurl) { use_counter.DidCommitLoad(kurl); }, "about:blank",
+ [&](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, InspectorDisablesMeasurement) {

Powered by Google App Engine
This is Rietveld 408576698