Implements Image Writer API for ChromeOS.

BUG=279327

Review URL: https://chromiumcodereview.appspot.com/23785007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224553 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 326 insertions, 142 deletions

diff --git a/chrome/browser/extensions/api/image_writer_private/error_messages.cc b/chrome/browser/extensions/api/image_writer_private/error_messages.cc
index ad92476..5b7060d9 100644
--- a/chrome/browser/extensions/api/image_writer_private/error_messages.cc
+++ b/chrome/browser/extensions/api/image_writer_private/error_messages.cc
@@ -19,6 +19,7 @@ const char kDownloadInterrupted[] = "Download interrupted.";
 const char kDownloadMD5[] = "Failed to calculate MD5 sum for download.";
 const char kEmptyUnzip[] = "Unzipped image contained no files.";
 const char kFileOperationsNotImplemented[] = "Write-from file not implemented.";
+const char kImageBurnerError[] = "Error contacting Image Burner process.";
 const char kImageMD5[] = "Failed to calculate MD5 sum for unzipped image.";
 const char kImageNotFound[] = "Unpacked image not found.";
 const char kImageSize[] = "Could not determine image size.";
diff --git a/chrome/browser/extensions/api/image_writer_private/error_messages.h b/chrome/browser/extensions/api/image_writer_private/error_messages.h
index 0042426..a45c775 100644
--- a/chrome/browser/extensions/api/image_writer_private/error_messages.h
+++ b/chrome/browser/extensions/api/image_writer_private/error_messages.h
@@ -20,6 +20,7 @@ extern const char kDownloadInterrupted[];
 extern const char kDownloadMD5[];
 extern const char kEmptyUnzip[];
 extern const char kFileOperationsNotImplemented[];
+extern const char kImageBurnerError[];
 extern const char kImageMD5[];
 extern const char kImageNotFound[];
 extern const char kImageSize[];
diff --git a/chrome/browser/extensions/api/image_writer_private/image_writer_private_api.cc b/chrome/browser/extensions/api/image_writer_private/image_writer_private_api.cc
index 0fb3b97..a9834be 100644
--- a/chrome/browser/extensions/api/image_writer_private/image_writer_private_api.cc
+++ b/chrome/browser/extensions/api/image_writer_private/image_writer_private_api.cc
@@ -30,10 +30,16 @@ bool ImageWriterPrivateWriteFromUrlFunction::RunImpl() {
     return false;
   }
 
+#if defined(OS_CHROMEOS)
+  // The Chrome OS temporary partition is too small for Chrome OS images, thus
+  // we must always use the downloads folder.
+  bool save_image_as_download = true;
+#else
   bool save_image_as_download = false;
   if (params->options.get() && params->options->save_as_download.get()) {
     save_image_as_download = true;
   }
+#endif
 
   std::string hash;
   if (params->options.get() && params->options->image_hash.get()) {
diff --git a/chrome/browser/extensions/api/image_writer_private/operation.cc b/chrome/browser/extensions/api/image_writer_private/operation.cc
index 510030c..711badb 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation.cc
+++ b/chrome/browser/extensions/api/image_writer_private/operation.cc
@@ -16,10 +16,17 @@ namespace image_writer {
 
 using content::BrowserThread;
 
-const int kBurningBlockSize = 8 * 1024;  // 8 KiB
+namespace {
+
 const int kMD5BufferSize = 1024;
 
-Operation::Operation(OperationManager* manager,
+void RemoveTempDirectory(const base::FilePath path) {
+  base::DeleteFile(path, true);
+}
+
+}  // namespace
+
+Operation::Operation(base::WeakPtr<OperationManager> manager,
                      const ExtensionId& extension_id,
                      const std::string& storage_unit_id)
     : manager_(manager),
@@ -36,55 +43,129 @@ void Operation::Cancel() {
   DVLOG(1) << "Cancelling image writing operation for ext: " << extension_id_;
 
   stage_ = image_writer_api::STAGE_NONE;
+
+  CleanUp();
 }
 
 void Operation::Abort() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   Error(error::kAborted);
 }
 
 void Operation::Error(const std::string& error_message) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
+    BrowserThread::PostTask(BrowserThread::FILE,
+                            FROM_HERE,
+                            base::Bind(&Operation::Error, this, error_message));
+    return;
+  }
+
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&OperationManager::OnError,
-                 manager_->AsWeakPtr(),
+                 manager_,
                  extension_id_,
                  stage_,
                  progress_,
                  error_message));
