Skip to content

Commit

Permalink
Remove blink::HistogramTester and use base::HistogramTester instead
Browse files Browse the repository at this point in the history
Blink has already had direct access to base histogram functions
and macros, so there is no need to have blink::HistogramTester
which had nothing special except that
- it provided only a subset of base::HistogramTester API;
- it didn't have the `location = FROM_HERE` parameter, which made
  it difficult to debug test failures.

Change-Id: I5724e3de73215e926bdade6e08f017c7944d3637
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4844180
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1192811}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Sep 6, 2023
1 parent 8d8fde5 commit 84d058f
Show file tree
Hide file tree
Showing 30 changed files with 70 additions and 168 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
#include "third_party/blink/renderer/core/testing/sim/sim_request.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/url_test_helpers.h"

Expand Down
10 changes: 5 additions & 5 deletions third_party/blink/renderer/core/animation/animation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <tuple>

#include "base/bits.h"
#include "base/test/metrics/histogram_tester.h"
#include "build/build_config.h"
#include "cc/trees/target_property.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -71,7 +72,6 @@
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/heap/thread_state.h"
#include "third_party/blink/renderer/platform/scheduler/public/event_loop.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/paint_test_configurations.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"
Expand Down Expand Up @@ -1430,7 +1430,7 @@ TEST_P(AnimationAnimationTestCompositing, PreCommitRecordsHistograms) {

// Initially the animation in this test has no target, so it is invalid.
{
HistogramTester histogram;
base::HistogramTester histogram;
ASSERT_TRUE(animation->PreCommit(0, nullptr, true));
histogram.ExpectBucketCount(
histogram_name,
Expand All @@ -1440,7 +1440,7 @@ TEST_P(AnimationAnimationTestCompositing, PreCommitRecordsHistograms) {

// Restart the animation with a target and compositing state.
{
HistogramTester histogram;
base::HistogramTester histogram;
ResetWithCompositedAnimation();
histogram.ExpectBucketCount(
histogram_name,
Expand All @@ -1452,7 +1452,7 @@ TEST_P(AnimationAnimationTestCompositing, PreCommitRecordsHistograms) {
animation->setPlaybackRate(0);
animation->NotifyReady(ANIMATION_TIME_DELTA_FROM_SECONDS(100));
{
HistogramTester histogram;
base::HistogramTester histogram;
ASSERT_TRUE(animation->PreCommit(0, nullptr, true));
histogram.ExpectBucketCount(
histogram_name,
Expand All @@ -1479,7 +1479,7 @@ TEST_P(AnimationAnimationTestCompositing, PreCommitRecordsHistograms) {
->SetKeyframes({start_keyframe, end_keyframe});
UpdateAllLifecyclePhasesForTest();
{
HistogramTester histogram;
base::HistogramTester histogram;
ASSERT_TRUE(animation->PreCommit(0, nullptr, true));
histogram.ExpectBucketCount(
histogram_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/heap/thread_state.h"
#include "third_party/blink/renderer/platform/testing/find_cc_layer.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/paint_test_configurations.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "third_party/blink/renderer/core/script/classic_script.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
#include "third_party/blink/renderer/platform/heap/thread_state.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"

namespace blink {

Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/exported/web_view_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <string>

#include "base/functional/callback_helpers.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_mock_time_task_runner.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -149,7 +150,6 @@
#include "third_party/blink/renderer/platform/heap/thread_state.h"
#include "third_party/blink/renderer/platform/keyboard_codes.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/url_loader_mock_factory.h"
Expand Down Expand Up @@ -5963,7 +5963,7 @@ TEST_F(WebViewTest, InputDelayReported) {

test_task_runner_->FastForwardBy(base::Milliseconds(70));

HistogramTester histogram_tester;
base::HistogramTester histogram_tester;
WebKeyboardEvent key_event1(WebInputEvent::Type::kRawKeyDown,
WebInputEvent::kNoModifiers,
WebInputEvent::GetStaticTimeStampForTests());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "third_party/blink/renderer/core/fragment_directive/text_fragment_anchor_metrics.h"

#include "base/test/metrics/histogram_tester.h"
#include "base/test/simple_test_tick_clock.h"
#include "components/ukm/test_ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_builders.h"
Expand All @@ -21,7 +22,6 @@
#include "third_party/blink/renderer/core/testing/sim/sim_test.h"
#include "third_party/blink/renderer/platform/scheduler/public/main_thread_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/scoped_fake_ukm_recorder.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"

Expand All @@ -48,7 +48,7 @@ class TextFragmentAnchorMetricsTest : public TextFragmentAnchorTestBase {
return scoped_fake_ukm_recorder_.recorder();
}

HistogramTester histogram_tester_;
base::HistogramTester histogram_tester_;
ScopedFakeUkmRecorder scoped_fake_ukm_recorder_;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "third_party/blink/renderer/core/frame/reporting_context.h"

#include "base/test/metrics/histogram_tester.h"
#include "base/time/time.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
Expand All @@ -14,7 +15,6 @@
#include "third_party/blink/renderer/core/frame/permissions_policy_violation_report_body.h"
#include "third_party/blink/renderer/core/frame/report.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"

namespace blink {

Expand Down Expand Up @@ -134,7 +134,7 @@ class MockReportingServiceProxy : public mojom::blink::ReportingServiceProxy {
};

TEST_F(ReportingContextTest, CountQueuedReports) {
HistogramTester tester;
base::HistogramTester tester;
auto dummy_page_holder = std::make_unique<DummyPageHolder>();
tester.ExpectTotalCount("Blink.UseCounter.Features.DeprecationReport", 0);
// Checking the feature state with reporting intent should record a potential
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/test/metrics/histogram_tester.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/scheme_registry.h"
#include "third_party/blink/public/mojom/use_counter/metrics/css_property_id.mojom-blink.h"
Expand All @@ -17,7 +18,6 @@
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/url_test_helpers.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"
Expand Down Expand Up @@ -111,7 +111,7 @@ class UseCounterImplTest : public testing::Test {
Document& GetDocument() { return dummy_->GetDocument(); }

std::unique_ptr<DummyPageHolder> dummy_;
HistogramTester histogram_tester_;
base::HistogramTester histogram_tester_;

void UpdateAllLifecyclePhases(Document& document) {
document.View()->UpdateAllLifecyclePhasesForTest();
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/renderer/core/frame/web_frame_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@
#include "third_party/blink/renderer/platform/scheduler/public/event_loop.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread.h"
#include "third_party/blink/renderer/platform/testing/find_cc_layer.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/testing_platform_support.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
#include "third_party/blink/renderer/core/testing/sim/sim_request.h"
#include "third_party/blink/renderer/core/testing/sim/sim_test.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"

namespace blink {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <tuple>

#include "base/test/metrics/histogram_tester.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
Expand All @@ -26,7 +27,6 @@
#include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_request.h"
#include "third_party/blink/renderer/platform/network/network_state_notifier.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"

Expand Down Expand Up @@ -89,7 +89,7 @@ TEST_F(LazyLoadImagesSimTest, ImgSrcset) {
}

TEST_F(LazyLoadImagesSimTest, LazyLoadedImageSizeHistograms) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;
SimRequest lazy_a_resource("https://example.com/lazy_a.png", "image/png");
SimRequest eager_resource("https://example.com/eager.png", "image/png");
SimRequest lazy_b_resource("https://example.com/lazy_b.png", "image/png");
Expand Down Expand Up @@ -650,7 +650,7 @@ TEST_F(LazyLoadImagesTest, ImageInsideLazyLoadedFrame) {
}

TEST_F(LazyLoadImagesTest, AboveTheFoldImageLoadedBeforeVisible) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;

SimRequest main_resource("https://example.com/", "text/html");
SimSubresourceRequest image_resource("https://example.com/image.png",
Expand Down Expand Up @@ -694,7 +694,7 @@ TEST_F(LazyLoadImagesTest, AboveTheFoldImageLoadedBeforeVisible) {

// A fully above-the-fold cached image should not report below-the-fold metrics.
TEST_F(LazyLoadImagesTest, AboveTheFoldCachedImageMetrics) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;
SimRequest main_resource("https://example.com/", "text/html");
SimSubresourceRequest image_resource("https://example.com/image.png",
"image/png");
Expand Down Expand Up @@ -748,7 +748,7 @@ TEST_F(LazyLoadImagesTest, AboveTheFoldCachedImageMetrics) {
// An image that loads immediately due to being cached should not report
// Blink.VisibleBeforeLoaded.LazyLoadImages metrics.
TEST_F(LazyLoadImagesTest, CachedImageVisibleBeforeLoadedMetrics) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;
SimRequest main_resource("https://a.com/", "text/html");
SimSubresourceRequest image_resource("https://a.com/image.png", "image/png");
LoadURL("https://a.com/");
Expand Down Expand Up @@ -798,7 +798,7 @@ TEST_F(LazyLoadImagesTest, CachedImageVisibleBeforeLoadedMetrics) {
}

TEST_F(LazyLoadImagesTest, AboveTheFoldImageVisibleBeforeLoaded) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;

SimRequest main_resource("https://example.com/", "text/html");
SimSubresourceRequest image_resource("https://example.com/image.png",
Expand Down Expand Up @@ -844,7 +844,7 @@ TEST_F(LazyLoadImagesTest, AboveTheFoldImageVisibleBeforeLoaded) {
}

TEST_F(LazyLoadImagesTest, BelowTheFoldImageLoadedBeforeVisible) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;

SimRequest main_resource("https://example.com/", "text/html");
LoadURL("https://example.com/");
Expand Down Expand Up @@ -905,7 +905,7 @@ TEST_F(LazyLoadImagesTest, BelowTheFoldImageLoadedBeforeVisible) {
}

TEST_F(LazyLoadImagesTest, BelowTheFoldImageVisibleBeforeLoaded) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;

SimRequest main_resource("https://example.com/", "text/html");
LoadURL("https://example.com/");
Expand Down Expand Up @@ -965,7 +965,7 @@ TEST_F(LazyLoadImagesTest, BelowTheFoldImageVisibleBeforeLoaded) {

// LazyLoadImages metrics should not be recorded for non-lazy image loads.
TEST_F(LazyLoadImagesTest, NonLazyIgnoredForLazyLoadImagesMetrics) {
HistogramTester histogram_tester;
base::HistogramTester histogram_tester;

SimRequest main_resource("https://aa.com/", "text/html");
SimSubresourceRequest above_resource("https://aa.com/above.png", "image/png");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include "third_party/blink/renderer/platform/loader/fetch/url_loader/url_loader_client.h"
#include "third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.h"
#include "third_party/blink/renderer/platform/storage/blink_storage_key.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/url_loader_mock_factory.h"
#include "third_party/blink/renderer/platform/testing/url_test_helpers.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "third_party/blink/renderer/core/loader/resource/font_resource.h"

#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/features.h"
Expand All @@ -24,7 +25,6 @@
#include "third_party/blink/renderer/platform/loader/testing/mock_resource_client.h"
#include "third_party/blink/renderer/platform/loader/testing/test_loader_factory.h"
#include "third_party/blink/renderer/platform/loader/testing/test_resource_fetcher_properties.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/mock_context_lifecycle_notifier.h"
#include "third_party/blink/renderer/platform/testing/testing_platform_support_with_mock_scheduler.h"
#include "third_party/blink/renderer/platform/testing/url_loader_mock_factory.h"
Expand Down Expand Up @@ -137,7 +137,7 @@ TEST_F(FontResourceTest,

// Tests if the RevalidationPolicy UMA works properly for fonts.
TEST_F(FontResourceTest, RevalidationPolicyMetrics) {
blink::HistogramTester histogram_tester;
base::HistogramTester histogram_tester;
auto* properties = MakeGarbageCollected<TestResourceFetcherProperties>();
MockFetchContext* context = MakeGarbageCollected<MockFetchContext>();
auto* fetcher = MakeGarbageCollected<ResourceFetcher>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include <memory>
#include "base/task/single_thread_task_runner.h"
#include "base/test/metrics/histogram_tester.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/loader/referrer_utils.h"
Expand Down Expand Up @@ -68,7 +69,6 @@
#include "third_party/blink/renderer/platform/network/http_names.h"
#include "third_party/blink/renderer/platform/scheduler/test/fake_frame_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/test/fake_task_runner.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/mock_context_lifecycle_notifier.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/scoped_mocked_url.h"
Expand Down Expand Up @@ -1182,7 +1182,7 @@ TEST_F(ImageResourceCounterTest, InstanceCounters_UserAgent) {
}

TEST_F(ImageResourceCounterTest, RevalidationPolicyMetrics) {
blink::HistogramTester histogram_tester;
base::HistogramTester histogram_tester;
auto* fetcher = CreateFetcher();

KURL test_url("http://127.0.0.1:8000/img.png");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/testing_platform_support.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
#include "third_party/blink/renderer/platform/testing/url_loader_mock_factory.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "base/containers/span.h"
#include "base/ranges/algorithm.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/time/time.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand All @@ -29,7 +30,6 @@
#include "third_party/blink/renderer/core/permissions_policy/permissions_policy_parser.h"
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
#include "third_party/blink/renderer/core/testing/null_execution_context.h"
#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
Expand Down Expand Up @@ -125,8 +125,7 @@ class OriginTrialContextTest : public testing::Test {
protected:
OriginTrialContextTest()
: token_validator_(new MockTokenValidator()),
execution_context_(MakeGarbageCollected<NullExecutionContext>()),
histogram_tester_(new HistogramTester()) {
execution_context_(MakeGarbageCollected<NullExecutionContext>()) {
execution_context_->GetOriginTrialContext()
->SetTrialTokenValidatorForTesting(
std::unique_ptr<MockTokenValidator>(token_validator_));
Expand Down Expand Up @@ -197,18 +196,18 @@ class OriginTrialContextTest : public testing::Test {
}

void ExpectStatusCount(OriginTrialTokenStatus status, int count) {
histogram_tester_->ExpectBucketCount(kResultHistogram,
static_cast<int>(status), count);
histogram_tester_.ExpectBucketCount(kResultHistogram,
static_cast<int>(status), count);
}

void ExpectStatusTotalMetric(int total) {
histogram_tester_->ExpectTotalCount(kResultHistogram, total);
histogram_tester_.ExpectTotalCount(kResultHistogram, total);
}

protected:
MockTokenValidator* token_validator_;
Persistent<NullExecutionContext> execution_context_;
std::unique_ptr<HistogramTester> histogram_tester_;
base::HistogramTester histogram_tester_;
};

// Check that validation status gets logged to the histogram
Expand Down
Loading

0 comments on commit 84d058f

Please sign in to comment.