Skip to content

Commit

Permalink
Reland "Clean up out-of-process heap_profiling support"
Browse files Browse the repository at this point in the history
This is a reland of cc5b68b

Reland fixes linkage of Fuchsia cast build. The original CL broke
it by moving the CastContentUtilityClient constructor out-of-line
into cast_content_utility_client.cc, which was not being linked into
Fuchsia builds. Now that it no longer depends on the heap_profiling
service, it's fine to build everywhere.

Original change's description:
> Clean up out-of-process heap_profiling support
>
> The heap_profiling service is now only run in-process, so this removes a
> bunch of dead utility process code supporting its use out-of-process.
>
> Bug: None
> Change-Id: If4836adfbb6c551d239927972013a74ef7822b38
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1621346
> Commit-Queue: Ken Rockot <rockot@google.com>
> Reviewed-by: Luke Halliwell <halliwell@chromium.org>
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#663191}

TBR=boliu@chromium.org
TBR=halliwell@chromium.org
TBR=sky@chromium.org

Bug: None
Change-Id: I4821cd50e926bb1764d861dfe3e4e621185876d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629170
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#663284}
  • Loading branch information
krockot authored and Commit Bot committed May 24, 2019
1 parent 3b5f257 commit 86091a4
Show file tree
Hide file tree
Showing 13 changed files with 4 additions and 175 deletions.
1 change: 0 additions & 1 deletion android_webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,6 @@ source_set("common") {
":generate_components_strings",
":native_jni",
"//android_webview/browser/gfx:gfx",
"//android_webview/utility",
"//base",
"//base/third_party/dynamic_annotations:dynamic_annotations",
"//components/about_ui",
Expand Down
6 changes: 0 additions & 6 deletions android_webview/lib/aw_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "android_webview/common/crash_reporter/crash_keys.h"
#include "android_webview/gpu/aw_content_gpu_client.h"
#include "android_webview/renderer/aw_content_renderer_client.h"
#include "android_webview/utility/aw_content_utility_client.h"
#include "base/android/apk_assets.h"
#include "base/android/build_info.h"
#include "base/bind.h"
Expand Down Expand Up @@ -339,9 +338,4 @@ content::ContentRendererClient* AwMainDelegate::CreateContentRendererClient() {
return content_renderer_client_.get();
}

content::ContentUtilityClient* AwMainDelegate::CreateContentUtilityClient() {
content_utility_client_.reset(new AwContentUtilityClient());
return content_utility_client_.get();
}

} // namespace android_webview
3 changes: 0 additions & 3 deletions android_webview/lib/aw_main_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ namespace android_webview {
class AwContentBrowserClient;
class AwContentGpuClient;
class AwContentRendererClient;
class AwContentUtilityClient;

// Android WebView implementation of ContentMainDelegate. The methods in
// this class runs per process, (browser and renderer) so when making changes
Expand All @@ -50,7 +49,6 @@ class AwMainDelegate : public content::ContentMainDelegate {
content::ContentBrowserClient* CreateContentBrowserClient() override;
content::ContentGpuClient* CreateContentGpuClient() override;
content::ContentRendererClient* CreateContentRendererClient() override;
content::ContentUtilityClient* CreateContentUtilityClient() override;

// Responsible for creating a feature list from the seed. This object must
// exist for the lifetime of the process as it contains the FieldTrialList
Expand All @@ -61,7 +59,6 @@ class AwMainDelegate : public content::ContentMainDelegate {
std::unique_ptr<AwContentBrowserClient> content_browser_client_;
std::unique_ptr<AwContentGpuClient> content_gpu_client_;
std::unique_ptr<AwContentRendererClient> content_renderer_client_;
std::unique_ptr<AwContentUtilityClient> content_utility_client_;
std::unique_ptr<safe_browsing::SafeBrowsingApiHandler>
safe_browsing_api_handler_;

Expand Down
17 changes: 0 additions & 17 deletions android_webview/utility/BUILD.gn

This file was deleted.

4 changes: 0 additions & 4 deletions android_webview/utility/DEPS

This file was deleted.

31 changes: 0 additions & 31 deletions android_webview/utility/aw_content_utility_client.cc

This file was deleted.

27 changes: 0 additions & 27 deletions android_webview/utility/aw_content_utility_client.h

This file was deleted.

2 changes: 0 additions & 2 deletions chrome/utility/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ static_library("utility") {
"//components/mirroring/mojom:constants",
"//components/mirroring/service:mirroring_service",
"//components/search_engines",
"//components/services/heap_profiling",
"//components/services/heap_profiling/public/cpp",
"//components/services/patch:lib",
"//components/services/unzip:lib",
"//components/strings",
Expand Down
2 changes: 0 additions & 2 deletions chrome/utility/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ include_rules = [
"+components/mirroring/mojom",
"+components/mirroring/service",
"+components/payments/content/utility",
"+components/services/heap_profiling/heap_profiling_service.h",
"+components/services/heap_profiling/public",
"+components/services/patch",
"+components/services/pdf_compositor/public/cpp",
"+components/services/pdf_compositor/public/interfaces",
Expand Down
16 changes: 1 addition & 15 deletions chrome/utility/chrome_content_utility_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
#include "components/mirroring/mojom/constants.mojom.h"
#include "components/mirroring/service/features.h"
#include "components/mirroring/service/mirroring_service.h"
#include "components/services/heap_profiling/heap_profiling_service.h"
#include "components/services/heap_profiling/public/mojom/constants.mojom.h"
#include "components/services/patch/patch_service.h"
#include "components/services/patch/public/interfaces/constants.mojom.h"
#include "components/services/unzip/public/interfaces/constants.mojom.h"
Expand Down Expand Up @@ -131,19 +129,12 @@ void RunServiceAsyncThenTerminateProcess(
base::BindOnce([] { content::UtilityThread::Get()->ReleaseProcess(); }));
}

std::unique_ptr<service_manager::Service> CreateHeapProfilingService(
service_manager::mojom::ServiceRequest request) {
return std::make_unique<heap_profiling::HeapProfilingService>(
std::move(request));
}

#if !defined(OS_ANDROID)
std::unique_ptr<service_manager::Service> CreateProxyResolverService(
service_manager::mojom::ServiceRequest request) {
return std::make_unique<proxy_resolver::ProxyResolverService>(
std::move(request));
}
#endif

using ServiceFactory =
base::OnceCallback<std::unique_ptr<service_manager::Service>()>;
Expand All @@ -161,6 +152,7 @@ void RunServiceOnIOThread(ServiceFactory factory) {
},
std::move(factory), std::move(terminate_process)));
}
#endif // !defined(OS_ANDROID)

} // namespace

