Revert of Use UtilityProcessHost to patch files. (https://codereview.…

…chromium.org/25909005/) Reason for revert: linux asan failure http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%2BLSan%20Tests%20%282%29/builds/901/steps/unit_tests/logs/DifferentialUpdateFails Original issue's description: > Use UtilityProcessHost to patch files. > > Second part of a 2-part changelist. > The first part is https://codereview.chromium.org/25883006/ > > BUG=304879 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=259726 TBR=sorin@chromium.org,dgarrett@chromium.org,sky@chromium.org,cdn@chromium.org,waffles@chromium.org NOTREECHECKS=true NOTRY=true BUG=304879 Review URL: https://codereview.chromium.org/213923002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259740 0039d316-1c4b-4281-b951-d872f2087c98
hylschwy · Mar 26, 2014 · 785c71f · 785c71f
1 parent 3bfbd89
commit 785c71f
Show file tree

Hide file tree

Showing 26 changed files with 535 additions and 680 deletions.
diff --git a/chrome/browser/DEPS b/chrome/browser/DEPS
@@ -40,7 +40,6 @@ include_rules = [
   "+components/webdata/common",
   "+content/public/browser",
   "+content/test/net",
-  "+courgette",
   "+device/bluetooth",
   "+device/media_transfer_protocol",
   "+extensions/browser",

diff --git a/chrome/browser/component_updater/DEPS b/chrome/browser/component_updater/DEPS
@@ -1,5 +1,4 @@
 include_rules = [
-  "+courgette",
   "+media/cdm/ppapi/supported_cdm_versions.h",
   "+ppapi/shared_impl/ppapi_permissions.h",
   "+ppapi/thunk",

diff --git a/chrome/browser/component_updater/component_patcher.cc b/chrome/browser/component_updater/component_patcher.cc
@@ -7,15 +7,11 @@
 #include <string>
 #include <vector>
 
-#include "base/basictypes.h"
 #include "base/file_util.h"
-#include "base/files/file_path.h"
 #include "base/json/json_file_value_serializer.h"
-#include "base/memory/weak_ptr.h"
 #include "base/values.h"
 #include "chrome/browser/component_updater/component_patcher_operation.h"
 #include "chrome/browser/component_updater/component_updater_service.h"
-#include "content/public/browser/browser_thread.h"
 
 namespace component_updater {
 
@@ -38,83 +34,58 @@ base::ListValue* ReadCommands(const base::FilePath& unpack_path) {
 
 }  // namespace
 
-ComponentPatcher::ComponentPatcher(
-    const base::FilePath& input_dir,
-    const base::FilePath& unpack_dir,
-    ComponentInstaller* installer,
-    bool in_process,
-    scoped_refptr<base::SequencedTaskRunner> task_runner)
-    : input_dir_(input_dir),
-      unpack_dir_(unpack_dir),
-      installer_(installer),
-      in_process_(in_process),
-      task_runner_(task_runner) {
-}
+// The patching support is not cross-platform at the moment.
+ComponentPatcherCrossPlatform::ComponentPatcherCrossPlatform() {}
 
-ComponentPatcher::~ComponentPatcher() {
+ComponentUnpacker::Error ComponentPatcherCrossPlatform::Patch(
+    PatchType patch_type,
+    const base::FilePath& input_file,
+    const base::FilePath& patch_file,
+    const base::FilePath& output_file,
+    int* error) {
+  return ComponentUnpacker::kDeltaUnsupportedCommand;
 }
 
-void ComponentPatcher::Start(const ComponentUnpacker::Callback& callback) {
-  callback_ = callback;
-  task_runner_->PostTask(FROM_HERE,
-                         base::Bind(&ComponentPatcher::StartPatching,
-                                    scoped_refptr<ComponentPatcher>(this)));
-}
 
-void ComponentPatcher::StartPatching() {
-  commands_.reset(ReadCommands(input_dir_));
-  if (!commands_.get()) {
-    DonePatching(ComponentUnpacker::kDeltaBadCommands, 0);
-  } else {
-    next_command_ = commands_->begin();
-    PatchNextFile();
-  }
-}
-
-void ComponentPatcher::PatchNextFile() {
-  if (next_command_ == commands_->end()) {
-    DonePatching(ComponentUnpacker::kNone, 0);
-    return;
-  }
-  if (!(*next_command_)->IsType(base::Value::TYPE_DICTIONARY)) {
-    DonePatching(ComponentUnpacker::kDeltaBadCommands, 0);
-    return;
-  }
-  const base::DictionaryValue* command_args =
-      static_cast<base::DictionaryValue*>(*next_command_);
-  current_operation_ = CreateDeltaUpdateOp(*command_args);
-  if (!current_operation_) {
-    DonePatching(ComponentUnpacker::kDeltaUnsupportedCommand, 0);
+// Takes the contents of a differential component update in input_dir
+// and produces the contents of a full component update in unpack_dir
+// using input_abs_path_ files that the installer knows about.
+void DifferentialUpdatePatch(
+    const base::FilePath& input_dir,
+    const base::FilePath& unpack_dir,
+    ComponentPatcher* patcher,
+    ComponentInstaller* installer,
+    base::Callback<void(ComponentUnpacker::Error, int)> callback) {
+  int error = 0;
+  scoped_ptr<base::ListValue> commands(ReadCommands(input_dir));
+  if (!commands.get()) {
+    callback.Run(ComponentUnpacker::kDeltaBadCommands, error);
     return;
   }
-  current_operation_->Run(
-      command_args,
-      input_dir_,
-      unpack_dir_,
-      installer_,
-      in_process_,
-      base::Bind(&ComponentPatcher::DonePatchingFile,
-                 scoped_refptr<ComponentPatcher>(this)),
-      task_runner_);
-}
 
-void ComponentPatcher::DonePatchingFile(ComponentUnpacker::Error error,
-                                        int extended_error) {
-  if (error != ComponentUnpacker::kNone) {
-    DonePatching(error, extended_error);
-  } else {
-    ++next_command_;
-    PatchNextFile();
+  for (base::ValueVector::const_iterator command = commands->begin(),
+      end = commands->end(); command != end; command++) {
+    if (!(*command)->IsType(base::Value::TYPE_DICTIONARY)) {
+      callback.Run(ComponentUnpacker::kDeltaBadCommands, error);
+      return;
+    }
+    base::DictionaryValue* command_args =
+        static_cast<base::DictionaryValue*>(*command);
+    scoped_ptr<DeltaUpdateOp> operation(CreateDeltaUpdateOp(command_args));
+    if (!operation) {
+      callback.Run(ComponentUnpacker::kDeltaUnsupportedCommand, error);
+      return;
+    }
+
+    ComponentUnpacker::Error result = operation->Run(
+        command_args, input_dir, unpack_dir, patcher, installer, &error);
+    if (result != ComponentUnpacker::kNone) {
+      callback.Run(result, error);
+      return;
+    }
   }
-}
 
-void ComponentPatcher::DonePatching(ComponentUnpacker::Error error,
-                                    int extended_error) {
-  current_operation_ = NULL;
-  task_runner_->PostTask(FROM_HERE, base::Bind(callback_,
-                                               error,
-                                               extended_error));
-  callback_.Reset();
+  callback.Run(ComponentUnpacker::kNone, error);
 }
 
 }  // namespace component_updater
diff --git a/chrome/browser/component_updater/component_patcher.h b/chrome/browser/component_updater/component_patcher.h
@@ -28,9 +28,9 @@
 #ifndef CHROME_BROWSER_COMPONENT_UPDATER_COMPONENT_PATCHER_H_
 #define CHROME_BROWSER_COMPONENT_UPDATER_COMPONENT_PATCHER_H_
 
+#include "base/basictypes.h"
 #include "base/callback_forward.h"
-#include "base/memory/ref_counted.h"
-#include "base/values.h"
+#include "base/compiler_specific.h"
 #include "chrome/browser/component_updater/component_unpacker.h"
 
 namespace base {
@@ -40,61 +40,53 @@ class FilePath;
 namespace component_updater {
 
 class ComponentInstaller;
-class DeltaUpdateOp;
 
-// The type of a patch file.
-enum PatchType {
-  kPatchTypeUnknown,
-  kPatchTypeCourgette,
-  kPatchTypeBsdiff,
+// Applies a delta patch to a single file. Specifically, creates a file at
+// |output_file| using |input_file| patched according to the algorithm
+// specified by |patch_type| using |patch_file|. Sets the value of error to
+// the error code of the failing patch operation, if there is such a failure.
+class ComponentPatcher {
+ public:
+  // The type of a patch file.
+  enum PatchType {
+    kPatchTypeUnknown,
+    kPatchTypeCourgette,
+    kPatchTypeBsdiff,
+  };
+
+  virtual ComponentUnpacker::Error Patch(PatchType patch_type,
+                                         const base::FilePath& input_file,
+                                         const base::FilePath& patch_file,
+                                         const base::FilePath& output_file,
+                                         int* error) = 0;
+  virtual ~ComponentPatcher() {}
 };
 
-// Encapsulates a task for applying a differential update to a component.
-class ComponentPatcher : public base::RefCountedThreadSafe<ComponentPatcher> {
+class ComponentPatcherCrossPlatform : public ComponentPatcher {
  public:
-  // Takes an unpacked differential CRX (|input_dir|) and a component installer,
-  // and sets up the class to create a new (non-differential) unpacked CRX.
-  // If |in_process| is true, patching will be done completely within the
-  // existing process. Otherwise, some steps of patching may be done
-  // out-of-process.
-  ComponentPatcher(const base::FilePath& input_dir,
-                   const base::FilePath& unpack_dir,
-                   ComponentInstaller* installer,
-                   bool in_process,
-                   scoped_refptr<base::SequencedTaskRunner> task_runner);
-
-  // Starts patching files. This member function returns immediately, after
-  // posting a task to do the patching. When patching has been completed,
-  // |callback| will be called with the error codes if any error codes were
-  // encountered.
-  void Start(const ComponentUnpacker::Callback& callback);
-
+  ComponentPatcherCrossPlatform();
+  virtual ComponentUnpacker::Error Patch(PatchType patch_type,
+                                         const base::FilePath& input_file,
+                                         const base::FilePath& patch_file,
+                                         const base::FilePath& output_file,
+                                         int* error) OVERRIDE;
  private:
-  friend class base::RefCountedThreadSafe<ComponentPatcher>;
-
-  virtual ~ComponentPatcher();
-
-  void StartPatching();
-
-  void PatchNextFile();
-
-  void DonePatchingFile(ComponentUnpacker::Error error, int extended_error);
-
-  void DonePatching(ComponentUnpacker::Error error, int extended_error);
-
-  const base::FilePath input_dir_;
-  const base::FilePath unpack_dir_;
-  ComponentInstaller* const installer_;
-  const bool in_process_;
-  ComponentUnpacker::Callback callback_;
-  scoped_ptr<base::ListValue> commands_;
-  base::ValueVector::const_iterator next_command_;
-  scoped_refptr<DeltaUpdateOp> current_operation_;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
-
-  DISALLOW_COPY_AND_ASSIGN(ComponentPatcher);
+  DISALLOW_COPY_AND_ASSIGN(ComponentPatcherCrossPlatform);
 };
 
+// This function takes an unpacked differential CRX (|input_dir|) and a
+// component installer, and creates a new (non-differential) unpacked CRX, which
+// is then installed normally.
+// The non-differential files are written into the |unpack_dir| directory.
+// When finished, calls the callback, passing error codes if any errors were
+// encountered.
+void DifferentialUpdatePatch(
+    const base::FilePath& input_dir,
+    const base::FilePath& unpack_dir,
+    ComponentPatcher* component_patcher,
+    ComponentInstaller* installer,
+    base::Callback<void(ComponentUnpacker::Error, int)> callback);
+
 }  // namespace component_updater
 
 #endif  // CHROME_BROWSER_COMPONENT_UPDATER_COMPONENT_PATCHER_H_