Convert utility process ImageWriter IPC to mojo

chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.cc

Index: chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.cc
diff --git a/chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.cc b/chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.cc
index e64605afe4159881e90bfbf6aabcfd969bd841d8..45575fd8f852e209db057893bc184e7226576fb6 100644
--- a/chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.cc
+++ b/chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.cc
@@ -2,43 +2,83 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.h"
+
 #include "base/bind.h"
+#include "base/callback.h"

[Sam McNally, 2017/02/02 21:54:17]

Remove.

[Noel Gordon, 2017/02/03 13:54:56]

Done.

+#include "base/files/file_path.h"
 #include "base/location.h"
-#include "base/strings/stringprintf.h"
+#include "base/memory/ptr_util.h"
+#include "base/optional.h"
+#include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "build/build_config.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.h"
-#include "chrome/common/extensions/chrome_utility_extensions_messages.h"
+#include "chrome/common/extensions/removable_storage_writer.mojom.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/utility_process_mojo_client.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "ui/base/l10n/l10n_util.h"
 
-using content::BrowserThread;
+class ImageWriterUtilityClient::RemovableStorageWriterClientImpl
+    : public extensions::mojom::RemovableStorageWriterClient {
+ public:
+  RemovableStorageWriterClientImpl(
+      ImageWriterUtilityClient* owner,
+      extensions::mojom::RemovableStorageWriterClientPtr* interface)
+      : binding_(this, interface), image_writer_utility_client_(owner) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+    binding_.set_connection_error_handler(
+        base::Bind(&ImageWriterUtilityClient::UtilityProcessError,
+                   image_writer_utility_client_));
+  }
+
+  ~RemovableStorageWriterClientImpl() override = default;
+
+ private:
+  void Progress(int64_t progress) override {
+    image_writer_utility_client_->OperationProgress(progress);
+  }
+
+  void Complete(const base::Optional<std::string>& error) override {
+    if (error) {
+      image_writer_utility_client_->OperationFailed(error.value());
+    } else {
+      image_writer_utility_client_->OperationSucceeded();
+    }
+  }
+
+  mojo::Binding<extensions::mojom::RemovableStorageWriterClient> binding_;
+  ImageWriterUtilityClient* const image_writer_utility_client_;

[Sam McNally, 2017/02/02 21:54:17]

Comment that |image_writer_utility_client_| owns |this|.

[Noel Gordon, 2017/02/03 13:54:56]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(RemovableStorageWriterClientImpl);
+};
 
 ImageWriterUtilityClient::ImageWriterUtilityClient()
     : task_runner_(base::ThreadTaskRunnerHandle::Get()) {
 }
-ImageWriterUtilityClient::~ImageWriterUtilityClient() {}
+
+ImageWriterUtilityClient::~ImageWriterUtilityClient() = default;
 
 void ImageWriterUtilityClient::Write(const ProgressCallback& progress_callback,
                                      const SuccessCallback& success_callback,
                                      const ErrorCallback& error_callback,
                                      const base::FilePath& source,
                                      const base::FilePath& target) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  StartHost();
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK(!removable_storage_writer_client_);
 
   progress_callback_ = progress_callback;
   success_callback_ = success_callback;
   error_callback_ = error_callback;
 
-  if (!Send(new ChromeUtilityMsg_ImageWriter_Write(source, target))) {
-    DLOG(ERROR) << "Unable to send Write message to Utility Process.";
-    task_runner_->PostTask(
-        FROM_HERE, base::Bind(error_callback_, "IPC communication failed"));
-  }
+  StartUtilityProcess();
+
+  extensions::mojom::RemovableStorageWriterClientPtr client;
+  removable_storage_writer_client_ =
+      base::MakeUnique<RemovableStorageWriterClientImpl>(this, &client);
+
+  UtilityProcessService()->Write(source, target, std::move(client));
 }
 
 void ImageWriterUtilityClient::Verify(const ProgressCallback& progress_callback,
@@ -46,124 +86,90 @@ void ImageWriterUtilityClient::Verify(const ProgressCallback& progress_callback,
                                       const ErrorCallback& error_callback,
                                       const base::FilePath& source,
                                       const base::FilePath& target) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  StartHost();
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK(!removable_storage_writer_client_);
 
   progress_callback_ = progress_callback;
   success_callback_ = success_callback;
   error_callback_ = error_callback;
 
-  if (!Send(new ChromeUtilityMsg_ImageWriter_Verify(source, target))) {
-    DLOG(ERROR) << "Unable to send Verify message to Utility Process.";
-    task_runner_->PostTask(
-        FROM_HERE, base::Bind(error_callback_, "IPC communication failed"));
-  }
-}
+  StartUtilityProcess();
 
-void ImageWriterUtilityClient::Cancel(const CancelCallback& cancel_callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  extensions::mojom::RemovableStorageWriterClientPtr client;
+  removable_storage_writer_client_ =
+      base::MakeUnique<RemovableStorageWriterClientImpl>(this, &client);
 
-  if (!utility_process_host_) {
-    // If we haven't connected, there is nothing to cancel.
-    task_runner_->PostTask(FROM_HERE, cancel_callback);
-    return;
-  }
+  UtilityProcessService()->Verify(source, target, std::move(client));
+}
 
