Convert utility process extension Unpacker IPC to mojo

chrome/browser/extensions/zipfile_installer.cc

Index: chrome/browser/extensions/zipfile_installer.cc
diff --git a/chrome/browser/extensions/zipfile_installer.cc b/chrome/browser/extensions/zipfile_installer.cc
index f0da06da424956808b216a4e2522e795f1626999..bb020af8de74e8094f20673e146c6785dcbb5e95 100644
--- a/chrome/browser/extensions/zipfile_installer.cc
+++ b/chrome/browser/extensions/zipfile_installer.cc
@@ -13,121 +13,117 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/utility_process_host.h"
-#include "extensions/common/extension_utility_messages.h"
 #include "ui/base/l10n/l10n_util.h"
 
-using content::BrowserThread;
-using content::UtilityProcessHost;
-
 namespace {
 
 const char kExtensionHandlerTempDirError[] =
     "Could not create temporary directory for zipped extension.";
+const char kExtensionHandlerFileUnzipError[] =
+    "Could not unzip extension for install.";
 
 }  // namespace
 
 namespace extensions {
 
-ZipFileInstaller::ZipFileInstaller(ExtensionService* extension_service)
-    : be_noisy_on_failure_(true),
-      extension_service_weak_(extension_service->AsWeakPtr()) {
+// static
+scoped_refptr<ZipFileInstaller> ZipFileInstaller::Create(
+    ExtensionService* service) {
+  DCHECK(service);
+  return make_scoped_refptr(new ZipFileInstaller(service));
 }
 
-void ZipFileInstaller::LoadFromZipFile(const base::FilePath& path) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  zip_path_ = path;
-  BrowserThread::PostTask(BrowserThread::FILE,
-                          FROM_HERE,
-                          base::Bind(&ZipFileInstaller::PrepareTempDir, this));
+void ZipFileInstaller::LoadFromZipFile(const base::FilePath& zip_file) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  zip_file_ = zip_file;
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ZipFileInstaller::PrepareTempDir, this));
 }
 
+ZipFileInstaller::ZipFileInstaller(ExtensionService* service)
+    : be_noisy_on_failure_(true),
+      extension_service_weak_(service->AsWeakPtr()) {}
+
+ZipFileInstaller::~ZipFileInstaller() = default;
+
 void ZipFileInstaller::PrepareTempDir() {
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  base::FilePath temp_dir;
-  PathService::Get(base::DIR_TEMP, &temp_dir);
-  base::FilePath new_temp_dir;
-  if (!base::CreateTemporaryDirInDir(
-          temp_dir,
-          zip_path_.RemoveExtension().BaseName().value() +
-              FILE_PATH_LITERAL("_"),
-          &new_temp_dir)) {
-    OnUnzipFailed(std::string(kExtensionHandlerTempDirError));
+  DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
+
+  base::FilePath dir_temp;
+  PathService::Get(base::DIR_TEMP, &dir_temp);
+
+  base::FilePath::StringType dir_name =
+      zip_file_.RemoveExtension().BaseName().value() + FILE_PATH_LITERAL("_");
+
+  if (!base::CreateTemporaryDirInDir(dir_temp, dir_name, &temp_dir_)) {

[Sam McNally, 2017/02/14 05:41:52]

This class would be easier to understand if fields were only accessed on the UI
thread. Returning the path to be set to a field on the UI thread would be nicer.

[Noel Gordon, 2017/02/14 12:35:37]

Done.  Passed the zip_file path in, passed out the temp_dir path, so fields
(members) are now only accessed on the UI thread.

+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(&ZipFileInstaller::ReportErrorOnUIThread, this,
+                   std::string(kExtensionHandlerTempDirError)));
     return;
   }
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&ZipFileInstaller::StartWorkOnIOThread, this, new_temp_dir));
-}
 
-ZipFileInstaller::~ZipFileInstaller() {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&ZipFileInstaller::UnzipOnIOThread, this));
 }
 