+
+  CleanUp();
 }
 
-void Operation::SendProgress() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+void Operation::SetProgress(int progress) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(&Operation::SetProgress,
+                   this,
+                   progress));
+    return;
+  }
+
+  if (IsCancelled()) {
+    return;
+  }
+
+  progress_ = progress;
+
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&OperationManager::OnProgress,
-                 manager_->AsWeakPtr(),
+                 manager_,
+                 extension_id_,
+                 stage_,
+                 progress_));
+}
+
+void Operation::SetStage(image_writer_api::Stage stage) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(&Operation::SetStage,
+                   this,
+                   stage));
+    return;
+  }
+
+  if (IsCancelled()) {
+    return;
+  }
+
+  stage_ = stage;
+  progress_ = 0;
+
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&OperationManager::OnProgress,
+                 manager_,
                  extension_id_,
                  stage_,
                  progress_));
 }
 
 void Operation::Finish() {
+  if (BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
+    BrowserThread::PostTask(BrowserThread::FILE,
+                            FROM_HERE,
+                            base::Bind(&Operation::Finish, this));
+  }
   DVLOG(1) << "Write operation complete.";
 
-  stage_ = image_writer_api::STAGE_NONE;
-  progress_ = 0;
+  CleanUp();
 
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&OperationManager::OnComplete,
-                 manager_->AsWeakPtr(),
+                 manager_,
                  extension_id_));
 }
 
 bool Operation::IsCancelled() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
   return stage_ == image_writer_api::STAGE_NONE;
 }
 
