Convert utility process zip creator IPC to mojo

chrome/browser/chromeos/file_manager/zip_file_creator.cc

Index: chrome/browser/chromeos/file_manager/zip_file_creator.cc
diff --git a/chrome/browser/chromeos/file_manager/zip_file_creator.cc b/chrome/browser/chromeos/file_manager/zip_file_creator.cc
index d5d66208ab835621f5e36f56db946b8bedb1be10..19887d92d8d342c368e5c876248bf742d61003cd 100644
--- a/chrome/browser/chromeos/file_manager/zip_file_creator.cc
+++ b/chrome/browser/chromeos/file_manager/zip_file_creator.cc
@@ -8,17 +8,11 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
-#include "base/message_loop/message_loop.h"
 #include "base/task_scheduler/post_task.h"
-#include "chrome/common/chrome_utility_messages.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/utility_process_host.h"
 #include "ui/base/l10n/l10n_util.h"
 
-using content::BrowserThread;
-using content::UtilityProcessHost;
-
 namespace {
 
 // Creates the destination zip file only if it does not already exist.
@@ -43,35 +37,19 @@ ZipFileCreator::ZipFileCreator(
 }
 
 void ZipFileCreator::Start() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   base::PostTaskWithTraitsAndReplyWithResult(
       FROM_HERE, base::TaskTraits().MayBlock(),
       base::Bind(&OpenFileHandleAsync, dest_file_),
-      base::Bind(&ZipFileCreator::OnOpenFileHandle, this));
-}
-
-ZipFileCreator::~ZipFileCreator() {
+      base::Bind(&ZipFileCreator::CreateZipFile, this));
 }
 
-bool ZipFileCreator::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(ZipFileCreator, message)
-    IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_CreateZipFile_Succeeded,
-                        OnCreateZipFileSucceeded)
-    IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_CreateZipFile_Failed,
-                        OnCreateZipFileFailed)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
-void ZipFileCreator::OnProcessCrashed(int exit_code) {
-  ReportDone(false);
-}
+ZipFileCreator::~ZipFileCreator() = default;
 
-void ZipFileCreator::OnOpenFileHandle(base::File file) {
+void ZipFileCreator::CreateZipFile(base::File file) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(!utility_process_mojo_client_);
 
   if (!file.IsValid()) {
     LOG(ERROR) << "Failed to create dest zip file " << dest_file_.value();
@@ -79,41 +57,30 @@ void ZipFileCreator::OnOpenFileHandle(base::File file) {
     return;
   }
 
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(
-          &ZipFileCreator::StartProcessOnIOThread, this, base::Passed(&file)));
-}
-
-void ZipFileCreator::StartProcessOnIOThread(base::File dest_file) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  base::FileDescriptor dest_fd(std::move(dest_file));
-
-  UtilityProcessHost* host = UtilityProcessHost::Create(
-      this, BrowserThread::GetTaskRunnerForThread(BrowserThread::UI).get());
-  host->SetName(
+  utility_process_mojo_client_ = base::MakeUnique<
+      content::UtilityProcessMojoClient<chrome::mojom::ZipFileCreator>>(
       l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_ZIP_FILE_CREATOR_NAME));
-  host->SetExposedDir(src_dir_);
-  host->Send(new ChromeUtilityMsg_CreateZipFile(src_dir_, src_relative_paths_,
-                                                dest_fd));
-}
+  utility_process_mojo_client_->set_error_callback(
+      base::Bind(&ZipFileCreator::ReportDone, this, false));
 
-void ZipFileCreator::OnCreateZipFileSucceeded() {
-  ReportDone(true);
-}
+  utility_process_mojo_client_->set_exposed_directory(src_dir_);
+
+  utility_process_mojo_client_->Start();
 
-void ZipFileCreator::OnCreateZipFileFailed() {
-  ReportDone(false);
+  utility_process_mojo_client_->service()->CreateZipFile(
+      src_dir_, src_relative_paths_, std::move(file),
+      base::Bind(&ZipFileCreator::ReportDone, this));
 }
 
 void ZipFileCreator::ReportDone(bool success) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  // Guard against calling observer multiple times.
-  if (!callback_.is_null())
-    base::ResetAndReturn(&callback_).Run(success);
+  // The current user of this class holds no reference to |this| so resetting
+  // the |utility_process_mojo_client_| here could release the last reference
+  // and delete |this|. So save |callback_| before resetting the client.

[Noel Gordon, 2017/03/01 03:22:47]

sammc, tibell: I figure that mojo users might need to add a similar comment in
their code, because of the way ref counted types and base::Bind work behind the
scenes, which (to me) sometimes makes it hard to reason about ref counts.

Was thinking that if mojo always used the pattern here (save the callback first,
before invoking it), then an extra ref on the callback's |this| client (for
refcounted |this|) would be assured.   So any clients that delete / reset their
mojo stuff during the mojo callback, as in this code, would not need the comment
... since |this| would be guaranteed to have at least one ref for the duration
of the mojo callback.

Something to think about, anyho ...

+  auto callback = base::ResetAndReturn(&callback_);
+  utility_process_mojo_client_.reset();
+  callback.Run(success);
 }
 
 }  // namespace file_manager