-  cancel_callback_ = cancel_callback;
+void ImageWriterUtilityClient::Cancel(const CancelCallback& cancel_callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  if (!Send(new ChromeUtilityMsg_ImageWriter_Cancel())) {
-    DLOG(ERROR) << "Unable to send Cancel message to Utility Process.";
-  }
+  ResetRequest();
+  task_runner_->PostTask(FROM_HERE, cancel_callback);
 }
 
 void ImageWriterUtilityClient::Shutdown() {
-  if (utility_process_host_ &&
-      Send(new ChromeUtilityMsg_ImageWriter_Cancel())) {
-    utility_process_host_->EndBatchMode();
-  }
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  // Clear handlers to not hold any reference to the caller.
-  success_callback_ = base::Closure();
-  progress_callback_ = base::Callback<void(int64_t)>();
-  error_callback_ = base::Callback<void(const std::string&)>();
-  cancel_callback_ = base::Closure();
+  ResetRequest();
+  utility_process_mojo_client_.reset();
 }
 
-void ImageWriterUtilityClient::StartHost() {
-  if (!utility_process_host_) {
-    scoped_refptr<base::SequencedTaskRunner> task_runner =
-        base::ThreadTaskRunnerHandle::Get();
-    utility_process_host_ = content::UtilityProcessHost::Create(
-                                this, task_runner.get())->AsWeakPtr();
-    utility_process_host_->SetName(l10n_util::GetStringUTF16(
-        IDS_UTILITY_PROCESS_IMAGE_WRITER_NAME));
+void ImageWriterUtilityClient::StartUtilityProcess() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
+  if (utility_process_mojo_client_)
+    return;
+
+  const base::string16 utility_process_name =
+      l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_IMAGE_WRITER_NAME);
+
+  utility_process_mojo_client_.reset(
+      new content::UtilityProcessMojoClient<
+          extensions::mojom::RemovableStorageWriter>(utility_process_name));
+  utility_process_mojo_client_->set_error_callback(
+      base::Bind(&ImageWriterUtilityClient::UtilityProcessError, this));
+
+  utility_process_mojo_client_->set_disable_sandbox();
 #if defined(OS_WIN)
-    utility_process_host_->ElevatePrivileges();
-#else
-    utility_process_host_->DisableSandbox();
+  utility_process_mojo_client_->set_run_elevated();
 #endif
-    utility_process_host_->StartBatchMode();
-  }
-}
 
-void ImageWriterUtilityClient::OnProcessCrashed(int exit_code) {
-  task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(error_callback_,
-                 base::StringPrintf("Utility process crashed with code %08x.",
-                                    exit_code)));
+  utility_process_mojo_client_->Start();
 }
 
-void ImageWriterUtilityClient::OnProcessLaunchFailed(int error_code) {
-  task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(error_callback_,
-                 base::StringPrintf("Process launch failed with code %08x.",
-                                    error_code)));
-}
+void ImageWriterUtilityClient::UtilityProcessError() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-bool ImageWriterUtilityClient::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(ImageWriterUtilityClient, message)
-  IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_ImageWriter_Succeeded,
-                      OnWriteImageSucceeded)
-  IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_ImageWriter_Cancelled,
-                      OnWriteImageCancelled)
-  IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_ImageWriter_Failed,
-                      OnWriteImageFailed)
-  IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_ImageWriter_Progress,
-                      OnWriteImageProgress)
-  IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
+  OperationFailed("Utility process mojo connection error");

[Sam McNally, 2017/02/02 21:54:17]

Is this message used for anything user-facing?

[Noel Gordon, 2017/02/03 13:54:56]

A chrome extension could could show it if it wanted.  Looking at diff-left, the
error_callback messages like this were:

"IPC communication failed"
"Utility process crashed with code %08x." (printf-ed)
"Process launch failed with code %08x. (printf-ed)

On a punt, I assumed your question was maybe about having "mojo" in the error
message, and looking at the other error messages above, I changed the error
message to:

"Utility process crashed or failed."

+  utility_process_mojo_client_.reset();
 }
 
-bool ImageWriterUtilityClient::Send(IPC::Message* msg) {
-  return utility_process_host_ && utility_process_host_->Send(msg);
+void ImageWriterUtilityClient::OperationProgress(int64_t progress) {
+  if (progress_callback_)
+    task_runner_->PostTask(FROM_HERE, base::Bind(progress_callback_, progress));
 }
 
-void ImageWriterUtilityClient::OnWriteImageSucceeded() {
-  if (!success_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, success_callback_);
-  }
+void ImageWriterUtilityClient::OperationSucceeded() {
+  SuccessCallback success_callback = success_callback_;
+  ResetRequest();
+  if (success_callback)
+    task_runner_->PostTask(FROM_HERE, success_callback);
 }
 
-void ImageWriterUtilityClient::OnWriteImageCancelled() {
-  if (!cancel_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, cancel_callback_);
-  }
+void ImageWriterUtilityClient::OperationFailed(const std::string& error) {
+  ErrorCallback error_callback = error_callback_;
+  ResetRequest();
+  if (error_callback)
+    task_runner_->PostTask(FROM_HERE, base::Bind(error_callback, error));
 }
 
-void ImageWriterUtilityClient::OnWriteImageFailed(const std::string& message) {
-  if (!error_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, base::Bind(error_callback_, message));
-  }
-}
+void ImageWriterUtilityClient::ResetRequest() {
+  removable_storage_writer_client_.reset();
 
-void ImageWriterUtilityClient::OnWriteImageProgress(int64_t progress) {
-  if (!progress_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, base::Bind(progress_callback_, progress));
-  }
+  // Clear handlers to not hold any reference to the caller.
+  progress_callback_.Reset();
+  success_callback_.Reset();
+  error_callback_.Reset();
 }