Commit 903df57a authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

Revert "Rationalize call stacks targets in //components/metrics/BUILD.gn"

This reverts commit fd3a7398.

Reason for revert: Likely culprit of https://crbug.com/879513.

Original change's description:
> Rationalize call stacks targets in //components/metrics/BUILD.gn
> 
> Renames targets to better reflect what they provide. Removes the
> :metrics dep from the target for child processes to avoid including
> unnecessary browser-process-only code in those processes. Breaks the
> direct dependency of CallStackProfileBuilder on :metrics by introducing
> a receiver callback abstraction.
> 
> Bug: 878509
> Change-Id: I03a590f31be8c9641c9f375d8dbc0f8bc254b7d4
> Reviewed-on: https://chromium-review.googlesource.com/1195808
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Ilya Sherman <isherman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#587726}

TBR=sky@chromium.org,wittman@chromium.org,isherman@chromium.org

Change-Id: I1ce64c7544b045a513980ff6deb6ddc804dee33f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 878509
Reviewed-on: https://chromium-review.googlesource.com/1199083Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587978}
parent 8600c449
...@@ -1763,7 +1763,7 @@ jumbo_split_static_library("browser") { ...@@ -1763,7 +1763,7 @@ jumbo_split_static_library("browser") {
"//components/language/content/browser", "//components/language/content/browser",
"//components/language/core/browser", "//components/language/core/browser",
"//components/live_tab_count_metrics", "//components/live_tab_count_metrics",
"//components/metrics:call_stack_profile_collector", "//components/metrics:call_stacks",
"//components/metrics:component_metrics", "//components/metrics:component_metrics",
"//components/metrics:gpu", "//components/metrics:gpu",
"//components/metrics:net", "//components/metrics:net",
......
...@@ -134,7 +134,6 @@ ...@@ -134,7 +134,6 @@
#include "components/language/core/common/language_experiments.h" #include "components/language/core/common/language_experiments.h"
#include "components/language_usage_metrics/language_usage_metrics.h" #include "components/language_usage_metrics/language_usage_metrics.h"
#include "components/metrics/call_stack_profile_builder.h" #include "components/metrics/call_stack_profile_builder.h"
#include "components/metrics/call_stack_profile_metrics_provider.h"
#include "components/metrics/call_stack_profile_params.h" #include "components/metrics/call_stack_profile_params.h"
#include "components/metrics/expired_histogram_util.h" #include "components/metrics/expired_histogram_util.h"
#include "components/metrics/metrics_reporting_default_state.h" #include "components/metrics/metrics_reporting_default_state.h"
...@@ -759,14 +758,6 @@ bool WaitUntilMachineLevelUserCloudPolicyEnrollmentFinished( ...@@ -759,14 +758,6 @@ bool WaitUntilMachineLevelUserCloudPolicyEnrollmentFinished(
#endif #endif
} }
// Sets up the ThreadProfiler for the browser process, runs it, and returns the
// profiler.
std::unique_ptr<ThreadProfiler> CreateAndStartBrowserMainThreadProfiler() {
ThreadProfiler::SetBrowserProcessReceiverCallback(base::BindRepeating(
&metrics::CallStackProfileMetricsProvider::ReceiveCompletedProfile));
return ThreadProfiler::CreateAndStartOnMainThread();
}
} // namespace } // namespace
namespace chrome_browser { namespace chrome_browser {
...@@ -794,7 +785,7 @@ ChromeBrowserMainParts::ChromeBrowserMainParts( ...@@ -794,7 +785,7 @@ ChromeBrowserMainParts::ChromeBrowserMainParts(
: parameters_(parameters), : parameters_(parameters),
parsed_command_line_(parameters.command_line), parsed_command_line_(parameters.command_line),
result_code_(service_manager::RESULT_CODE_NORMAL_EXIT), result_code_(service_manager::RESULT_CODE_NORMAL_EXIT),
ui_thread_profiler_(CreateAndStartBrowserMainThreadProfiler()), ui_thread_profiler_(ThreadProfiler::CreateAndStartOnMainThread()),
should_call_pre_main_loop_start_startup_on_variations_service_( should_call_pre_main_loop_start_startup_on_variations_service_(
!parameters.ui_task), !parameters.ui_task),
profile_(NULL), profile_(NULL),
......
...@@ -290,7 +290,7 @@ static_library("common") { ...@@ -290,7 +290,7 @@ static_library("common") {
] ]
deps = [ deps = [
"//components/metrics:call_stack_profile_builder", "//components/metrics:call_stack_profile",
] ]
if (enable_plugins) { if (enable_plugins) {
......
...@@ -146,13 +146,6 @@ void ThreadProfiler::StartOnChildThread(CallStackProfileParams::Thread thread) { ...@@ -146,13 +146,6 @@ void ThreadProfiler::StartOnChildThread(CallStackProfileParams::Thread thread) {
g_child_thread_profiler_sequence_local_storage.Get().Set(std::move(profiler)); g_child_thread_profiler_sequence_local_storage.Get().Set(std::move(profiler));
} }
// static
void ThreadProfiler::SetBrowserProcessReceiverCallback(
const base::RepeatingCallback<void(base::TimeTicks,
metrics::SampledProfile)>& callback) {
metrics::CallStackProfileBuilder::SetBrowserProcessReceiverCallback(callback);
}
// static // static
void ThreadProfiler::SetServiceManagerConnectorForChildProcess( void ThreadProfiler::SetServiceManagerConnectorForChildProcess(
service_manager::Connector* connector) { service_manager::Connector* connector) {
......
...@@ -72,13 +72,6 @@ class ThreadProfiler { ...@@ -72,13 +72,6 @@ class ThreadProfiler {
static void StartOnChildThread( static void StartOnChildThread(
metrics::CallStackProfileParams::Thread thread); metrics::CallStackProfileParams::Thread thread);
// Sets the callback to use for reporting browser process profiles. This
// indirection is required to avoid a dependency on unnecessary metrics code
// in child processes.
static void SetBrowserProcessReceiverCallback(
const base::RepeatingCallback<void(base::TimeTicks,
metrics::SampledProfile)>& callback);
// This function must be called within child processes to supply the Service // This function must be called within child processes to supply the Service
// Manager's connector, to bind the interface through which a profile is sent // Manager's connector, to bind the interface through which a profile is sent
// back to the browser process. // back to the browser process.
......
...@@ -6,7 +6,7 @@ import("//media/media_options.gni") ...@@ -6,7 +6,7 @@ import("//media/media_options.gni")
static_library("gpu") { static_library("gpu") {
deps = [ deps = [
"//components/metrics:child_call_stack_profile_builder", "//components/metrics:child_call_stacks",
"//content/public/common", "//content/public/common",
"//content/public/gpu", "//content/public/gpu",
] ]
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/command_line.h"
#include "base/metrics/histogram_samples.h" #include "base/metrics/histogram_samples.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
...@@ -19,7 +18,6 @@ ...@@ -19,7 +18,6 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/renderer/searchbox/search_bouncer.h" #include "chrome/renderer/searchbox/search_bouncer.h"
#include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h" #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/webplugininfo.h" #include "content/public/common/webplugininfo.h"
#include "extensions/buildflags/buildflags.h" #include "extensions/buildflags/buildflags.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -82,15 +80,8 @@ void AddContentTypeHandler(content::WebPluginInfo* info, ...@@ -82,15 +80,8 @@ void AddContentTypeHandler(content::WebPluginInfo* info,
} // namespace } // namespace
class ChromeContentRendererClientTest : public testing::Test { typedef testing::Test ChromeContentRendererClientTest;
public:
void SetUp() override {
// Ensure that this looks like the renderer process based on the command
// line.
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kProcessType, switches::kRendererProcess);
}
};
#if BUILDFLAG(ENABLE_EXTENSIONS) #if BUILDFLAG(ENABLE_EXTENSIONS)
scoped_refptr<const extensions::Extension> CreateTestExtension( scoped_refptr<const extensions::Extension> CreateTestExtension(
......
...@@ -105,6 +105,7 @@ static_library("metrics") { ...@@ -105,6 +105,7 @@ static_library("metrics") {
] ]
public_deps = [ public_deps = [
":child_call_stacks",
"//third_party/metrics_proto", "//third_party/metrics_proto",
] ]
...@@ -256,66 +257,43 @@ static_library("single_sample_metrics") { ...@@ -256,66 +257,43 @@ static_library("single_sample_metrics") {
] ]
} }
source_set("call_stack_profile_params") { source_set("call_stack_profile") {
public = [
"call_stack_profile_encoding.h",
"call_stack_profile_params.h",
]
sources = [ sources = [
"call_stack_profile_builder.cc",
"call_stack_profile_builder.h",
"call_stack_profile_encoding.cc", "call_stack_profile_encoding.cc",
"call_stack_profile_encoding.h",
"call_stack_profile_params.h",
] ]
deps = [ deps = [
":metrics",
"//base:base", "//base:base",
"//third_party/metrics_proto", "//third_party/metrics_proto",
] ]
} }
# Dependency for child processes that use the CallStackProfileBuilder. source_set("call_stacks") {
source_set("child_call_stack_profile_builder") {
public = [
"call_stack_profile_builder.h",
"child_call_stack_profile_collector.h",
]
sources = [ sources = [
"call_stack_profile_builder.cc", "call_stack_profile_collector.cc",
"child_call_stack_profile_collector.cc", "call_stack_profile_collector.h",
]
public_deps = [
":call_stack_profile_params",
]
deps = [
"//base",
"//components/metrics/public/interfaces:call_stack_mojo_bindings",
"//third_party/metrics_proto",
] ]
# This target must not depend on :metrics because that code is intended solely
# for use in the browser process.
assert_no_deps = [ ":metrics" ]
}
# Dependency for browser process use of the CallStackProfileBuilder.
source_set("call_stack_profile_builder") {
deps = [ deps = [
":call_stack_profile",
":metrics", ":metrics",
] "//components/metrics/public/interfaces:call_stack_mojo_bindings",
public_deps = [
":child_call_stack_profile_builder",
] ]
} }
# The browser process mojo service for collecting profiles from child source_set("child_call_stacks") {
# processes.
source_set("call_stack_profile_collector") {
sources = [ sources = [
"call_stack_profile_collector.cc", "child_call_stack_profile_collector.cc",
"call_stack_profile_collector.h", "child_call_stack_profile_collector.h",
] ]
deps = [ deps = [
":call_stack_profile_params",
":metrics",
"//components/metrics/public/interfaces:call_stack_mojo_bindings", "//components/metrics/public/interfaces:call_stack_mojo_bindings",
"//services/service_manager/public/cpp",
"//third_party/metrics_proto",
] ]
} }
...@@ -389,7 +367,8 @@ source_set("unit_tests") { ...@@ -389,7 +367,8 @@ source_set("unit_tests") {
] ]
deps = [ deps = [
":call_stack_profile_builder", ":call_stack_profile",
":child_call_stacks",
":component_metrics", ":component_metrics",
":metrics", ":metrics",
":net", ":net",
......
...@@ -12,9 +12,9 @@ ...@@ -12,9 +12,9 @@
#include "base/lazy_instance.h" #include "base/lazy_instance.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/metrics_hashes.h" #include "base/metrics/metrics_hashes.h"
#include "base/no_destructor.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "components/metrics/call_stack_profile_encoding.h" #include "components/metrics/call_stack_profile_encoding.h"
#include "components/metrics/call_stack_profile_metrics_provider.h"
namespace metrics { namespace metrics {
...@@ -24,14 +24,6 @@ namespace { ...@@ -24,14 +24,6 @@ namespace {
base::LazyInstance<ChildCallStackProfileCollector>::Leaky base::LazyInstance<ChildCallStackProfileCollector>::Leaky
g_child_call_stack_profile_collector = LAZY_INSTANCE_INITIALIZER; g_child_call_stack_profile_collector = LAZY_INSTANCE_INITIALIZER;
base::RepeatingCallback<void(base::TimeTicks, SampledProfile)>&
GetBrowserProcessReceiverCallbackInstance() {
static base::NoDestructor<
base::RepeatingCallback<void(base::TimeTicks, SampledProfile)>>
instance;
return *instance;
}
// Identifies an unknown module. // Identifies an unknown module.
const size_t kUnknownModuleIndex = static_cast<size_t>(-1); const size_t kUnknownModuleIndex = static_cast<size_t>(-1);
...@@ -274,18 +266,11 @@ void CallStackProfileBuilder::OnProfileCompleted( ...@@ -274,18 +266,11 @@ void CallStackProfileBuilder::OnProfileCompleted(
std::move(completed_callback_).Run(); std::move(completed_callback_).Run();
} }
// static
void CallStackProfileBuilder::SetBrowserProcessReceiverCallback(
const base::RepeatingCallback<void(base::TimeTicks, SampledProfile)>&
callback) {
GetBrowserProcessReceiverCallbackInstance() = callback;
}
void CallStackProfileBuilder::PassProfilesToMetricsProvider( void CallStackProfileBuilder::PassProfilesToMetricsProvider(
SampledProfile sampled_profile) { SampledProfile sampled_profile) {
if (profile_params_.process == CallStackProfileParams::BROWSER_PROCESS) { if (profile_params_.process == CallStackProfileParams::BROWSER_PROCESS) {
GetBrowserProcessReceiverCallbackInstance().Run(profile_start_time_, CallStackProfileMetricsProvider::ReceiveCompletedProfile(
std::move(sampled_profile)); profile_start_time_, std::move(sampled_profile));
} else { } else {
g_child_call_stack_profile_collector.Get() g_child_call_stack_profile_collector.Get()
.ChildCallStackProfileCollector::Collect(profile_start_time_, .ChildCallStackProfileCollector::Collect(profile_start_time_,
......
...@@ -18,8 +18,6 @@ ...@@ -18,8 +18,6 @@
namespace metrics { namespace metrics {
class SampledProfile;
// An instance of the class is meant to be passed to base::StackSamplingProfiler // An instance of the class is meant to be passed to base::StackSamplingProfiler
// to collect profiles. The profiles collected are uploaded via the metrics log. // to collect profiles. The profiles collected are uploaded via the metrics log.
class CallStackProfileBuilder class CallStackProfileBuilder
...@@ -98,13 +96,6 @@ class CallStackProfileBuilder ...@@ -98,13 +96,6 @@ class CallStackProfileBuilder
void OnProfileCompleted(base::TimeDelta profile_duration, void OnProfileCompleted(base::TimeDelta profile_duration,
base::TimeDelta sampling_period) override; base::TimeDelta sampling_period) override;
// Sets the callback to use for reporting browser process profiles. This
// indirection is required to avoid a dependency on unnecessary metrics code
// in child processes.
static void SetBrowserProcessReceiverCallback(
const base::RepeatingCallback<void(base::TimeTicks, SampledProfile)>&
callback);
// Sets the current system state that is recorded with each captured stack // Sets the current system state that is recorded with each captured stack
// frame. This is thread-safe so can be called from anywhere. The parameter // frame. This is thread-safe so can be called from anywhere. The parameter
// value should be from an enumeration of the appropriate type with values // value should be from an enumeration of the appropriate type with values
......
...@@ -25,7 +25,7 @@ static_library("lib") { ...@@ -25,7 +25,7 @@ static_library("lib") {
deps = [ deps = [
"//base", "//base",
"//components/metrics:call_stack_profile_builder", "//components/metrics:call_stack_profile",
"//components/metrics:metrics", "//components/metrics:metrics",
"//components/prefs", "//components/prefs",
"//components/version_info", "//components/version_info",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment