Skip to content

Commit

Permalink
Revert of Eliminate kMojoChannelToken usage for render processes (pat…
Browse files Browse the repository at this point in the history
…chset chromium#5 id:80001 of https://codereview.chromium.org/2201183005/ )

Reason for revert:
Compile error in Webkit Mac Builder.

child_thread_impl.cc:272:47: error: non-virtual member function marked 'override' hides virtual member function
                 shell::Connector* connector) override {

Original issue's description:
> Eliminate kMojoChannelToken usage for render processes
>
> Changes RenderProcessHostImpl to connect its IPC Channel
> by requesting a nominal IPC::mojom::ChannelBootstrap
> interface from its shell::Connection to the render process.
>
> Changes ChildThreadImpl to support binding such requests
> by fusing them to its local IPC Channel if the (now deprecated)
> kMojoChannelToken was not provided.
>
> BUG=623396
> R=ben@chromium.org
>
> Committed: https://crrev.com/7a7f8f7e6977500d09270ff9d9b3817dda5e5b8a
> Cr-Commit-Position: refs/heads/master@{#410656}

TBR=ben@chromium.org,tsepez@chromium.org,jschuh@chromium.org,rockot@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=623396

Review-Url: https://codereview.chromium.org/2222373002
Cr-Commit-Position: refs/heads/master@{#410658}
  • Loading branch information
samuelhuang authored and Commit bot committed Aug 9, 2016
1 parent f4f9dd5 commit 703abc1
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 192 deletions.
2 changes: 0 additions & 2 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ source_set("browser") {
"//gpu",
"//gpu/command_buffer/client:gles2_implementation",
"//gpu/command_buffer/client:gles2_interface",
"//ipc",
"//ipc:mojom",
"//media",
"//media/capture",
"//media/gpu/ipc/client",
Expand Down
185 changes: 71 additions & 114 deletions content/browser/mojo/mojo_shell_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,44 @@

#include "content/browser/mojo/mojo_shell_context.h"

#include <memory>
#include <string>
#include <unordered_map>
#include <utility>

#include "base/bind.h"
#include "base/command_line.h"
#include "base/lazy_instance.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/path_service.h"
#include "base/single_thread_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "content/browser/gpu/gpu_process_host.h"
#include "content/common/mojo/constants.h"
#include "content/common/mojo/mojo_shell_connection_impl.h"
#include "content/common/process_control.mojom.h"
#include "content/grit/content_resources.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/content_browser_client.h"
#include "content/public/browser/utility_process_host.h"
#include "content/public/browser/utility_process_host_client.h"
#include "content/public/common/content_client.h"
#include "content/public/common/mojo_shell_connection.h"
#include "content/public/common/content_switches.h"
#include "mojo/edk/embedder/embedder.h"
#include "mojo/public/cpp/bindings/interface_request.h"
#include "mojo/public/cpp/bindings/string.h"
#include "services/catalog/catalog.h"
#include "services/catalog/manifest_provider.h"
#include "services/catalog/store.h"
#include "services/shell/connect_params.h"
#include "services/shell/native_runner.h"
#include "services/shell/public/cpp/connector.h"
#include "services/shell/public/cpp/identity.h"
#include "services/shell/public/cpp/service.h"
#include "services/shell/public/interfaces/connector.mojom.h"
#include "services/shell/public/interfaces/service.mojom.h"
#include "services/shell/public/interfaces/service_factory.mojom.h"
#include "services/shell/runner/common/client_util.h"
#include "services/shell/runner/host/in_process_native_runner.h"
#include "services/shell/service_manager.h"
#include "services/user/public/cpp/constants.h"

namespace content {
Expand All @@ -46,9 +53,10 @@ base::LazyInstance<std::unique_ptr<shell::Connector>>::Leaky

void DestroyConnectorOnIOThread() { g_io_thread_connector.Get().reset(); }

void StartUtilityProcessOnIOThread(mojom::ProcessControlRequest request,
const base::string16& process_name,
bool use_sandbox) {
void StartUtilityProcessOnIOThread(
mojo::InterfaceRequest<mojom::ProcessControl> request,
const base::string16& process_name,
bool use_sandbox) {
UtilityProcessHost* process_host =
UtilityProcessHost::Create(nullptr, nullptr);
process_host->SetName(process_name);
Expand Down Expand Up @@ -114,33 +122,40 @@ void LaunchAppInGpuProcess(const std::string& app_name,

#endif // ENABLE_MOJO_MEDIA_IN_GPU_PROCESS

} // namespace

// A ManifestProvider which resolves application names to builtin manifest
// resources for the catalog service to consume.
class BuiltinManifestProvider : public catalog::ManifestProvider {
class MojoShellContext::BuiltinManifestProvider
: public catalog::ManifestProvider {
public:
BuiltinManifestProvider() {}
~BuiltinManifestProvider() override {}

void AddManifestResource(const std::string& name, int resource_id) {
auto result = manifest_resources_.insert(
std::make_pair(name, resource_id));
DCHECK(result.second);
}

void AddManifests(std::unique_ptr<
ContentBrowserClient::MojoApplicationManifestMap> manifests) {
DCHECK(!manifests_);
manifests_ = std::move(manifests);
}

void AddManifestResource(const std::string& name, int resource_id) {
std::string contents = GetContentClient()->GetDataResource(
resource_id, ui::ScaleFactor::SCALE_FACTOR_NONE).as_string();
DCHECK(!contents.empty());

DCHECK(manifests_);
auto result = manifests_->insert(std::make_pair(name, contents));
DCHECK(result.second) << "Duplicate manifest entry: " << name;
}

private:
// catalog::ManifestProvider:
bool GetApplicationManifest(const base::StringPiece& name,
std::string* manifest_contents) override {
auto it = manifest_resources_.find(name.as_string());
if (it != manifest_resources_.end()) {
*manifest_contents =
GetContentClient()
->GetDataResource(it->second, ui::ScaleFactor::SCALE_FACTOR_NONE)
.as_string();
DCHECK(!manifest_contents->empty());
return true;
}
auto manifest_it = manifests_->find(name.as_string());
if (manifest_it != manifests_->end()) {
*manifest_contents = manifest_it->second;
Expand All @@ -150,110 +165,53 @@ class BuiltinManifestProvider : public catalog::ManifestProvider {
return false;
}

std::unordered_map<std::string, int> manifest_resources_;
std::unique_ptr<ContentBrowserClient::MojoApplicationManifestMap> manifests_;

DISALLOW_COPY_AND_ASSIGN(BuiltinManifestProvider);
};

} // namespace

// State which lives on the IO thread and drives the ServiceManager.
class MojoShellContext::InProcessServiceManagerContext
: public base::RefCountedThreadSafe<InProcessServiceManagerContext> {
public:
InProcessServiceManagerContext() {}

shell::mojom::ServiceRequest Start(
std::unique_ptr<BuiltinManifestProvider> manifest_provider) {
shell::mojom::ServicePtr embedder_service_proxy;
shell::mojom::ServiceRequest embedder_service_request =
mojo::GetProxy(&embedder_service_proxy);
shell::mojom::ServicePtrInfo embedder_service_proxy_info =
embedder_service_proxy.PassInterface();
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)->PostTask(
FROM_HERE,
base::Bind(&InProcessServiceManagerContext::StartOnIOThread, this,
base::Passed(&manifest_provider),
base::Passed(&embedder_service_proxy_info)));
return embedder_service_request;
}

void ShutDown() {
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)->PostTask(
FROM_HERE,
base::Bind(&InProcessServiceManagerContext::ShutDownOnIOThread, this));
}

private:
friend class base::RefCountedThreadSafe<InProcessServiceManagerContext>;

~InProcessServiceManagerContext() {}

void StartOnIOThread(
std::unique_ptr<BuiltinManifestProvider> manifest_provider,
shell::mojom::ServicePtrInfo embedder_service_proxy_info) {
manifest_provider_ = std::move(manifest_provider);

scoped_refptr<base::SingleThreadTaskRunner> file_task_runner =
BrowserThread::GetTaskRunnerForThread(BrowserThread::FILE);
std::unique_ptr<shell::NativeRunnerFactory> native_runner_factory(
new shell::InProcessNativeRunnerFactory(
BrowserThread::GetBlockingPool()));
catalog_.reset(new catalog::Catalog(
file_task_runner.get(), nullptr, manifest_provider_.get()));
service_manager_.reset(new shell::ServiceManager(
std::move(native_runner_factory), catalog_->TakeService()));

shell::mojom::ServiceRequest request =
service_manager_->StartEmbedderService(kBrowserMojoApplicationName);
mojo::FuseInterface(
std::move(request), std::move(embedder_service_proxy_info));
}

void ShutDownOnIOThread() {
service_manager_.reset();
catalog_.reset();
manifest_provider_.reset();
}

std::unique_ptr<BuiltinManifestProvider> manifest_provider_;
std::unique_ptr<catalog::Catalog> catalog_;
std::unique_ptr<shell::ServiceManager> service_manager_;

DISALLOW_COPY_AND_ASSIGN(InProcessServiceManagerContext);
};


MojoShellContext::MojoShellContext() {
scoped_refptr<base::SingleThreadTaskRunner> file_task_runner =
BrowserThread::GetTaskRunnerForThread(BrowserThread::FILE);
std::unique_ptr<shell::NativeRunnerFactory> native_runner_factory(
new shell::InProcessNativeRunnerFactory(
BrowserThread::GetBlockingPool()));

// Allow the embedder to register additional Mojo application manifests
// beyond the default ones below.
std::unique_ptr<ContentBrowserClient::MojoApplicationManifestMap> manifests(
new ContentBrowserClient::MojoApplicationManifestMap);
GetContentClient()->browser()->RegisterMojoApplicationManifests(
manifests.get());

manifest_provider_.reset(new BuiltinManifestProvider);
manifest_provider_->AddManifests(std::move(manifests));
manifest_provider_->AddManifestResource(kBrowserMojoApplicationName,
IDR_MOJO_CONTENT_BROWSER_MANIFEST);
manifest_provider_->AddManifestResource(kGpuMojoApplicationName,
IDR_MOJO_CONTENT_GPU_MANIFEST);
manifest_provider_->AddManifestResource(kRendererMojoApplicationName,
IDR_MOJO_CONTENT_RENDERER_MANIFEST);
manifest_provider_->AddManifestResource(kUtilityMojoApplicationName,
IDR_MOJO_CONTENT_UTILITY_MANIFEST);
manifest_provider_->AddManifestResource("mojo:catalog",
IDR_MOJO_CATALOG_MANIFEST);
manifest_provider_->AddManifestResource(user_service::kUserServiceName,
IDR_MOJO_PROFILE_MANIFEST);

catalog_.reset(new catalog::Catalog(file_task_runner.get(), nullptr,
manifest_provider_.get()));

shell::mojom::ServiceRequest request;
if (shell::ShellIsRemote()) {
mojo::edk::SetParentPipeHandleFromCommandLine();
request = shell::GetServiceRequestFromCommandLine();
} else {
// Allow the embedder to register additional Mojo application manifests
// beyond the default ones below.
std::unique_ptr<ContentBrowserClient::MojoApplicationManifestMap> manifests(
new ContentBrowserClient::MojoApplicationManifestMap);
GetContentClient()->browser()->RegisterMojoApplicationManifests(
manifests.get());
std::unique_ptr<BuiltinManifestProvider> manifest_provider =
base::MakeUnique<BuiltinManifestProvider>();
manifest_provider->AddManifests(std::move(manifests));
manifest_provider->AddManifestResource(kBrowserMojoApplicationName,
IDR_MOJO_CONTENT_BROWSER_MANIFEST);
manifest_provider->AddManifestResource(kGpuMojoApplicationName,
IDR_MOJO_CONTENT_GPU_MANIFEST);
manifest_provider->AddManifestResource(kRendererMojoApplicationName,
IDR_MOJO_CONTENT_RENDERER_MANIFEST);
manifest_provider->AddManifestResource(kUtilityMojoApplicationName,
IDR_MOJO_CONTENT_UTILITY_MANIFEST);
manifest_provider->AddManifestResource("mojo:catalog",
IDR_MOJO_CATALOG_MANIFEST);
manifest_provider->AddManifestResource(user_service::kUserServiceName,
IDR_MOJO_PROFILE_MANIFEST);

in_process_context_ = new InProcessServiceManagerContext;
request = in_process_context_->Start(std::move(manifest_provider));
service_manager_.reset(new shell::ServiceManager(
std::move(native_runner_factory), catalog_->TakeService()));
request =
service_manager_->StartEmbedderService(kBrowserMojoApplicationName);
}
MojoShellConnection::SetForProcess(MojoShellConnection::Create(
std::move(request),
Expand Down Expand Up @@ -306,8 +264,7 @@ MojoShellContext::~MojoShellContext() {
MojoShellConnection::DestroyForProcess();
BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
base::Bind(&DestroyConnectorOnIOThread));
if (in_process_context_)
in_process_context_->ShutDown();
catalog_.reset();
}

// static
Expand Down
22 changes: 14 additions & 8 deletions content/browser/mojo/mojo_shell_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,23 @@
#ifndef CONTENT_BROWSER_MOJO_MOJO_SHELL_CONTEXT_H_
#define CONTENT_BROWSER_MOJO_MOJO_SHELL_CONTEXT_H_

#include <map>
#include <memory>

#include "base/callback_forward.h"
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "content/common/content_export.h"
#include "services/shell/service_manager.h"

namespace shell {
class Connector;
namespace catalog {
class Catalog;
}

namespace content {

// MojoShellContext manages the browser's connection to the ServiceManager,
// hosting a new in-process ServiceManager if the browser was not launched from
// an external one.
// MojoShellContext hosts the browser's ApplicationManager, coordinating
// app registration and interconnection.
class CONTENT_EXPORT MojoShellContext {
public:
MojoShellContext();
Expand All @@ -27,9 +31,11 @@ class CONTENT_EXPORT MojoShellContext {
static shell::Connector* GetConnectorForIOThread();

private:
class InProcessServiceManagerContext;
class BuiltinManifestProvider;

scoped_refptr<InProcessServiceManagerContext> in_process_context_;
std::unique_ptr<BuiltinManifestProvider> manifest_provider_;
std::unique_ptr<catalog::Catalog> catalog_;
std::unique_ptr<shell::ServiceManager> service_manager_;

DISALLOW_COPY_AND_ASSIGN(MojoShellContext);
};
Expand Down
17 changes: 11 additions & 6 deletions content/browser/renderer_host/render_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@
#include "gpu/command_buffer/service/gpu_switches.h"
#include "ipc/attachment_broker.h"
#include "ipc/attachment_broker_privileged.h"
#include "ipc/ipc.mojom.h"
#include "ipc/ipc_channel.h"
#include "ipc/ipc_channel_mojo.h"
#include "ipc/ipc_logging.h"
Expand Down Expand Up @@ -821,7 +820,8 @@ bool RenderProcessHostImpl::Init() {
g_renderer_main_thread_factory(InProcessChildThreadParams(
channel_id,
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
std::string(), mojo_child_connection_->service_token())));
mojo_channel_token_,
mojo_child_connection_->service_token())));

base::Thread::Options options;
#if defined(OS_WIN) && !defined(OS_MACOSX)
Expand Down Expand Up @@ -881,11 +881,12 @@ std::unique_ptr<IPC::ChannelProxy> RenderProcessHostImpl::CreateChannelProxy(
const std::string& channel_id) {
scoped_refptr<base::SingleThreadTaskRunner> runner =
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO);
IPC::mojom::ChannelBootstrapPtr bootstrap;
GetRemoteInterfaces()->GetInterface(&bootstrap);
mojo_channel_token_ = mojo::edk::GenerateRandomToken();
mojo::ScopedMessagePipeHandle handle =
mojo::edk::CreateParentMessagePipe(mojo_channel_token_, child_token_);

std::unique_ptr<IPC::ChannelFactory> channel_factory =
IPC::ChannelMojo::CreateServerFactory(
bootstrap.PassInterface().PassHandle(), runner);
IPC::ChannelMojo::CreateServerFactory(std::move(handle), runner);

// Do NOT expand ifdef or run time condition checks here! Synchronous
// IPCs from browser process are banned. It is only narrowly allowed
Expand Down Expand Up @@ -1420,6 +1421,10 @@ void RenderProcessHostImpl::AppendRendererCommandLine(

AppendCompositorCommandLineFlags(command_line);

if (!mojo_channel_token_.empty()) {
command_line->AppendSwitchASCII(switches::kMojoChannelToken,
mojo_channel_token_);
}
command_line->AppendSwitchASCII(switches::kMojoApplicationChannelToken,
mojo_child_connection_->service_token());
}
Expand Down
3 changes: 3 additions & 0 deletions content/browser/renderer_host/render_process_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,9 @@ class CONTENT_EXPORT RenderProcessHostImpl
base::WaitableEvent never_signaled_;
#endif

std::string mojo_channel_token_;
mojo::ScopedMessagePipeHandle in_process_renderer_handle_;

base::WeakPtrFactory<RenderProcessHostImpl> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(RenderProcessHostImpl);
Expand Down
2 changes: 0 additions & 2 deletions content/child/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ target(link_target_type, "child") {
"//content/public/common:common_sources",
"//crypto:platform",
"//gpu/command_buffer/client",
"//ipc",
"//ipc:mojom",
"//media",
"//mojo/common",
"//mojo/edk/system",
Expand Down
Loading

0 comments on commit 703abc1

Please sign in to comment.