Remove utility process batch mode support and its IPCs
The utility_process_host_impl batch mode feature has no users
anymore, nor is it a useful feature in the age of mojo. Ditch
the feature:
- remove batch mode IPC and the message file.
- remove StartBatchMode / EndBatchMode code.
- change IPC unit tests to use AutomationMsgStart
IPC start name (instead of UtilityMsgStart).
- remove UtilityMsgStart IPC message start name.
- utility_process_host_impl.*, document members
and remove old / unused #include files.
- utility_thread{,impl}.h, ditto.
No change in behavior, no new tests.
Bug: 738648
Change-Id: Ifcb65939ba7bf0a52451d2b8cb3a8a254399fcd2
Reviewed-on: https://chromium-review.googlesource.com/558526
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484557}diff --git a/content/browser/utility_process_host_impl.cc b/content/browser/utility_process_host_impl.cc
index 7aa297db..f6c4e30 100644
--- a/content/browser/utility_process_host_impl.cc
+++ b/content/browser/utility_process_host_impl.cc
@@ -11,17 +11,9 @@
#include "base/bind_helpers.h"
#include "base/command_line.h"
#include "base/files/file_path.h"
-#include "base/lazy_instance.h"
-#include "base/macros.h"
#include "base/memory/ptr_util.h"
-#include "base/message_loop/message_loop.h"
-#include "base/process/process_handle.h"
-#include "base/run_loop.h"
#include "base/sequenced_task_runner.h"
#include "base/strings/utf_string_conversions.h"
-#include "base/synchronization/lock.h"
-#include "base/synchronization/waitable_event.h"
-#include "build/build_config.h"
#include "components/network_session_configurator/common/network_switches.h"
#include "content/browser/browser_child_process_host_impl.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
@@ -29,19 +21,16 @@
#include "content/common/child_process_host_impl.h"
#include "content/common/in_process_child_thread_params.h"
#include "content/common/service_manager/child_connection.h"
-#include "content/common/utility_messages.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/content_browser_client.h"
#include "content/public/browser/utility_process_host_client.h"
#include "content/public/common/content_switches.h"
-#include "content/public/common/mojo_channel_switches.h"
#include "content/public/common/process_type.h"
#include "content/public/common/sandbox_type.h"
#include "content/public/common/sandboxed_process_launcher_delegate.h"
#include "content/public/common/service_manager_connection.h"
#include "content/public/common/service_names.mojom.h"
#include "media/base/media_switches.h"
-#include "mojo/edk/embedder/embedder.h"
#include "services/service_manager/public/cpp/interface_provider.h"
#include "ui/base/ui_base_switches.h"
@@ -146,7 +135,6 @@
const scoped_refptr<base::SequencedTaskRunner>& client_task_runner)
: client_(client),
client_task_runner_(client_task_runner),
- is_batch_mode_(false),
no_sandbox_(false),
run_elevated_(false),
#if defined(OS_LINUX)
@@ -163,8 +151,6 @@
UtilityProcessHostImpl::~UtilityProcessHostImpl() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- if (is_batch_mode_)
- EndBatchMode();
}
base::WeakPtr<UtilityProcessHost> UtilityProcessHostImpl::AsWeakPtr() {
@@ -178,19 +164,6 @@
return process_->Send(message);
}
-bool UtilityProcessHostImpl::StartBatchMode() {
- CHECK(!is_batch_mode_);
- is_batch_mode_ = StartProcess();
- Send(new UtilityMsg_BatchMode_Started());
- return is_batch_mode_;
-}
-
-void UtilityProcessHostImpl::EndBatchMode() {
- CHECK(is_batch_mode_);
- is_batch_mode_ = false;
- Send(new UtilityMsg_BatchMode_Finished());
-}
-
void UtilityProcessHostImpl::SetExposedDir(const base::FilePath& dir) {
exposed_dir_ = dir;
}
@@ -211,12 +184,10 @@
}
#if defined(OS_POSIX)
-
void UtilityProcessHostImpl::SetEnv(const base::EnvironmentMap& env) {
env_ = env;
}
-
-#endif // OS_POSIX
+#endif
bool UtilityProcessHostImpl::Start() {
return StartProcess();
@@ -236,11 +207,8 @@
bool UtilityProcessHostImpl::StartProcess() {
if (started_)
return true;
+
started_ = true;
-
- if (is_batch_mode_)
- return true;
-
process_->SetName(name_);
process_->GetHost()->CreateChannelMojo();
diff --git a/content/browser/utility_process_host_impl.h b/content/browser/utility_process_host_impl.h
index 76dc35507..517fb3c 100644
--- a/content/browser/utility_process_host_impl.h
+++ b/content/browser/utility_process_host_impl.h
@@ -7,10 +7,7 @@
#include <memory>
#include <string>
-#include <vector>
-#include "base/compiler_specific.h"
-#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
@@ -19,6 +16,7 @@
#include "content/public/browser/utility_process_host.h"
namespace base {
+class FilePath;
class SequencedTaskRunner;
class Thread;
}
@@ -42,11 +40,9 @@
const scoped_refptr<base::SequencedTaskRunner>& client_task_runner);
~UtilityProcessHostImpl() override;
- // UtilityProcessHost implementation:
+ // UtilityProcessHost:
base::WeakPtr<UtilityProcessHost> AsWeakPtr() override;
bool Send(IPC::Message* message) override;
- bool StartBatchMode() override;
- void EndBatchMode() override;
void SetExposedDir(const base::FilePath& dir) override;
void DisableSandbox() override;
#if defined(OS_WIN)
@@ -64,8 +60,7 @@
void set_child_flags(int flags) { child_flags_ = flags; }
private:
- // Starts a process if necessary. Returns true if it succeeded or a process
- // has already been started via StartBatchMode().
+ // Starts the child process if needed, returns true on success.
bool StartProcess();
// BrowserChildProcessHost:
@@ -81,35 +76,37 @@
base::WeakPtr<UtilityProcessHostImpl> host,
int error_code);
- // A pointer to our client interface, who will be informed of progress.
+ // Pointer to our client interface used for progress notifications.
scoped_refptr<UtilityProcessHostClient> client_;
- scoped_refptr<base::SequencedTaskRunner> client_task_runner_;
- // True when running in batch mode, i.e., StartBatchMode() has been called
- // and the utility process will run until EndBatchMode().
- bool is_batch_mode_;
+ // Task runner used for posting progess notifications to |client_|.
+ scoped_refptr<base::SequencedTaskRunner> client_task_runner_;
+
+ // Directory opened through the child process sandbox if needed.
base::FilePath exposed_dir_;
- // Whether to pass switches::kNoSandbox to the child.
+ // Whether to launch the child process with switches::kNoSandbox.
bool no_sandbox_;
- // Whether to launch the process with elevated privileges.
+ // Whether to launch the child process with elevated privileges.
bool run_elevated_;
- // Flags defined in ChildProcessHost with which to start the process.
+ // ChildProcessHost flags to use when starting the child process.
int child_flags_;
+ // Map of environment variables to values.
base::EnvironmentMap env_;
+ // True if StartProcess() has been called.
bool started_;
- // A user-visible name identifying this process. Used to indentify this
- // process in the task manager.
+ // The process name used to identify the process in task manager.
base::string16 name_;
+ // Child process host implementation.
std::unique_ptr<BrowserChildProcessHostImpl> process_;
- // Used in single-process mode instead of process_.
+ // Used in single-process mode instead of |process_|.
std::unique_ptr<base::Thread> in_process_thread_;
// Used to vend weak pointers, and should always be declared last.
diff --git a/content/common/BUILD.gn b/content/common/BUILD.gn
index c26f875f..f871bb4 100644
--- a/content/common/BUILD.gn
+++ b/content/common/BUILD.gn
@@ -346,7 +346,6 @@
"url_schemes.cc",
"url_schemes.h",
"user_agent.cc",
- "utility_messages.h",
"view_message_enums.h",
"view_messages.h",
"worker_messages.h",
diff --git a/content/common/content_message_generator.h b/content/common/content_message_generator.h
index 2e2108ec..a86a6ac 100644
--- a/content/common/content_message_generator.h
+++ b/content/common/content_message_generator.h
@@ -38,7 +38,6 @@
#include "content/common/service_worker/service_worker_messages.h"
#include "content/common/speech_recognition_messages.h"
#include "content/common/text_input_client_messages.h"
-#include "content/common/utility_messages.h"
#include "content/common/view_messages.h"
#include "content/common/worker_messages.h"
#include "media/media_features.h"
diff --git a/content/common/utility_messages.h b/content/common/utility_messages.h
deleted file mode 100644
index 6a43e5a..0000000
--- a/content/common/utility_messages.h
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Multiply-included message file, so no include guard.
-
-#include "content/common/content_export.h"
-#include "ipc/ipc_message_macros.h"
-
-#undef IPC_MESSAGE_EXPORT
-#define IPC_MESSAGE_EXPORT CONTENT_EXPORT
-#define IPC_MESSAGE_START UtilityMsgStart
-
-//------------------------------------------------------------------------------
-// Utility process messages:
-// These are messages from the browser to the utility process.
-
-// Tells the utility process that it's running in batch mode.
-IPC_MESSAGE_CONTROL0(UtilityMsg_BatchMode_Started)
-
-// Tells the utility process that it can shutdown.
-IPC_MESSAGE_CONTROL0(UtilityMsg_BatchMode_Finished)
diff --git a/content/public/browser/utility_process_host.h b/content/public/browser/utility_process_host.h
index 5a404dbe..fc1b97d 100644
--- a/content/public/browser/utility_process_host.h
+++ b/content/public/browser/utility_process_host.h
@@ -52,13 +52,6 @@
virtual base::WeakPtr<UtilityProcessHost> AsWeakPtr() = 0;
- // Starts utility process in batch mode. Caller must call EndBatchMode()
- // to finish the utility process.
- virtual bool StartBatchMode() = 0;
-
- // Ends the utility process. Must be called after StartBatchMode().
- virtual void EndBatchMode() = 0;
-
// Allows a directory to be opened through the sandbox, in case it's needed by
// the operation.
virtual void SetExposedDir(const base::FilePath& dir) = 0;
diff --git a/content/public/utility/utility_thread.h b/content/public/utility/utility_thread.h
index c76efda..49aa8a6 100644
--- a/content/public/utility/utility_thread.h
+++ b/content/public/utility/utility_thread.h
@@ -18,7 +18,7 @@
UtilityThread();
~UtilityThread() override;
- // Releases the process if we are not (or no longer) in batch mode.
+ // Releases the process. TODO(noel): rename this routine and update callers.
virtual void ReleaseProcessIfNeeded() = 0;
// Initializes blink if it hasn't already been initialized.
diff --git a/content/utility/utility_thread_impl.cc b/content/utility/utility_thread_impl.cc
index ec50aba8..d7059fd 100644
--- a/content/utility/utility_thread_impl.cc
+++ b/content/utility/utility_thread_impl.cc
@@ -4,30 +4,17 @@
#include "content/utility/utility_thread_impl.h"
-#include <stddef.h>
#include <utility>
-#include "base/command_line.h"
-#include "build/build_config.h"
#include "content/child/blink_platform_impl.h"
#include "content/child/child_process.h"
-#include "content/common/child_process_messages.h"
-#include "content/common/utility_messages.h"
-#include "content/public/common/content_switches.h"
#include "content/public/common/service_manager_connection.h"
#include "content/public/common/simple_connection_filter.h"
#include "content/public/utility/content_utility_client.h"
#include "content/utility/utility_blink_platform_impl.h"
#include "content/utility/utility_service_factory.h"
#include "ipc/ipc_sync_channel.h"
-#include "ppapi/features/features.h"
#include "services/service_manager/public/cpp/binder_registry.h"
-#include "third_party/WebKit/public/web/WebKit.h"
-
-#if defined(OS_POSIX) && BUILDFLAG(ENABLE_PLUGINS)
-#include "base/files/file_path.h"
-#include "content/common/plugin_list.h"
-#endif
namespace content {
@@ -46,43 +33,41 @@
Init();
}
-UtilityThreadImpl::~UtilityThreadImpl() {
-}
+UtilityThreadImpl::~UtilityThreadImpl() = default;
void UtilityThreadImpl::Shutdown() {
ChildThreadImpl::Shutdown();
}
void UtilityThreadImpl::ReleaseProcessIfNeeded() {
- if (batch_mode_)
- return;
-
- if (IsInBrowserProcess()) {
- // Close the channel to cause UtilityProcessHostImpl to be deleted. We need
- // to take a different code path than the multi-process case because that
- // depends on the child process going away to close the channel, but that
- // can't happen when we're in single process mode.
- channel()->Close();
- } else {
+ if (!IsInBrowserProcess()) {
ChildProcess::current()->ReleaseProcess();
+ return;
}
+
+ // Close the channel to cause the UtilityProcessHostImpl to be deleted. We
+ // need to take a different code path than the multi-process case because
+ // that case depends on the child process going away to close the channel,
+ // but that can't happen when we're in single process mode.
+ channel()->Close();
}
void UtilityThreadImpl::EnsureBlinkInitialized() {
- if (blink_platform_impl_ || IsInBrowserProcess()) {
- // We can only initialize WebKit on one thread, and in single process mode
- // we run the utility thread on separate thread. This means that if any code
- // needs WebKit initialized in the utility process, they need to have
- // another path to support single process mode.
+ if (blink_platform_impl_)
return;
- }
+
+ // We can only initialize Blink on one thread, and in single process mode
+ // we run the utility thread on a separate thread. This means that if any
+ // code needs Blink initialized in the utility process, they need to have
+ // another path to support single process mode.
+ if (IsInBrowserProcess())
+ return;
blink_platform_impl_.reset(new UtilityBlinkPlatformImpl);
blink::Platform::Initialize(blink_platform_impl_.get());
}
void UtilityThreadImpl::Init() {
- batch_mode_ = false;
ChildProcess::current()->AddRefProcess();
auto registry = base::MakeUnique<service_manager::BinderRegistry>();
@@ -106,25 +91,7 @@
}
bool UtilityThreadImpl::OnControlMessageReceived(const IPC::Message& msg) {
- if (GetContentClient()->utility()->OnMessageReceived(msg))
- return true;
-
- bool handled = true;
- IPC_BEGIN_MESSAGE_MAP(UtilityThreadImpl, msg)
- IPC_MESSAGE_HANDLER(UtilityMsg_BatchMode_Started, OnBatchModeStarted)
- IPC_MESSAGE_HANDLER(UtilityMsg_BatchMode_Finished, OnBatchModeFinished)
- IPC_MESSAGE_UNHANDLED(handled = false)
- IPC_END_MESSAGE_MAP()
- return handled;
-}
-
-void UtilityThreadImpl::OnBatchModeStarted() {
- batch_mode_ = true;
-}
-
-void UtilityThreadImpl::OnBatchModeFinished() {
- batch_mode_ = false;
- ReleaseProcessIfNeeded();
+ return GetContentClient()->utility()->OnMessageReceived(msg);
}
void UtilityThreadImpl::BindServiceFactoryRequest(
diff --git a/content/utility/utility_thread_impl.h b/content/utility/utility_thread_impl.h
index 6293e08e..9bfdc75 100644
--- a/content/utility/utility_thread_impl.h
+++ b/content/utility/utility_thread_impl.h
@@ -6,14 +6,10 @@
#define CONTENT_UTILITY_UTILITY_THREAD_IMPL_H_
#include <memory>
-#include <string>
-#include <vector>
#include "base/compiler_specific.h"
#include "base/macros.h"
-#include "build/build_config.h"
#include "content/child/child_thread_impl.h"
-#include "content/common/content_export.h"
#include "content/public/utility/utility_thread.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "services/service_manager/public/interfaces/service_factory.mojom.h"
@@ -37,31 +33,27 @@
public ChildThreadImpl {
public:
UtilityThreadImpl();
- // Constructor that's used when running in single process mode.
+ // Constructor used when running in single process mode.
explicit UtilityThreadImpl(const InProcessChildThreadParams& params);
~UtilityThreadImpl() override;
void Shutdown() override;
+ // UtilityThread:
void ReleaseProcessIfNeeded() override;
void EnsureBlinkInitialized() override;
private:
void Init();
- // ChildThread implementation.
+ // ChildThread:
bool OnControlMessageReceived(const IPC::Message& msg) override;
- // IPC message handlers.
- void OnBatchModeStarted();
- void OnBatchModeFinished();
-
+ // Binds requests to our |service factory_|.
void BindServiceFactoryRequest(
const service_manager::BindSourceInfo& source_info,
service_manager::mojom::ServiceFactoryRequest request);
- // True when we're running in batch mode.
- bool batch_mode_;
-
+ // blink::Platform implementation if needed.
std::unique_ptr<UtilityBlinkPlatformImpl> blink_platform_impl_;
// service_manager::mojom::ServiceFactory for service_manager::Service
diff --git a/ipc/OWNERS b/ipc/OWNERS
index b8baa34..0617e7bf 100644
--- a/ipc/OWNERS
+++ b/ipc/OWNERS
@@ -8,6 +8,8 @@
# new sandbox escapes.
per-file ipc_message_start.h=set noparent
per-file ipc_message_start.h=file://ipc/SECURITY_OWNERS
+per-file *_messages*.h=set noparent
+per-file *_messages*.h=file://ipc/SECURITY_OWNERS
per-file *.mojom=set noparent
per-file *.mojom=file://ipc/SECURITY_OWNERS
per-file *_param_traits*.*=set noparent
diff --git a/ipc/ipc_channel_proxy_unittest.cc b/ipc/ipc_channel_proxy_unittest.cc
index f059099..01b8955 100644
--- a/ipc/ipc_channel_proxy_unittest.cc
+++ b/ipc/ipc_channel_proxy_unittest.cc
@@ -90,7 +90,7 @@
IPC_BEGIN_MESSAGE_MAP(ChannelReflectorListener, message)
IPC_MESSAGE_HANDLER(TestMsg_Bounce, OnTestBounce)
IPC_MESSAGE_HANDLER(TestMsg_SendBadMessage, OnSendBadMessage)
- IPC_MESSAGE_HANDLER(UtilityMsg_Bounce, OnUtilityBounce)
+ IPC_MESSAGE_HANDLER(AutomationMsg_Bounce, OnAutomationBounce)
IPC_MESSAGE_HANDLER(WorkerMsg_Bounce, OnBounce)
IPC_MESSAGE_HANDLER(WorkerMsg_Quit, OnQuit)
IPC_END_MESSAGE_MAP()
@@ -105,9 +105,7 @@
channel_->Send(new TestMsg_BadMessage(BadType()));
}
- void OnUtilityBounce() {
- channel_->Send(new UtilityMsg_Bounce());
- }
+ void OnAutomationBounce() { channel_->Send(new AutomationMsg_Bounce()); }
void OnBounce() {
channel_->Send(new WorkerMsg_Bounce());
@@ -282,17 +280,17 @@
TEST_F(IPCChannelProxyTest, MessageClassFilters) {
// Construct a filter per message class.
- std::vector<scoped_refptr<MessageCountFilter> > class_filters;
- class_filters.push_back(make_scoped_refptr(
- new MessageCountFilter(TestMsgStart)));
- class_filters.push_back(make_scoped_refptr(
- new MessageCountFilter(UtilityMsgStart)));
+ std::vector<scoped_refptr<MessageCountFilter>> class_filters;
+ class_filters.push_back(
+ make_scoped_refptr(new MessageCountFilter(TestMsgStart)));
+ class_filters.push_back(
+ make_scoped_refptr(new MessageCountFilter(AutomationMsgStart)));
for (size_t i = 0; i < class_filters.size(); ++i)
channel_proxy()->AddFilter(class_filters[i].get());
// Send a message for each class; each filter should receive just one message.
- sender()->Send(new TestMsg_Bounce());
- sender()->Send(new UtilityMsg_Bounce());
+ sender()->Send(new TestMsg_Bounce);
+ sender()->Send(new AutomationMsg_Bounce);
// Send some messages not assigned to a specific or valid message class.
sender()->Send(new WorkerMsg_Bounce);
@@ -320,7 +318,7 @@
sender()->Send(new TestMsg_Bounce);
// A message of a different class should be seen only by the global filter.
- sender()->Send(new UtilityMsg_Bounce);
+ sender()->Send(new AutomationMsg_Bounce);
// Flush all messages.
SendQuitMessageAndWaitForIdle();
@@ -347,7 +345,7 @@
// Send some messages; they should not be seen by either filter.
sender()->Send(new TestMsg_Bounce);
- sender()->Send(new UtilityMsg_Bounce);
+ sender()->Send(new AutomationMsg_Bounce);
// Ensure that the filters were removed and did not receive any messages.
SendQuitMessageAndWaitForIdle();
diff --git a/ipc/ipc_channel_proxy_unittest_messages.h b/ipc/ipc_channel_proxy_unittest_messages.h
index 288ce53..ae45059 100644
--- a/ipc/ipc_channel_proxy_unittest_messages.h
+++ b/ipc/ipc_channel_proxy_unittest_messages.h
@@ -30,15 +30,15 @@
#endif // IPC_CHANNEL_PROXY_UNITTEST_MESSAGES_H_
-
+#undef IPC_MESSAGE_START
#define IPC_MESSAGE_START TestMsgStart
IPC_MESSAGE_CONTROL0(TestMsg_Bounce)
IPC_MESSAGE_CONTROL0(TestMsg_SendBadMessage)
IPC_MESSAGE_CONTROL1(TestMsg_BadMessage, BadType)
#undef IPC_MESSAGE_START
-#define IPC_MESSAGE_START UtilityMsgStart
-IPC_MESSAGE_CONTROL0(UtilityMsg_Bounce)
+#define IPC_MESSAGE_START AutomationMsgStart
+IPC_MESSAGE_CONTROL0(AutomationMsg_Bounce)
#undef IPC_MESSAGE_START
#define IPC_MESSAGE_START WorkerMsgStart
diff --git a/ipc/ipc_message_start.h b/ipc/ipc_message_start.h
index 7b5935e..dba1acb4 100644
--- a/ipc/ipc_message_start.h
+++ b/ipc/ipc_message_start.h
@@ -19,7 +19,6 @@
DevToolsMsgStart,
WorkerMsgStart,
NaClMsgStart,
- UtilityMsgStart,
GpuChannelMsgStart,
MediaMsgStart,
ServiceMsgStart,