Use UtilityProcessHost to patch files.

chrome/browser/component_updater/component_patcher_operation.h

Index: chrome/browser/component_updater/component_patcher_operation.h
diff --git a/chrome/browser/component_updater/component_patcher_operation.h b/chrome/browser/component_updater/component_patcher_operation.h
index c9bc0b1690c3d62ccfb76499b3ddde8cf43ecf11..25ae07919ae369cdc316fbf885b5eaebeb0602c6 100644
--- a/chrome/browser/component_updater/component_patcher_operation.h
+++ b/chrome/browser/component_updater/component_patcher_operation.h
@@ -6,10 +6,15 @@
 #define CHROME_BROWSER_COMPONENT_UPDATER_COMPONENT_PATCHER_OPERATION_H_
 
 #include <string>
+
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/component_updater/component_patcher.h"
 #include "chrome/browser/component_updater/component_unpacker.h"
+#include "content/public/browser/utility_process_host_client.h"
 
 namespace base {
 class DictionaryValue;
@@ -18,29 +23,34 @@ class DictionaryValue;
 namespace component_updater {
 
 class ComponentInstaller;
-class ComponentPatcher;
 
-class DeltaUpdateOp {
+class DeltaUpdateOp : public base::RefCountedThreadSafe<DeltaUpdateOp> {
  public:
-
   DeltaUpdateOp();
-  virtual ~DeltaUpdateOp();
 
-  // Parses, runs, and verifies the operation, returning an error code if an
-  // error is encountered, and DELTA_OK otherwise. In case of errors,
-  // extended error information can be returned in the |error| parameter.
-  ComponentUnpacker::Error Run(base::DictionaryValue* command_args,
-                               const base::FilePath& input_dir,
-                               const base::FilePath& unpack_dir,
-                               ComponentPatcher* patcher,
-                               ComponentInstaller* installer,
-                               int* error);
+  // Parses, runs, and verifies the operation. Calls |callback| with the
+  // result of the operation. The callback is called using |task_runner|.
+  void Run(base::DictionaryValue* command_args,
+           const base::FilePath& input_dir,
+           const base::FilePath& unpack_dir,
+           ComponentInstaller* installer,
+           bool in_process,
+           const base::Callback<void(ComponentUnpacker::Error, int)>& callback,
+           scoped_refptr<base::SequencedTaskRunner> task_runner);
 
  protected:
+  typedef base::Callback<void(ComponentUnpacker::Error, int)> Callback;

[Sorin Jianu, 2014/02/27 20:53:57]

This type could be a typedef in ComponentUnpacker. This typedef here already has
a dependency on the ComponentUnpacker and this compilation unit is including the
header for that type. The idea is that there are too many of the callback types.
At the first glance, they all serve the same function: get called with an
unpacking error and an int.

[waffles, 2014/02/28 00:52:43]

Done.

+
+  virtual ~DeltaUpdateOp();
+
+  bool InProcess();

[Sorin Jianu, 2014/02/27 20:53:57]

Is there a way to remove some of the protected members here. For example,
InProcess could be public. This is not exposing any implementation detail since
in_process is an argument that the caller already knows about.

InProcess can be const.

[waffles, 2014/02/28 00:52:43]

Let's leave this one to a later CL if you don't mind tolerating it for a little
while longer.

+
   std::string output_sha256_;
   base::FilePath output_abs_path_;
 
  private:
+  friend class base::RefCountedThreadSafe<DeltaUpdateOp>;
+
   ComponentUnpacker::Error CheckHash();
 
   // Subclasses must override DoParseArguments to parse operation-specific
@@ -52,10 +62,18 @@ class DeltaUpdateOp {
       ComponentInstaller* installer) = 0;
 
   // Subclasses must override DoRun to actually perform the patching operation.
-  // DoRun returns DELTA_OK on success; any other code represents failure.
-  // Additional error information can be returned in the |error| parameter.
-  virtual ComponentUnpacker::Error DoRun(ComponentPatcher* patcher,
-                                         int* error) = 0;
+  // They must call the provided callback when they have completed their
+  // operations. In practice, the provided callback is always for "DoneRunning".
+  virtual void DoRun(const Callback& callback) = 0;
+
+  // Callback given to subclasses for when they complete their operation.
+  // Validates the output, and posts a task to the patching operation's
+  // callback.
+  void DoneRunning(ComponentUnpacker::Error error, int extended_error);
+
+  bool in_process_;
+  Callback callback_;
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(DeltaUpdateOp);
 };
@@ -67,6 +85,9 @@ class DeltaUpdateOpCopy : public DeltaUpdateOp {
  public:
   DeltaUpdateOpCopy();
 
+ protected:
+  virtual ~DeltaUpdateOpCopy();
+
  private:
   // Overrides of DeltaUpdateOp.
   virtual ComponentUnpacker::Error DoParseArguments(
@@ -74,8 +95,7 @@ class DeltaUpdateOpCopy : public DeltaUpdateOp {
       const base::FilePath& input_dir,
       ComponentInstaller* installer) OVERRIDE;
 
-  virtual ComponentUnpacker::Error DoRun(ComponentPatcher* patcher,
-                                         int* error) OVERRIDE;
+  virtual void DoRun(const Callback& callback) OVERRIDE;
 
   base::FilePath input_abs_path_;
 
@@ -90,6 +110,9 @@ class DeltaUpdateOpCreate : public DeltaUpdateOp {
  public:
   DeltaUpdateOpCreate();
 
+ protected:
+  virtual ~DeltaUpdateOpCreate();
+
  private:
   // Overrides of DeltaUpdateOp.
   virtual ComponentUnpacker::Error DoParseArguments(
@@ -97,62 +120,80 @@ class DeltaUpdateOpCreate : public DeltaUpdateOp {
       const base::FilePath& input_dir,
       ComponentInstaller* installer) OVERRIDE;
 
-  virtual ComponentUnpacker::Error DoRun(ComponentPatcher* patcher,
-                                         int* error) OVERRIDE;
+  virtual void DoRun(const Callback& callback) OVERRIDE;
 
   base::FilePath patch_abs_path_;
 
   DISALLOW_COPY_AND_ASSIGN(DeltaUpdateOpCreate);
 };
 
-// A 'bsdiff' operation takes an existing file on disk, and a bsdiff-
-// format patch file provided in the delta update package, and runs bsdiff
-// to construct an output file in the unpacking directory.
-class DeltaUpdateOpPatchBsdiff : public DeltaUpdateOp {
+class DeltaUpdateOpPatchTraits {

[Sorin Jianu, 2014/02/27 20:53:57]

Let's rename that Traits to something else, with the idea that the terminology
for traits in C++ is relevant to type traits. A couple of related patterns here
are Bridge or Strategy.

[waffles, 2014/02/28 00:52:43]

Done.

  public:
-  DeltaUpdateOpPatchBsdiff();
+  virtual ~DeltaUpdateOpPatchTraits();
 
- private:
-  // Overrides of DeltaUpdateOp.
-  virtual ComponentUnpacker::Error DoParseArguments(
-      base::DictionaryValue* command_args,
-      const base::FilePath& input_dir,
-      ComponentInstaller* installer) OVERRIDE;
+  // Returns an integer to add to error codes to disambiguate their source.

[Sorin Jianu, 2014/02/27 20:53:57]

can any of these be const?

[waffles, 2014/02/28 00:52:43]

Done.

+  virtual int GetErrorOffset() = 0;
 
-  virtual ComponentUnpacker::Error DoRun(ComponentPatcher* patcher,
-                                         int* error) OVERRIDE;
+  // Returns the "error code" that is expected in the successful install case.
+  virtual int GetSuccessCode() = 0;
 
-  base::FilePath patch_abs_path_;
-  base::FilePath input_abs_path_;
+  // Returns an IPC message that will start patching if it is sent to a
+  // UtilityProcessClient.
+  virtual IPC::Message* GetPatchMessage(base::FilePath input_abs_path,
+                                        base::FilePath patch_abs_path,
+                                        base::FilePath output_abs_path) = 0;
 
-  DISALLOW_COPY_AND_ASSIGN(DeltaUpdateOpPatchBsdiff);
+  // Does the actual patching operation, and returns an error code.
+  virtual int Patch(base::FilePath input_abs_path,
+                    base::FilePath patch_abs_path,
+                    base::FilePath output_abs_path) = 0;
 };
 
-// A 'courgette' operation takes an existing file on disk, and a Courgette-
-// format patch file provided in the delta update package, and runs Courgette
-// to construct an output file in the unpacking directory.
-class DeltaUpdateOpPatchCourgette : public DeltaUpdateOp {
+// Both 'bsdiff' and 'courgette' operations take an existing file on disk,
+// and a bsdiff- or Courgette-format patch file provided in the delta update
+// package, and run bsdiff or Courgette to construct an output file in the
+// unpacking directory.
+class DeltaUpdateOpPatch : public DeltaUpdateOp,
+                           public content::UtilityProcessHostClient {
  public:
-  DeltaUpdateOpPatchCourgette();
+  explicit DeltaUpdateOpPatch(scoped_ptr<DeltaUpdateOpPatchTraits> traits);
+
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;

[Sorin Jianu, 2014/02/27 20:53:57]

Can we make these two virtuals private and add the boilerplate comment
//Overrides of...

[waffles, 2014/02/28 00:52:43]

Done.

+
+  virtual void OnProcessCrashed(int exit_code) OVERRIDE;
+
+ protected:
+  virtual ~DeltaUpdateOpPatch();
 
  private:
+  void DonePatching(ComponentUnpacker::Error error, int error_code);
+
+  void OnPatchSucceeded();
+
+  void OnPatchFailed(int error_code);
+
   // Overrides of DeltaUpdateOp.
   virtual ComponentUnpacker::Error DoParseArguments(
       base::DictionaryValue* command_args,
       const base::FilePath& input_dir,
       ComponentInstaller* installer) OVERRIDE;
 
-  virtual ComponentUnpacker::Error DoRun(ComponentPatcher* patcher,
-                                         int* error) OVERRIDE;
+  virtual void DoRun(const Callback& callback) OVERRIDE;
+
+  void StartProcess();
 
+  Callback callback_;
   base::FilePath patch_abs_path_;
   base::FilePath input_abs_path_;
+  scoped_ptr<DeltaUpdateOpPatchTraits> traits_;
 
-  DISALLOW_COPY_AND_ASSIGN(DeltaUpdateOpPatchCourgette);
+  DISALLOW_COPY_AND_ASSIGN(DeltaUpdateOpPatch);
 };
 
-// Factory function to create DeltaUpdateOp instances.
-DeltaUpdateOp* CreateDeltaUpdateOp(base::DictionaryValue* command);
+// Factory functions to create DeltaUpdateOp instances.
+DeltaUpdateOp* CreateDeltaUpdateOp(const base::DictionaryValue& command);
+
+DeltaUpdateOp* CreateDeltaUpdateOp(const std::string& operation);
 
 }  // namespace component_updater