Support asynchronous patching operations in the component updater.

chrome/browser/component_updater/component_unpacker.cc

Index: chrome/browser/component_updater/component_unpacker.cc
diff --git a/chrome/browser/component_updater/component_unpacker.cc b/chrome/browser/component_updater/component_unpacker.cc
index 5541a9b5d269281f871dbd99743aa75d7fe5de60..d0590a8c231153a7020512cd7263e9a6aee5cb00 100644
--- a/chrome/browser/component_updater/component_unpacker.cc
+++ b/chrome/browser/component_updater/component_unpacker.cc
@@ -7,7 +7,9 @@
 #include <string>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/file_util.h"
+#include "base/files/file_path.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/logging.h"
 #include "base/memory/scoped_handle.h"
@@ -17,6 +19,7 @@
 #include "chrome/browser/component_updater/component_patcher.h"
 #include "chrome/browser/component_updater/component_updater_service.h"
 #include "chrome/common/extensions/extension_constants.h"
+#include "content/public/browser/browser_thread.h"
 #include "crypto/secure_hash.h"
 #include "crypto/signature_verifier.h"
 #include "extensions/common/crx_file.h"
@@ -30,7 +33,7 @@ namespace {
 // and well formed.
 class CRXValidator {
  public:
-  explicit CRXValidator(FILE* crx_file) : valid_(false), delta_(false) {
+  explicit CRXValidator(FILE* crx_file) : valid_(false), is_delta_(false) {
     extensions::CrxFile::Header header;
     size_t len = fread(&header, 1, sizeof(header), crx_file);
     if (len < sizeof(header))
@@ -41,7 +44,7 @@ class CRXValidator {
         extensions::CrxFile::Parse(header, &error));
     if (!crx.get())
       return;
-    delta_ = extensions::CrxFile::HeaderIsDelta(header);
+    is_delta_ = extensions::CrxFile::HeaderIsDelta(header);
 
     std::vector<uint8> key(header.key_size);
     len = fread(&key[0], sizeof(uint8), header.key_size, crx_file);
@@ -77,17 +80,38 @@ class CRXValidator {
 
   bool valid() const { return valid_; }
 
-  bool delta() const { return delta_; }
+  bool is_delta() const { return is_delta_; }
 
   const std::vector<uint8>& public_key() const { return public_key_; }
 
  private:
   bool valid_;
-  bool delta_;
+  bool is_delta_;
   std::vector<uint8> public_key_;
 };
 
-}  // namespace.
+}  // namespace
+
+namespace component_updater {
+
+ComponentUnpacker::ComponentUnpacker(
+    const std::vector<uint8>& pk_hash,
+    const base::FilePath& path,
+    const std::string& fingerprint,
+    ComponentPatcher* patcher,
+    ComponentInstaller* installer,
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+    : pk_hash_(pk_hash),
+      path_(path),
+      is_delta_(false),
+      fingerprint_(fingerprint),
+      patcher_(patcher),
+      installer_(installer),
+      error_(kNone),
+      extended_error_(0),
+      ptr_factory_(this),
+      task_runner_(task_runner) {
+}
 
 // TODO(cpu): add a specific attribute check to a component json that the
 // extension unpacker will reject, so that a component cannot be installed
@@ -109,102 +133,142 @@ scoped_ptr<base::DictionaryValue> ReadManifest(
       static_cast<base::DictionaryValue*>(root.release())).Pass();
 }
 
-ComponentUnpacker::ComponentUnpacker(const std::vector<uint8>& pk_hash,
-                                     const base::FilePath& path,
-                                     const std::string& fingerprint,
-                                     ComponentPatcher* patcher,
-                                     ComponentInstaller* installer)
-    : error_(kNone),
-      extended_error_(0) {
-  if (pk_hash.empty() || path.empty()) {
-    error_ = kInvalidParams;
+// At the end of this execution, either Finish() will be called or
+// DonePatching() will be called asynchronously.
+void ComponentUnpacker::Unpack(
+    const base::Callback<void(Error, int)>& callback) {
+  callback_ = callback;
+  if (!Unzip()) {
+    Finish();
     return;
   }
+  if (!BeginPatching())
+    Finish();
+}
+
+bool ComponentUnpacker::Unzip() {
+  if (pk_hash_.empty() || path_.empty()) {
+    error_ = kInvalidParams;
+    return false;
+  }
   // First, validate the CRX header and signature. As of today
   // this is SHA1 with RSA 1024.
-  ScopedStdioHandle file(file_util::OpenFile(path, "rb"));
+  ScopedStdioHandle file(file_util::OpenFile(path_, "rb"));
   if (!file.get()) {
     error_ = kInvalidFile;
-    return;
+    return false;
   }
   CRXValidator validator(file.get());
   if (!validator.valid()) {
     error_ = kInvalidFile;
-    return;
+    return false;
   }
-  file.Close();
+  is_delta_ = validator.is_delta();
 
   // File is valid and the digital signature matches. Now make sure
   // the public key hash matches the expected hash. If they do we fully
   // trust this CRX.
-  uint8 hash[32];
+  uint8 hash[32] = {};
   scoped_ptr<SecureHash> sha256(SecureHash::Create(SecureHash::SHA256));
   sha256->Update(&(validator.public_key()[0]), validator.public_key().size());
   sha256->Finish(hash, arraysize(hash));
 
-  if (!std::equal(pk_hash.begin(), pk_hash.end(), hash)) {
+  if (!std::equal(pk_hash_.begin(), pk_hash_.end(), hash)) {
     error_ = kInvalidId;
-    return;
+    return false;
   }
+  base::FilePath& destination = is_delta_ ? unpack_diff_path_ : unpack_path_;
   if (!file_util::CreateNewTempDirectory(FILE_PATH_LITERAL(""),
-                                         &unpack_path_)) {
+                                         &destination)) {
     error_ = kUnzipPathError;
-    return;
+    return false;
   }
-  if (validator.delta()) {  // Package is a diff package.
-    // We want a different temp directory for the delta files; we'll put the
-    // patch output into unpack_path_.
-    base::FilePath unpack_diff_path;
+  file.Close();

[Sorin Jianu, 2013/11/27 00:34:01]

Is |file| used after we have validated it? If not,perhaps we could close it
sooner than later. We could also encapsulate the whole file verification in a
function. The idea is that we have this stray Close call in a middle of a
function.

[waffles, 2013/11/27 01:26:43]

Done.

+  if (!zip::Unzip(path_, destination))
+    error_ = kUnzipFailed;
+  return true;
+}
+
+
+bool ComponentUnpacker::BeginPatching() {
+  if (is_delta_) {  // Package is a diff package.

[Sorin Jianu, 2013/11/27 00:34:01]

comment is not needed.

[waffles, 2013/11/27 01:26:43]

Done.

+    // We want a different temp directory to put the patch output files into.
     if (!file_util::CreateNewTempDirectory(FILE_PATH_LITERAL(""),
-                                           &unpack_diff_path)) {
+                                           &unpack_path_)) {
       error_ = kUnzipPathError;
-      return;
-    }
-    if (!zip::Unzip(path, unpack_diff_path)) {
-      error_ = kUnzipFailed;
-      return;
-    }
-    ComponentUnpacker::Error result = DifferentialUpdatePatch(unpack_diff_path,
-                                                              unpack_path_,
-                                                              patcher,
-                                                              installer,
-                                                              &extended_error_);
-    base::DeleteFile(unpack_diff_path, true);
-    unpack_diff_path.clear();
-    error_ = result;
-    if (error_ != kNone) {
-      return;
+      return false;
     }
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&DifferentialUpdatePatch,
+                              unpack_diff_path_,
+                              unpack_path_,
+                              patcher_,
+                              installer_,
+                              base::Bind(&ComponentUnpacker::DonePatching,
+                                         GetWeakPtr())));
   } else {
-    // Package is a normal update/install; unzip it into unpack_path_ directly.
-    if (!zip::Unzip(path, unpack_path_)) {
-      error_ = kUnzipFailed;
-      return;
-    }
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&ComponentUnpacker::DonePatching,
+                              GetWeakPtr(),
+                              kNone,
+                              0));
   }
-  scoped_ptr<base::DictionaryValue> manifest(ReadManifest(unpack_path_));
-  if (!manifest.get()) {
-    error_ = kBadManifest;
+  return true;
+}
+
+void ComponentUnpacker::DonePatching(Error error, int extended_error) {
+  error_ = error;
+  extended_error_ = extended_error;
+  if (error_ != kNone) {
+    Finish();
     return;
   }
+  // Optimization: clean up patch files early, in case we're too low on disk to
+  // install otherwise.
+  if (!unpack_diff_path_.empty()) {
+    base::DeleteFile(unpack_diff_path_, true);
+    unpack_diff_path_.clear();
+  }
+  Install();
+  Finish();
+}
+
+void ComponentUnpacker::Install() {
   // Write the fingerprint to disk.
-  if (static_cast<int>(fingerprint.size()) !=
+  if (static_cast<int>(fingerprint_.size()) !=
       file_util::WriteFile(
           unpack_path_.Append(FILE_PATH_LITERAL("manifest.fingerprint")),
-          fingerprint.c_str(),
-          fingerprint.size())) {
+          fingerprint_.c_str(),
+          fingerprint_.size())) {
     error_ = kFingerprintWriteFailed;
     return;
   }
-  if (!installer->Install(*manifest, unpack_path_)) {
-    error_ = kInstallerError;
-    return;
+  if (error_ == kNone) {
+    scoped_ptr<base::DictionaryValue> manifest(ReadManifest(unpack_path_));
+    if (!manifest.get()) {
+      error_ = kBadManifest;
+      return;
+    }
+    if (!installer_->Install(*manifest, unpack_path_)) {
+      error_ = kInstallerError;
+      return;
+    }
   }
-  // Installation successful. The directory is not our concern now.
-  unpack_path_.clear();
 }
 
-ComponentUnpacker::~ComponentUnpacker() {
+void ComponentUnpacker::Finish() {
+  if (!unpack_diff_path_.empty())
+    base::DeleteFile(unpack_diff_path_, true);
   if (!unpack_path_.empty())
     base::DeleteFile(unpack_path_, true);
+  callback_.Run(error_, extended_error_);
+}
+
+base::WeakPtr<ComponentUnpacker> ComponentUnpacker::GetWeakPtr() {
+  return ptr_factory_.GetWeakPtr();
 }
+
+ComponentUnpacker::~ComponentUnpacker() {
+}
+
+}  // namespace component_updater