Add the UtilityProcessMojoClient class and convert SafeJsonParser

content/browser/utility_process_mojo_client_impl.cc

Index: content/browser/utility_process_mojo_client_impl.cc
diff --git a/content/browser/utility_process_mojo_client_impl.cc b/content/browser/utility_process_mojo_client_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..32828f32ec607d94dd43ecf38a0625f4171470bf
--- /dev/null
+++ b/content/browser/utility_process_mojo_client_impl.cc
@@ -0,0 +1,94 @@
+// Copyright 2016 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.
+
+#include "content/public/browser/utility_process_mojo_client_impl.h"
+
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/utility_process_host.h"
+#include "content/public/browser/utility_process_host_client.h"
+#include "content/public/common/service_registry.h"
+
+namespace content {
+
+// Helper ref-counted class that takes care of managing the lifetime of the

[michaeln, 2016/06/14 18:37:27]

Generally we like to avoid refcounting. This class doesn't really need to be
refcounted given it has a single consumer.

It'd be fine to use a std::unique_ptr<Helper> and to call
BrowserThread::DeleteSoon(IO, helper_.release).

[Patrick Monette, 2016/06/14 21:23:32]

Good idea. Done.

+// utility process on the IO thread.
+class UtilityProcessMojoClientImpl::Helper
+    : public base::RefCountedThreadSafe<Helper> {
+ public:
+  explicit Helper(const base::string16& process_name)
+      : process_name_(process_name) {}
+
+  // Starts the utility process on the IO thread.
+  void Start(const std::string& mojo_interface_name,
+             mojo::ScopedMessagePipeHandle interface_pipe) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&Helper::StartOnIOThread, this, mojo_interface_name,
+                   base::Passed(&interface_pipe)));
+  }
+
+  // Shuts down the utility process.
+  void Shutdown() {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&Helper::ShutdownOnIOThread, this));
+  }
+
+  void set_disable_sandbox() { disable_sandbox_ = true; }

[michaeln, 2016/06/14 18:37:26]

Presumably the setter is for use on the main thread. This could be racey, the
value is read on the IO thread.

[Patrick Monette, 2016/06/14 21:23:32]

The contract of the class is explicit about calling set_disable_sandbox() before
calling Start(), which posts the task that will eventually read
disable_sandbox_. I'll add a DCHECK(!start_called) so developers can immediately
see their mistake if they do this.

+
+ private:
+  friend class base::RefCountedThreadSafe<Helper>;
+
+  ~Helper() = default;
+
+  // Starts the utility process and connects to the remote Mojo service.
+  void StartOnIOThread(const std::string& mojo_interface_name,
+                       mojo::ScopedMessagePipeHandle interface_pipe) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    utility_host_ = UtilityProcessHost::Create(nullptr, nullptr)->AsWeakPtr();

[michaeln, 2016/06/14 18:37:27]

What is the reason to use a WeakPtr<> here? It looks like this object strictly
owns the utility_host_. Sp long as the object is destructed on the IO thread,
the shutdown methods might not be needed at all.

[Patrick Monette, 2016/06/14 21:23:32]

utility_host_ manages its own lifetime. The WeakPtr<> is kept to be able to
force a termination if it is still running.

[michaeln, 2016/06/14 22:10:50]

i see, thnx, you might want a comment about that to explain the weakptr usage, a
code comment around the call to delete utility_host_.get() would do it

[Patrick Monette, 2016/06/15 15:28:09]

Done.

+    utility_host_->SetName(process_name_);
+    if (disable_sandbox_)
+      utility_host_->DisableSandbox();
+
+    utility_host_->Start();
+
+    ServiceRegistry* service_registry = utility_host_->GetServiceRegistry();
+    service_registry->ConnectToRemoteService(mojo_interface_name,
+                                             std::move(interface_pipe));
+  }
+
+  void ShutdownOnIOThread() {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    delete utility_host_.get();
+  }
+
+  // Properties of the utility process.
+  base::string16 process_name_;
+  bool disable_sandbox_ = false;
+
+  // Must only be accessed on the IO thread.
+  base::WeakPtr<UtilityProcessHost> utility_host_;
+
+  DISALLOW_COPY_AND_ASSIGN(Helper);
+};
+
+UtilityProcessMojoClientImpl::UtilityProcessMojoClientImpl(
+    const base::string16& process_name) {
+  helper_ = new Helper(process_name);
+}
+
+UtilityProcessMojoClientImpl::~UtilityProcessMojoClientImpl() {
+  helper_->Shutdown();
+}
+
+void UtilityProcessMojoClientImpl::set_disable_sandbox() {
+  helper_->set_disable_sandbox();
+}
+
+void UtilityProcessMojoClientImpl::Start(
+    const std::string& mojo_interface_name,
+    mojo::ScopedMessagePipeHandle interface_pipe) {
+  helper_->Start(mojo_interface_name, std::move(interface_pipe));
+}
+
+}  // namespace content