Convert utility process extension Unpacker IPC to mojo

extensions/browser/sandboxed_unpacker.h

Index: extensions/browser/sandboxed_unpacker.h
diff --git a/extensions/browser/sandboxed_unpacker.h b/extensions/browser/sandboxed_unpacker.h
index 47d07b27a537f023e24ff64c5a57b71cf4dd3b10..bb6d29bf29e3f5ee2189f30ca891c028109a5734 100644
--- a/extensions/browser/sandboxed_unpacker.h
+++ b/extensions/browser/sandboxed_unpacker.h
@@ -14,9 +14,10 @@
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/utility_process_host_client.h"
+#include "content/public/browser/utility_process_mojo_client.h"
 #include "extensions/browser/crx_file_info.h"
 #include "extensions/browser/install/crx_install_error.h"
+#include "extensions/common/extension_unpacker.mojom.h"
 #include "extensions/common/manifest.h"
 
 class SkBitmap;
@@ -26,10 +27,6 @@ class DictionaryValue;
 class SequencedTaskRunner;
 }
 
-namespace content {
-class UtilityProcessHost;
-}
-
 namespace extensions {
 class Extension;
 
@@ -63,7 +60,7 @@ class SandboxedUnpackerClient
   friend class base::RefCountedDeleteOnSequence<SandboxedUnpackerClient>;
   friend class base::DeleteHelper<SandboxedUnpackerClient>;
 
-  virtual ~SandboxedUnpackerClient() {}
+  virtual ~SandboxedUnpackerClient() = default;
 };
 
 // SandboxedUnpacker does work to optionally unpack and then validate/sanitize
@@ -80,15 +77,14 @@ class SandboxedUnpackerClient
 //
 // Lifetime management:
 //
-// This class is ref-counted by each call it makes to itself on another thread,
-// and by UtilityProcessHost.
+// This class is ref-counted by each call it makes to itself on another thread.
 //
 // Additionally, we hold a reference to our own client so that it lives at least
 // long enough to receive the result of unpacking.
 //
 //
 // NOTE: This class should only be used on the file thread.