-// static
-scoped_refptr<ZipFileInstaller> ZipFileInstaller::Create(
-    ExtensionService* extension_service) {
-  DCHECK(extension_service);
-  return scoped_refptr<ZipFileInstaller>(
-      new ZipFileInstaller(extension_service));
-}
+void ZipFileInstaller::UnzipOnIOThread() {

[Sam McNally, 2017/02/14 05:41:52]

Let's run this on the UI thread instead and let the UtilityProcessMojoClient
deal with the IO thread.

[Noel Gordon, 2017/02/14 12:35:37]

Done. Simplifies things and removes the need for LoadOnUIThread too.

+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK(!utility_process_mojo_client_);
 
-void ZipFileInstaller::StartWorkOnIOThread(const base::FilePath& temp_dir) {
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  UtilityProcessHost* host =
-      UtilityProcessHost::Create(this,
-                                 base::ThreadTaskRunnerHandle::Get().get());
-  host->SetName(l10n_util::GetStringUTF16(
-      IDS_UTILITY_PROCESS_ZIP_FILE_INSTALLER_NAME));
-  host->SetExposedDir(temp_dir);
-  host->Send(new ExtensionUtilityMsg_UnzipToDir(zip_path_, temp_dir));
-}
+  const base::string16 utility_process_name =
+      l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_ZIP_FILE_INSTALLER_NAME);
 
-void ZipFileInstaller::ReportSuccessOnUIThread(
-    const base::FilePath& unzipped_path) {
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (extension_service_weak_.get())
-    UnpackedInstaller::Create(extension_service_weak_.get())
-        ->Load(unzipped_path);
+  utility_process_mojo_client_.reset(

[Sam McNally, 2017/02/14 05:41:52]

utility_process_mojo_client_ =
base::MakeUnique<content::UtilityProcessMojoClient<extensions::mojom::ExtensionUnpacker>>(utility_process_name);

[Noel Gordon, 2017/02/14 12:35:37]

Done.

+      new content::UtilityProcessMojoClient<
+          extensions::mojom::ExtensionUnpacker>(utility_process_name));
+  utility_process_mojo_client_->set_error_callback(
+      base::Bind(&ZipFileInstaller::UnzipDone, this, false));
+
+  utility_process_mojo_client_->set_exposed_directory(temp_dir_);
+
+  utility_process_mojo_client_->Start();
+
+  utility_process_mojo_client_->service()->Unzip(
+      zip_file_, temp_dir_, base::Bind(&ZipFileInstaller::UnzipDone, this));
 }
 
-void ZipFileInstaller::ReportErrorOnUIThread(const std::string& error) {
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (extension_service_weak_.get()) {
-    ExtensionErrorReporter::GetInstance()->ReportLoadError(
-        zip_path_,
-        error,
-        extension_service_weak_->profile(),
-        be_noisy_on_failure_);
+void ZipFileInstaller::UnzipDone(bool success) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  utility_process_mojo_client_.reset();
+
+  if (!success) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(&ZipFileInstaller::ReportErrorOnUIThread, this,
+                   std::string(kExtensionHandlerFileUnzipError)));
+    return;
   }
-}
 
-void ZipFileInstaller::OnUnzipSucceeded(const base::FilePath& unzipped_path) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(
-          &ZipFileInstaller::ReportSuccessOnUIThread, this, unzipped_path));
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&ZipFileInstaller::LoadOnUIThread, this));
 }
 
-void ZipFileInstaller::OnUnzipFailed(const std::string& error) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&ZipFileInstaller::ReportErrorOnUIThread, this, error));
+void ZipFileInstaller::LoadOnUIThread() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (extension_service_weak_)
+    UnpackedInstaller::Create(extension_service_weak_.get())->Load(temp_dir_);
 }
 
-bool ZipFileInstaller::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(ZipFileInstaller, message)
-  IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnzipToDir_Succeeded,
-                      OnUnzipSucceeded)
-  IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnzipToDir_Failed, OnUnzipFailed)
-  IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
+void ZipFileInstaller::ReportErrorOnUIThread(const std::string& error) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (extension_service_weak_) {
+    ExtensionErrorReporter::GetInstance()->ReportLoadError(
+        zip_file_, error, extension_service_weak_->profile(),
+        be_noisy_on_failure_);
+  }
 }
 
 }  // namespace extensions