Expand Down Expand Up @@ -232,12 +224,6 @@ bool ChromeContentUtilityClient::HandleServiceRequest(
return false;
}

if (service_name == heap_profiling::mojom::kServiceName) {
RunServiceOnIOThread(
base::BindOnce(&CreateHeapProfilingService, std::move(request)));
return true;
}

#if !defined(OS_ANDROID)
if (service_name == proxy_resolver::mojom::kProxyResolverServiceName) {
RunServiceOnIOThread(
Expand Down
12 changes: 1 addition & 11 deletions chromecast/utility/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import("//chromecast/chromecast.gni")

cast_source_set("utility") {
sources = [
"cast_content_utility_client.cc",
"cast_content_utility_client.h",
]

Expand All @@ -17,15 +18,4 @@ cast_source_set("utility") {
if (chromecast_branding == "public") {
sources += [ "cast_content_utility_client_simple.cc" ]
}

if (!is_fuchsia) {
sources += [ "cast_content_utility_client.cc" ]

deps += [
"//chromecast:chromecast_buildflags",
"//components/services/heap_profiling",
"//components/services/heap_profiling/public/mojom",
"//services/service_manager/public/cpp",
]
}
}
47 changes: 1 addition & 46 deletions chromecast/utility/cast_content_utility_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,55 +4,10 @@

#include "chromecast/utility/cast_content_utility_client.h"

#include <string>
#include <utility>

#include "base/callback_forward.h"
#include "components/services/heap_profiling/heap_profiling_service.h"
#include "components/services/heap_profiling/public/mojom/constants.mojom.h"
#include "content/public/utility/utility_thread.h"

namespace {

std::unique_ptr<service_manager::Service> CreateHeapProfilingService(
service_manager::mojom::ServiceRequest request) {
return std::make_unique<heap_profiling::HeapProfilingService>(
std::move(request));
}

using ServiceFactory =
base::OnceCallback<std::unique_ptr<service_manager::Service>()>;
void RunServiceOnIOThread(ServiceFactory factory) {
base::OnceClosure terminate_process = base::BindOnce(
base::IgnoreResult(&base::SequencedTaskRunner::PostTask),
base::SequencedTaskRunnerHandle::Get(), FROM_HERE,
base::BindOnce([] { content::UtilityThread::Get()->ReleaseProcess(); }));
content::ChildThread::Get()->GetIOTaskRunner()->PostTask(
FROM_HERE,
base::BindOnce(
[](ServiceFactory factory, base::OnceClosure terminate_process) {
service_manager::Service::RunAsyncUntilTermination(
std::move(factory).Run(), std::move(terminate_process));
},
std::move(factory), std::move(terminate_process)));
}

} // namespace

namespace chromecast {
namespace shell {

bool CastContentUtilityClient::HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request) {
if (service_name == heap_profiling::mojom::kServiceName) {
RunServiceOnIOThread(
base::BindOnce(&CreateHeapProfilingService, std::move(request)));
return true;
}

return false;
}
CastContentUtilityClient::CastContentUtilityClient() = default;

} // namespace shell
} // namespace chromecast
11 changes: 1 addition & 10 deletions chromecast/utility/cast_content_utility_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
#define CHROMECAST_UTILITY_CAST_CONTENT_UTILITY_CLIENT_H_

#include <memory>
#include <string>

#include "base/macros.h"
#include "build/build_config.h"
#include "content/public/utility/content_utility_client.h"

namespace chromecast {
Expand All @@ -19,14 +17,7 @@ class CastContentUtilityClient : public content::ContentUtilityClient {
public:
static std::unique_ptr<CastContentUtilityClient> Create();

CastContentUtilityClient() {}

#if !defined(OS_FUCHSIA)
// content::ContentUtilityClient implementation:
bool HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request) override;
#endif // !defined(OS_FUCHSIA)
CastContentUtilityClient();

private:
DISALLOW_COPY_AND_ASSIGN(CastContentUtilityClient);
Expand Down

0 comments on commit 86091a4

Please sign in to comment.