-class SandboxedUnpacker : public content::UtilityProcessHostClient {
+class SandboxedUnpacker : public base::RefCountedThreadSafe<SandboxedUnpacker> {
  public:
   // Creates a SanboxedUnpacker that will do work to unpack an extension,
   // passing the |location| and |creation_flags| to Extension::Create. The
@@ -111,7 +107,7 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient {
                           const base::FilePath& directory);
 
  private:
-  class ProcessHostClient;
+  friend class base::RefCountedThreadSafe<SandboxedUnpacker>;
 
   // Enumerate all the ways unpacking can fail.  Calls to ReportFailure()
   // take a failure reason as an argument, and put it in histogram
@@ -125,14 +121,14 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient {
     FAILED_TO_COPY_EXTENSION_FILE_TO_TEMP_DIRECTORY,
     COULD_NOT_GET_SANDBOX_FRIENDLY_PATH,
 
-    // SandboxedUnpacker::OnUnpackExtensionSucceeded()
+    // SandboxedUnpacker::UnpackExtensionSucceeded()

[Devlin, 2017/02/14 17:24:59]

What's the motivation for changing these names?  OnFoo seems like it's just as
accurate or more accurate.

[Noel Gordon, 2017/02/15 17:28:08]

Short discussion about that in connection with a browser message filter
conversion example here:
https://www.chromium.org/developers/design-documents/mojo/mojo-migration-guide

TLDR: OnFoo-ness is the old message system.  Mojo is more orientated around
procedure calls (local or remote).  If I added a string copy service to mojo
say, I'd maybe name its API strcpy() rather than OnStrCpy.  We take out the
On's, but retain the postfix about which thread it runs on, if any.

     COULD_NOT_LOCALIZE_EXTENSION,
     INVALID_MANIFEST,
 
-    // SandboxedUnpacker::OnUnpackExtensionFailed()
+    // SandboxedUnpacker::UnpackExtensionFailed()
     UNPACKER_CLIENT_FAILED,
 
-    // SandboxedUnpacker::OnProcessCrashed()
+    // SandboxedUnpacker::UtilityProcessFailed()
     UTILITY_PROCESS_CRASHED_WHILE_TRYING_TO_INSTALL,
 
     // SandboxedUnpacker::ValidateSignature()
@@ -174,19 +170,17 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient {
 
     UNZIP_FAILED,
     DIRECTORY_MOVE_FAILED,
-    COULD_NOT_START_UTILITY_PROCESS,
 
     NUM_FAILURE_REASONS
   };
 
-  friend class ProcessHostClient;
   friend class SandboxedUnpackerTest;
 
-  ~SandboxedUnpacker() override;
+  ~SandboxedUnpacker();
 
   // Set |temp_dir_| as a temporary directory to unpack the extension in.
   // Return true on success.
-  virtual bool CreateTempDirectory();
+  bool CreateTempDirectory();
 
   // Helper functions to simplify calls to ReportFailure.
   base::string16 FailureReasonToString16(FailureReason reason);
@@ -197,22 +191,25 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient {
   bool ValidateSignature(const base::FilePath& crx_path,
                          const std::string& expected_hash);
 
-  void StartUnzipOnIOThread(const base::FilePath& crx_path);
-  void StartUnpackOnIOThread(const base::FilePath& directory_path);
+  // Ensures the utility process is created.
+  void StartUtilityProcess(const base::FilePath& directory);
+
+  // Utility process crashed or failed while trying to install.
+  void UtilityProcessCrashed();
 
-  // UtilityProcessHostClient
-  bool OnMessageReceived(const IPC::Message& message) override;
-  void OnProcessCrashed(int exit_code) override;
+  void UnzipOnIOThread(const base::FilePath& crx_path);
+  void UnzipDone(const base::FilePath& directory, bool success);
 
-  // IPC message handlers.
-  void OnUnzipToDirSucceeded(const base::FilePath& directory);
-  void OnUnzipToDirFailed(const std::string& error);
-  void OnUnpackExtensionSucceeded(const base::DictionaryValue& manifest);
-  void OnUnpackExtensionFailed(const base::string16& error_message);
+  void UnpackOnIOThread(const base::FilePath& directory);
+  void UnpackDone(const base::string16& error,
+                  std::unique_ptr<base::DictionaryValue> manifest);
+  void UnpackExtensionSucceeded(
+      std::unique_ptr<base::DictionaryValue> manifest);
+  void UnpackExtensionFailed(const base::string16& error);
 
-  void ReportFailure(FailureReason reason, const base::string16& message);
-  void ReportSuccess(const base::DictionaryValue& original_manifest,
+  void ReportSuccess(std::unique_ptr<base::DictionaryValue> original_manifest,
                      const SkBitmap& install_icon);
+  void ReportFailure(FailureReason reason, const base::string16& error);
 
   // Overwrites original manifest with safe result from utility process.
   // Returns NULL on error. Caller owns the returned object.
@@ -227,38 +224,6 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient {
   // Cleans up temp directory artifacts.
   void Cleanup();
 
-  // This is a helper class to make it easier to keep track of the lifecycle of
-  // a UtilityProcessHost, including automatic begin and end of batch mode.
-  class UtilityHostWrapper : public base::RefCountedThreadSafe<
-                                 UtilityHostWrapper,
-                                 content::BrowserThread::DeleteOnIOThread> {
-   public:
-    UtilityHostWrapper();
-
-    // Start up the utility process if it is not already started, putting it
-    // into batch mode and giving it access to |exposed_dir|. This should only
-    // be called on the IO thread. Returns false if there was an error starting
-    // the utility process or putting it into batch mode.
-    bool StartIfNeeded(
-        const base::FilePath& exposed_dir,
-        const scoped_refptr<UtilityProcessHostClient>& client,
-        const scoped_refptr<base::SequencedTaskRunner>& client_task_runner);
-
-    // This should only be called on the IO thread.
-    content::UtilityProcessHost* host() const;
-
-   private:
-    friend struct content::BrowserThread::DeleteOnThread<
-        content::BrowserThread::IO>;
-    friend class base::DeleteHelper<UtilityHostWrapper>;
-    ~UtilityHostWrapper();
-
-    // Should only be used on the IO thread.
-    base::WeakPtr<content::UtilityProcessHost> utility_host_;
-
-    DISALLOW_COPY_AND_ASSIGN(UtilityHostWrapper);
-  };
-
   // If we unpacked a crx file, we hold on to the path for use in various
   // histograms.
   base::FilePath crx_path_for_histograms_;
@@ -278,9 +243,6 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient {
   // Represents the extension we're unpacking.
   scoped_refptr<Extension> extension_;
 
-  // Whether we've received a response from the utility process yet.
-  bool got_response_;
-
   // The public key that was extracted from the CRX header.
   std::string public_key_;
 
@@ -299,11 +261,13 @@ class SandboxedUnpacker : public content::UtilityProcessHostClient {
   // when calling Extenion::Create() by the crx installer.
   int creation_flags_;
 
-  // Sequenced task runner where file I/O operations will be performed at.
+  // Sequenced task runner where file I/O operations will be performed.
   scoped_refptr<base::SequencedTaskRunner> unpacker_io_task_runner_;
 
-  // Used for sending tasks to the utility process.
-  scoped_refptr<UtilityHostWrapper> utility_wrapper_;
+  // Utility client used for sending tasks to the utility process.
+  std::unique_ptr<
+      content::UtilityProcessMojoClient<extensions::mojom::ExtensionUnpacker>>
+      utility_process_mojo_client_;
 
   DISALLOW_COPY_AND_ASSIGN(SandboxedUnpacker);
 };