Convert utility process ImageWriter IPC to mojo

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

 // Copyright 2014 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 "chrome/browser/extensions/api/image_writer_private/image_writer_utility_client.h"
+
 #include "base/bind.h"
+#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_;
+  // |image_writer_utility_client_| owns |this|.
+  ImageWriterUtilityClient* const image_writer_utility_client_;
+
+  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);
+
+  utility_process_mojo_client_->service()->Write(source, target,
+                                                 std::move(client));
 }
 
 void ImageWriterUtilityClient::Verify(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_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();
+
+  extensions::mojom::RemovableStorageWriterClientPtr client;
+  removable_storage_writer_client_ =
+      base::MakeUnique<RemovableStorageWriterClientImpl>(this, &client);
+
+  utility_process_mojo_client_->service()->Verify(source, target,
+                                                  std::move(client));
 }
 
 void ImageWriterUtilityClient::Cancel(const CancelCallback& cancel_callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  if (!utility_process_host_) {
-    // If we haven't connected, there is nothing to cancel.
-    task_runner_->PostTask(FROM_HERE, cancel_callback);
-    return;
-  }
-
-  cancel_callback_ = cancel_callback;
-
-  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);
+
+  ResetRequest();
+  utility_process_mojo_client_.reset();
+}
+
+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_mojo_client_->set_run_elevated();
+#endif
+
+  utility_process_mojo_client_->Start();
+}
+
+void ImageWriterUtilityClient::UtilityProcessError() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  OperationFailed("Utility process crashed or failed.");
+  utility_process_mojo_client_.reset();
+}
+
+void ImageWriterUtilityClient::OperationProgress(int64_t progress) {
+  if (progress_callback_)
+    task_runner_->PostTask(FROM_HERE, base::Bind(progress_callback_, progress));
+}
+
+void ImageWriterUtilityClient::OperationSucceeded() {
+  SuccessCallback success_callback = success_callback_;
+  ResetRequest();

[dcheng, 2017/02/06 05:52:02]

It seems like we call ResetRequest() a lot more, rather than just in Shutdown().
Is there any particular reason this changed? If so, it might be nice to document
this in the CL description or somewhere else.

[Noel Gordon, 2017/02/07 11:31:09]

a lot more -> once per operation, which seems ideal.  It makes this code
stateful - it wasn't before, and it wasn't clear if a VERIFY can be requested
while a WRITE is in progress.  DCHECK(!removable_storage_writer_client_) at the
start of these operation now state that no existing operation should be in
progress.  That was the original code contract, it just wasn't ever enforced.

+  if (success_callback)
+    task_runner_->PostTask(FROM_HERE, success_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::ResetRequest() {
+  removable_storage_writer_client_.reset();
 
   // 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();
+  progress_callback_.Reset();
+  success_callback_.Reset();
+  error_callback_.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));
-
-#if defined(OS_WIN)
-    utility_process_host_->ElevatePrivileges();
-#else
-    utility_process_host_->DisableSandbox();
-#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)));
-}
-
-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)));
-}
-
-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;
-}
-
-bool ImageWriterUtilityClient::Send(IPC::Message* msg) {
-  return utility_process_host_ && utility_process_host_->Send(msg);
-}
-
-void ImageWriterUtilityClient::OnWriteImageSucceeded() {
-  if (!success_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, success_callback_);
-  }
-}
-
-void ImageWriterUtilityClient::OnWriteImageCancelled() {
-  if (!cancel_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, cancel_callback_);
-  }
-}
-
-void ImageWriterUtilityClient::OnWriteImageFailed(const std::string& message) {
-  if (!error_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, base::Bind(error_callback_, message));
-  }
-}
-
-void ImageWriterUtilityClient::OnWriteImageProgress(int64_t progress) {
-  if (!progress_callback_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, base::Bind(progress_callback_, progress));
-  }
-}