+void Operation::AddCleanUpFunction(base::Closure callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  cleanup_functions_.push_back(callback);
+}
+
+void Operation::CleanUp() {
+  for (std::vector<base::Closure>::iterator it = cleanup_functions_.begin();
+       it != cleanup_functions_.end();
+       ++it) {
+    it->Run();
+  }
+  cleanup_functions_.clear();
+}
+
 void Operation::UnzipStart(scoped_ptr<base::FilePath> zip_file) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   if (IsCancelled()) {
@@ -93,17 +174,18 @@ void Operation::UnzipStart(scoped_ptr<base::FilePath> zip_file) {
 
   DVLOG(1) << "Starting unzip stage for " << zip_file->value();
 
-  stage_ = image_writer_api::STAGE_UNZIP;
-  progress_ = 0;
-
-  SendProgress();
+  SetStage(image_writer_api::STAGE_UNZIP);
 
   base::FilePath tmp_dir;
-  if (!file_util::CreateNewTempDirectory(FILE_PATH_LITERAL(""), &tmp_dir)) {
+  if (!file_util::CreateTemporaryDirInDir(zip_file->DirName(),
+                                          FILE_PATH_LITERAL("image_writer"),
+                                          &tmp_dir)) {
     Error(error::kTempDir);
     return;
   }
 
+  AddCleanUpFunction(base::Bind(&RemoveTempDirectory, tmp_dir));
+
   if (!zip::Unzip(*zip_file, tmp_dir)) {
     Error(error::kUnzip);
     return;
@@ -128,8 +210,7 @@ void Operation::UnzipStart(scoped_ptr<base::FilePath> zip_file) {
 
   DVLOG(1) << "Successfully unzipped as " << unzipped_file->value();
 
-  progress_ = 100;
-  SendProgress();
+  SetProgress(kProgressComplete);
 
   image_path_ = *unzipped_file;
 
@@ -197,14 +278,13 @@ void Operation::MD5Chunk(
 
   if (len > 0) {
     base::MD5Update(&md5_context_, base::StringPiece(buffer, len));
-    int percent_prev = (bytes_processed * progress_scale + progress_offset)
-                       / (bytes_total);
-    int percent_curr = ((bytes_processed + len) * progress_scale
-                        + progress_offset) / (bytes_total);
-    progress_ = percent_curr;
-
+    int percent_prev = (bytes_processed * progress_scale + progress_offset) /
+                       (bytes_total);
+    int percent_curr = ((bytes_processed + len) * progress_scale +
+                        progress_offset) /
+                       (bytes_total);
     if (percent_curr > percent_prev) {
-      SendProgress();
+      SetProgress(progress_);
     }
 
     BrowserThread::PostTask(BrowserThread::FILE,
@@ -234,5 +314,5 @@ void Operation::MD5Chunk(
   }
 }
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
diff --git a/chrome/browser/extensions/api/image_writer_private/operation.h b/chrome/browser/extensions/api/image_writer_private/operation.h
index 5cc06f2..17a88e9 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation.h
+++ b/chrome/browser/extensions/api/image_writer_private/operation.h
@@ -8,6 +8,7 @@
 #include "base/callback.h"
 #include "base/md5.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/extensions/api/image_writer_private/image_writer_utils.h"
 #include "chrome/common/cancelable_task_tracker.h"
@@ -16,14 +17,14 @@
 namespace image_writer_api = extensions::api::image_writer_private;
 
 namespace base {
-
 class FilePath;
-
-} // namespace base
+}  // namespace base
 
 namespace extensions {
 namespace image_writer {
 
+const int kProgressComplete = 100;
+
 class OperationManager;
 
 // Encapsulates an operation being run on behalf of the
@@ -43,7 +44,7 @@ class Operation
   typedef base::Callback<void(bool, const std::string&)> CancelWriteCallback;
   typedef std::string ExtensionId;
 
-  Operation(OperationManager* manager,
+  Operation(base::WeakPtr<OperationManager> manager,
             const ExtensionId& extension_id,
             const std::string& storage_unit_id);
 
@@ -52,12 +53,11 @@ class Operation
 
   // Cancel the operation. This must be called to clean up internal state and
   // cause the the operation to actually stop.  It will not be destroyed until
-  // all callbacks have completed.  This method may be overridden to provide
-  // subclass cancelling.  It should still call the superclass method.
-  virtual void Cancel();
+  // all callbacks have completed.
+  void Cancel();
 
   // Aborts the operation, cancelling it and generating an error.
-  virtual void Abort();
+  void Abort();
  protected:
   virtual ~Operation();
 
@@ -65,12 +65,21 @@ class Operation
   // |error_message| is used to create an OnWriteError event which is
   // sent to the extension
   virtual void Error(const std::string& error_message);
-  // Sends a progress notification.
-  void SendProgress();
+
+  // Set |progress_| and send an event.  Progress should be in the interval
+  // [0,100]
+  void SetProgress(int progress);
+  // Change to a new |stage_| and set |progress_| to zero.
+  void SetStage(image_writer_api::Stage stage);
 
   // Can be queried to safely determine if the operation has been cancelled.
   bool IsCancelled();
 
+  // Adds a callback that will be called during clean-up, whether the operation
+  // is aborted, encounters and error, or finishes successfully.  These
+  // functions will be run on the FILE thread.
+  void AddCleanUpFunction(base::Closure);
+
   void UnzipStart(scoped_ptr<base::FilePath> zip_file);
   void WriteStart();
   void VerifyWriteStart();
@@ -89,34 +98,21 @@ class Operation
       int progress_scale,
       const base::Callback<void(scoped_ptr<std::string>)>& callback);
 
-  OperationManager* manager_;
+  base::WeakPtr<OperationManager> manager_;
   const ExtensionId extension_id_;
 
-  // This field is owned by the UI thread.
-  image_writer_api::Stage stage_;
-
-  // The amount of work completed for the current stage as a percentage.  This
-  // variable may be modified by both the FILE and UI threads, but it is only
-  // modified by the basic activity flow of the operation which is logically
-  // single threaded.  Progress events will read this variable from the UI
-  // thread which has the potential to read a value newer than when the event
-  // was posted.  However, the stage is never advanced except on the UI thread
-  // and so will be later in the queue than those progress events.  Thus
-  // progress events may report higher progress numbers than when queued, but
-  // will never report the wrong stage and appear to move backwards.
-  int progress_;
-
   base::FilePath image_path_;
   const std::string storage_unit_id_;
  private:
   friend class base::RefCountedThreadSafe<Operation>;
 
+  // TODO(haven): Clean up these switches. http://crbug.com/292956
 #if defined(OS_LINUX) && !defined(CHROMEOS)
   void WriteRun();
   void WriteChunk(scoped_ptr<image_writer_utils::ImageReader> reader,
                   scoped_ptr<image_writer_utils::ImageWriter> writer,
                   int64 bytes_written);
-  bool WriteCleanup(scoped_ptr<image_writer_utils::ImageReader> reader,
+  bool WriteCleanUp(scoped_ptr<image_writer_utils::ImageReader> reader,
                     scoped_ptr<image_writer_utils::ImageWriter> writer);
   void WriteComplete();
 
@@ -125,6 +121,18 @@ class Operation
                           scoped_ptr<std::string> device_hash);
 #endif
 
+#if defined(OS_CHROMEOS)
+  void StartWriteOnUIThread();
+
+  void OnBurnFinished(const std::string& target_path,
+                      bool success,
+                      const std::string& error);
+  void OnBurnProgress(const std::string& target_path,
+                      int64 num_bytes_burnt,
+                      int64 total_size);
+  void OnBurnError();
+#endif
+
   void MD5Chunk(scoped_ptr<image_writer_utils::ImageReader> reader,
                 int64 bytes_processed,
                 int64 bytes_total,
@@ -132,13 +140,24 @@ class Operation
                 int progress_scale,
                 const base::Callback<void(scoped_ptr<std::string>)>& callback);
 
+  // Runs all cleanup functions.
+  void CleanUp();
+
+  // |stage_| and |progress_| are owned by the FILE thread, use |SetStage| and
+  // |SetProgress| to update.  Progress should be in the interval [0,100]
+  image_writer_api::Stage stage_;
+  int progress_;
+
   // MD5 contexts don't play well with smart pointers.  Just going to allocate
   // memory here.  This requires that we only do one MD5 sum at a time.
   base::MD5Context md5_context_;
 
+  // CleanUp operations that must be run.  All these functions are run on the
+  // FILE thread.
+  std::vector<base::Closure> cleanup_functions_;
 };
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_IMAGE_WRITER_PRIVATE_OPERATION_H_
diff --git a/chrome/browser/extensions/api/image_writer_private/operation_chromeos.cc b/chrome/browser/extensions/api/image_writer_private/operation_chromeos.cc
index 5aeb7f5..067614d 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation_chromeos.cc
+++ b/chrome/browser/extensions/api/image_writer_private/operation_chromeos.cc
@@ -4,17 +4,84 @@
 
 #include "chrome/browser/extensions/api/image_writer_private/error_messages.h"
 #include "chrome/browser/extensions/api/image_writer_private/operation.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/image_burner_client.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace extensions {
 namespace image_writer {
 
+using chromeos::ImageBurnerClient;
+using content::BrowserThread;
+
+namespace {
+
+void ClearImageBurner() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  chromeos::DBusThreadManager::Get()->
+      GetImageBurnerClient()->
+      ResetEventHandlers();
+}
+
+void CleanUpImageBurner() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  BrowserThread::PostTask(BrowserThread::UI,
+                          FROM_HERE,
+                          base::Bind(&ClearImageBurner));
+}
+
+}  // namespace
+
 void Operation::WriteStart() {
-  Error(error::kUnsupportedOperation);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  SetStage(image_writer_api::STAGE_WRITE);
+
+  BrowserThread::PostTask(BrowserThread::UI,
+                          FROM_HERE,
+                          base::Bind(&Operation::StartWriteOnUIThread, this));
+
+  AddCleanUpFunction(base::Bind(&CleanUpImageBurner));
+}
+
+void Operation::StartWriteOnUIThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DVLOG(1) << "Starting burn.";
+
+  ImageBurnerClient* burner =
+      chromeos::DBusThreadManager::Get()->GetImageBurnerClient();
+
+  burner->SetEventHandlers(
+      base::Bind(&Operation::OnBurnFinished, this),
+      base::Bind(&Operation::OnBurnProgress, this));
+
+  burner->BurnImage(image_path_.value(),
+                    storage_unit_id_,
+                    base::Bind(&Operation::OnBurnError, this));
+}
+
+void Operation::OnBurnFinished(const std::string& target_path,
+                    bool success,
+                    const std::string& error) {
+  DVLOG(1) << "Burn finished: " << success;
+
+  if (success) {
+    SetProgress(kProgressComplete);
+    Finish();
+  } else {
+    Error(error);
+  }
+}
+
+void Operation::OnBurnProgress(const std::string& target_path,
+                    int64 num_bytes_burnt,
+                    int64 total_size) {
+  int progress = kProgressComplete * num_bytes_burnt / total_size;
+  SetProgress(progress);
 }
 
-void Operation::VerifyWriteStart() {
-  Error(error::kUnsupportedOperation);
+void Operation::OnBurnError() {
+  Error(error::kImageBurnerError);
 }
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
diff --git a/chrome/browser/extensions/api/image_writer_private/operation_linux.cc b/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
index 1945053..8250363 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
+++ b/chrome/browser/extensions/api/image_writer_private/operation_linux.cc
@@ -32,9 +32,7 @@ void Operation::WriteStart() {
   DVLOG(1) << "Starting write of " << image_path_.value()
            << " to " << storage_unit_id_;
 
-  stage_ = image_writer_api::STAGE_WRITE;
-  progress_ = 0;
-  SendProgress();
+  SetStage(image_writer_api::STAGE_WRITE);
 
   // TODO (haven): Unmount partitions before writing. http://crbug.com/284834
 
@@ -70,7 +68,7 @@ void Operation::WriteChunk(
     scoped_ptr<image_writer_utils::ImageWriter> writer,
     int64 bytes_written) {
   if (IsCancelled()) {
-    WriteCleanup(reader.Pass(), writer.Pass());
+    WriteCleanUp(reader.Pass(), writer.Pass());
     return;
   }
 
@@ -80,12 +78,11 @@ void Operation::WriteChunk(
 
   if (len > 0) {
     if (writer->Write(buffer, len) == len) {
-      int percent_prev = bytes_written * 100 / image_size;
-      int percent_curr = (bytes_written + len) * 100 / image_size;
-      progress_ = (bytes_written + len) * 100 / image_size;
+      int percent_prev = kProgressComplete * bytes_written / image_size;
+      int percent_curr = kProgressComplete * (bytes_written + len) / image_size;
 
       if (percent_curr > percent_prev) {
-        SendProgress();
+        SetProgress(percent_curr);
       }
 
       BrowserThread::PostTask(
@@ -97,12 +94,12 @@ void Operation::WriteChunk(
                    base::Passed(&writer),
                    bytes_written + len));
     } else {
-      WriteCleanup(reader.Pass(), writer.Pass());
+      WriteCleanUp(reader.Pass(), writer.Pass());
       Error(error::kWriteImage);
     }
   } else if (len == 0) {
     if (bytes_written == image_size) {
-      if (WriteCleanup(reader.Pass(), writer.Pass())) {
+      if (WriteCleanUp(reader.Pass(), writer.Pass())) {
         BrowserThread::PostTask(
           BrowserThread::FILE,
           FROM_HERE,
@@ -110,16 +107,16 @@ void Operation::WriteChunk(
                      this));
       }
     } else {
-      WriteCleanup(reader.Pass(), writer.Pass());
+      WriteCleanUp(reader.Pass(), writer.Pass());
       Error(error::kPrematureEndOfFile);
     }
   } else { // len < 0
-    WriteCleanup(reader.Pass(), writer.Pass());
+    WriteCleanUp(reader.Pass(), writer.Pass());
     Error(error::kReadImage);
   }
 }
 
-bool Operation::WriteCleanup(
+bool Operation::WriteCleanUp(
     scoped_ptr<image_writer_utils::ImageReader> reader,
     scoped_ptr<image_writer_utils::ImageWriter> writer) {
 
@@ -139,8 +136,7 @@ bool Operation::WriteCleanup(
 void Operation::WriteComplete() {
 
   DVLOG(2) << "Completed write of " << image_path_.value();
-  progress_ = 100;
-  SendProgress();
+  SetProgress(kProgressComplete);
 
   BrowserThread::PostTask(
     BrowserThread::FILE,
@@ -157,9 +153,7 @@ void Operation::VerifyWriteStart() {
 
   DVLOG(1) << "Starting verification stage.";
 
-  stage_ = image_writer_api::STAGE_VERIFYWRITE;
-  progress_ = 0;
-  SendProgress();
+  SetStage(image_writer_api::STAGE_VERIFYWRITE);
 
   scoped_ptr<base::FilePath> image_path(new base::FilePath(image_path_));
 
@@ -206,10 +200,10 @@ void Operation::VerifyWriteCompare(
 
   DVLOG(2) << "Completed write verification of " << image_path_.value();
 
-  progress_ = 100;
-  SendProgress();
+  SetProgress(kProgressComplete);
+
   Finish();
 }
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
diff --git a/chrome/browser/extensions/api/image_writer_private/operation_manager.cc b/chrome/browser/extensions/api/image_writer_private/operation_manager.cc
index 3f34675..c6d8c85 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation_manager.cc
+++ b/chrome/browser/extensions/api/image_writer_private/operation_manager.cc
@@ -27,7 +27,8 @@ namespace image_writer {
 using content::BrowserThread;
 
 OperationManager::OperationManager(Profile* profile)
-    : profile_(profile) {
+    : profile_(profile),
+      weak_factory_(this) {
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
                  content::Source<Profile>(profile_));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
@@ -71,8 +72,12 @@ void OperationManager::StartWriteFromUrl(
   }
 
   scoped_refptr<Operation> operation(
-      new WriteFromUrlOperation(this, extension_id, rvh, url,
-                                hash, saveImageAsDownload,
+      new WriteFromUrlOperation(weak_factory_.GetWeakPtr(),
+                                extension_id,
+                                rvh,
+                                url,
+                                hash,
+                                saveImageAsDownload,
                                 storage_unit_id));
 
   operations_[extension_id] = operation;
@@ -101,6 +106,9 @@ void OperationManager::CancelWrite(
   if (existing_operation == NULL) {
     callback.Run(false, error::kNoOperationInProgress);
   } else {
+    BrowserThread::PostTask(BrowserThread::FILE,
+                            FROM_HERE,
+                            base::Bind(&Operation::Cancel, existing_operation));
     DeleteOperation(extension_id);
     callback.Run(true, "");
   }
@@ -148,7 +156,6 @@ void OperationManager::OnError(const ExtensionId& extension_id,
   DLOG(ERROR) << "ImageWriter error: " << error_message;
 
   // TODO(haven): Set up error messages. http://crbug.com/284880
-
   info.stage = stage;
   info.percent_complete = progress;
 
@@ -174,10 +181,6 @@ Operation* OperationManager::GetOperation(const ExtensionId& extension_id) {
 void OperationManager::DeleteOperation(const ExtensionId& extension_id) {
   OperationMap::iterator existing_operation = operations_.find(extension_id);
   if (existing_operation != operations_.end()) {
-    BrowserThread::PostTask(BrowserThread::FILE,
-                            FROM_HERE,
-                            base::Bind(&Operation::Cancel,
-                                       existing_operation->second));
     operations_.erase(existing_operation);
   }
 }
@@ -229,5 +232,5 @@ ProfileKeyedAPIFactory<OperationManager>*
 }
 
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
diff --git a/chrome/browser/extensions/api/image_writer_private/operation_manager.h b/chrome/browser/extensions/api/image_writer_private/operation_manager.h
index f959745..d801e86 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation_manager.h
+++ b/chrome/browser/extensions/api/image_writer_private/operation_manager.h
@@ -102,10 +102,12 @@ class OperationManager
   OperationMap operations_;
   content::NotificationRegistrar registrar_;
 
+  base::WeakPtrFactory<OperationManager> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(OperationManager);
 };
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_IMAGE_WRITER_PRIVATE_OPERATION_MANAGER_H_
diff --git a/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.cc b/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.cc
index 73ecece..ddcf032 100644
--- a/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.cc
+++ b/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.cc
@@ -10,7 +10,7 @@ namespace extensions {
 namespace image_writer {
 
 WriteFromFileOperation::WriteFromFileOperation(
-    OperationManager* manager,
+    base::WeakPtr<OperationManager> manager,
     const ExtensionId& extension_id,
     const base::FilePath& path,
     const std::string& storage_unit_id)
@@ -25,5 +25,5 @@ void WriteFromFileOperation::Start() {
   Error(error::kUnsupportedOperation);
 }
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
diff --git a/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.h b/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.h
index 478ae85..702bdf2 100644
--- a/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.h
+++ b/chrome/browser/extensions/api/image_writer_private/write_from_file_operation.h
@@ -13,7 +13,7 @@ namespace image_writer {
 // Encapsulates a write of an image from a local file.
 class WriteFromFileOperation : public Operation {
  public:
-  WriteFromFileOperation(OperationManager* manager,
+  WriteFromFileOperation(base::WeakPtr<OperationManager> manager,
                          const ExtensionId& extension_id,
                          const base::FilePath& path,
                          const std::string& storage_unit_id);
@@ -24,7 +24,7 @@ class WriteFromFileOperation : public Operation {
   const base::FilePath path_;
 };
 
-} // namespace image_writer
-} // namespace extensions
+}  // namespace image_writer
+}  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_IMAGE_WRITER_PRIVATE_WRITE_FROM_FILE_OPERATION_H_
diff --git a/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
index ad446ea..dec61bf 100644
--- a/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
+++ b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.cc
@@ -21,7 +21,7 @@ namespace image_writer {
 using content::BrowserThread;
 
 WriteFromUrlOperation::WriteFromUrlOperation(
-    OperationManager* manager,
+    base::WeakPtr<OperationManager> manager,
     const ExtensionId& extension_id,
     content::RenderViewHost* rvh,
     GURL url,
@@ -33,20 +33,17 @@ WriteFromUrlOperation::WriteFromUrlOperation(
       url_(url),
       hash_(hash),
       saveImageAsDownload_(saveImageAsDownload),
-      download_(NULL){
+      download_stopped_(false),
+      download_(NULL) {
 }
 
 WriteFromUrlOperation::~WriteFromUrlOperation() {
-  if (stage_ == image_writer_api::STAGE_DOWNLOAD && download_) {
-    download_->RemoveObserver(this);
-    download_->Cancel(false);
-  }
 }
+
 void WriteFromUrlOperation::Start() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  stage_ = image_writer_api::STAGE_DOWNLOAD;
-  progress_ = 0;
+  SetStage(image_writer_api::STAGE_DOWNLOAD);
 
   if (saveImageAsDownload_){
     BrowserThread::PostTask(
@@ -59,16 +56,8 @@ void WriteFromUrlOperation::Start() {
         FROM_HERE,
         base::Bind(&WriteFromUrlOperation::CreateTempFile, this));
   }
-}
 
-void WriteFromUrlOperation::Cancel() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  if (stage_ == image_writer_api::STAGE_DOWNLOAD && download_) {
-    download_->RemoveObserver(this);
-    download_->Cancel(true);
-  }
-
-  Operation::Cancel();
+  AddCleanUpFunction(base::Bind(&WriteFromUrlOperation::DownloadCleanUp, this));
 }
 
 void WriteFromUrlOperation::CreateTempFile() {
@@ -91,14 +80,12 @@ void WriteFromUrlOperation::CreateTempFile() {
 // The downloader runs on the UI thread.
 void WriteFromUrlOperation::DownloadStart() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DVLOG(1) << "Starting download of URL: " << url_;
 
-  if (IsCancelled()) {
+  if (download_stopped_) {
     return;
   }
 
-  stage_ = image_writer_api::STAGE_DOWNLOAD;
-  progress_ = 0;
+  DVLOG(1) << "Starting download of URL: " << url_;
 
   Profile* current_profile = manager_->profile();
 
@@ -124,6 +111,14 @@ void WriteFromUrlOperation::DownloadStart() {
 void WriteFromUrlOperation::OnDownloadStarted(content::DownloadItem* item,
                                               net::Error error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (download_stopped_) {
+    // At this point DownloadCleanUp was called but the |download_| wasn't
+    // stored yet and still hasn't been cancelled.
+    item->Cancel(true);
+    return;
+  }
+
   if (item) {
     DCHECK_EQ(net::OK, error);
 
@@ -142,20 +137,14 @@ void WriteFromUrlOperation::OnDownloadStarted(content::DownloadItem* item,
 }
 
 // Always called from the UI thread.
-void WriteFromUrlOperation::OnDownloadUpdated(
-  content::DownloadItem* download) {
+void WriteFromUrlOperation::OnDownloadUpdated(content::DownloadItem* download) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (IsCancelled()) {
-    download->Cancel(false);
+  if (download_stopped_) {
     return;
   }
 
-  progress_ = download->PercentComplete();
-
-  BrowserThread::PostTask(
-      BrowserThread::FILE,
-      FROM_HERE,
-      base::Bind(&WriteFromUrlOperation::SendProgress, this));
+  SetProgress(download->PercentComplete());
 
   if (download->GetState() == content::DownloadItem::COMPLETE) {
     download_path_ = download_->GetTargetFilePath();
@@ -176,15 +165,35 @@ void WriteFromUrlOperation::OnDownloadUpdated(
 }
 
 void WriteFromUrlOperation::DownloadComplete() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DVLOG(1) << "Download complete.";
 
-  progress_ = 100;
-  SendProgress();
+  SetProgress(kProgressComplete);
 
   VerifyDownloadStart();
 }
 
+void WriteFromUrlOperation::DownloadCleanUp() {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&WriteFromUrlOperation::DownloadCleanUp, this));
+    return;
+  }
+
+  download_stopped_ = true;
+
+  if (download_) {
+    download_->RemoveObserver(this);
+    download_->Cancel(true);
+    download_ = NULL;
+  }
+}
+
 void WriteFromUrlOperation::VerifyDownloadStart() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
   if (IsCancelled()) {
     return;
   }
@@ -199,10 +208,7 @@ void WriteFromUrlOperation::VerifyDownloadStart() {
 
   DVLOG(1) << "Download verification started.";
 
-  stage_ = image_writer_api::STAGE_VERIFYDOWNLOAD;
-  progress_ = 0;
-
-  SendProgress();
+  SetStage(image_writer_api::STAGE_VERIFYDOWNLOAD);
 
   BrowserThread::PostTask(
       BrowserThread::FILE,
@@ -211,17 +217,19 @@ void WriteFromUrlOperation::VerifyDownloadStart() {
 }
 
 void WriteFromUrlOperation::VerifyDownloadRun() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   scoped_ptr<base::FilePath> download_path(new base::FilePath(download_path_));
   GetMD5SumOfFile(
       download_path.Pass(),
       0,
       0,
-      100,
+      kProgressComplete,
       base::Bind(&WriteFromUrlOperation::VerifyDownloadCompare, this));
 }
 
 void WriteFromUrlOperation::VerifyDownloadCompare(
     scoped_ptr<std::string> download_hash) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   if (*download_hash != hash_) {
     Error(error::kDownloadHash);
     return;
@@ -234,14 +242,14 @@ void WriteFromUrlOperation::VerifyDownloadCompare(
 }
 
 void WriteFromUrlOperation::VerifyDownloadComplete() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   if (IsCancelled()) {
     return;
   }
 
   DVLOG(1) << "Download verification complete.";
 
-  progress_ = 100;
-  SendProgress();
+  SetProgress(kProgressComplete);
 
   scoped_ptr<base::FilePath> download_path(new base::FilePath(download_path_));
   UnzipStart(download_path.Pass());
diff --git a/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h
index 8ae0aa2..95e5574 100644
--- a/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h
+++ b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h
@@ -26,14 +26,13 @@ class OperationManager;
 class WriteFromUrlOperation : public Operation,
                               public content::DownloadItem::Observer {
  public:
-  WriteFromUrlOperation(OperationManager* manager,
+  WriteFromUrlOperation(base::WeakPtr<OperationManager> manager,
                         const ExtensionId& extension_id,
                         content::RenderViewHost* rvh,
                         GURL url,
                         const std::string& hash,
                         bool saveImageAsDownload,
                         const std::string& storage_unit_id);
-  virtual void Cancel() OVERRIDE;
   virtual void Start() OVERRIDE;
  private:
   virtual ~WriteFromUrlOperation();
@@ -43,19 +42,23 @@ class WriteFromUrlOperation : public Operation,
   void OnDownloadStarted(content::DownloadItem*, net::Error);
   virtual void OnDownloadUpdated(content::DownloadItem* download) OVERRIDE;
   void DownloadComplete();
+  void DownloadCleanUp();
 
   void VerifyDownloadStart();
   void VerifyDownloadRun();
   void VerifyDownloadCompare(scoped_ptr<std::string> download_hash);
   void VerifyDownloadComplete();
 
+  // Arguments
   content::RenderViewHost* rvh_;
   GURL url_;
   const std::string hash_;
   const bool saveImageAsDownload_;
+
+  // Local state
   scoped_ptr<base::FilePath> tmp_file_;
+  bool download_stopped_;
   content::DownloadItem* download_;
-
   base::FilePath download_path_;
 };