Significantly cleans up the ImageWriter Operation class and subclasses.

chrome/browser/extensions/api/image_writer_private/operation.h

Index: chrome/browser/extensions/api/image_writer_private/operation.h
diff --git a/chrome/browser/extensions/api/image_writer_private/operation.h b/chrome/browser/extensions/api/image_writer_private/operation.h
index 80092cfa59a807f4e683a458ef76a47a531aef52..15374d211d18e82b6885cfefbcc8c1da529438f1 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation.h
+++ b/chrome/browser/extensions/api/image_writer_private/operation.h
@@ -6,11 +6,11 @@
 #define CHROME_BROWSER_EXTENSIONS_API_IMAGE_WRITER_PRIVATE_OPERATION_H_
 
 #include "base/callback.h"
+#include "base/files/scoped_temp_dir.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"
 #include "chrome/common/extensions/api/image_writer_private.h"
 #include "third_party/zlib/google/zip_reader.h"
@@ -46,7 +46,7 @@ class Operation : public base::RefCountedThreadSafe<Operation> {
 
   Operation(base::WeakPtr<OperationManager> manager,
             const ExtensionId& extension_id,
-            const std::string& storage_unit_id);
+            const std::string& device_path);
 
   // Starts the operation.
   virtual void Start() = 0;
@@ -66,6 +66,19 @@ class Operation : public base::RefCountedThreadSafe<Operation> {
  protected:
   virtual ~Operation();
 
+  // Unzips the current file if it ends in ".zip".  The current_file will be set
+  // to the unzipped file.
+  void Unzip(const base::Closure& continuation);
+
+  // Writes the current file to device_path.
+  void Write(const base::Closure& continuation);
+
+  // Verifies that the current file and device_path contents match.
+  void VerifyWrite(const base::Closure& continuation);
+
+  // Completes the operation.
+  void Finish();
+
   // Generates an error.
   // |error_message| is used to create an OnWriteError event which is
   // sent to the extension
@@ -84,12 +97,7 @@ class Operation : public base::RefCountedThreadSafe<Operation> {
   // 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();
-  void Finish();
+  void AddCleanUpFunction(const base::Closure& callback);
 
   // If |file_size| is non-zero, only |file_size| bytes will be read from file,
   // otherwise the entire file will be read.
@@ -98,20 +106,20 @@ class Operation : public base::RefCountedThreadSafe<Operation> {
   // sum.  |progress_offset| is an percentage that will be added to the progress
   // of the MD5 sum before updating |progress_| but after scaling.
   void GetMD5SumOfFile(
-      scoped_ptr<base::FilePath> file,
+      const base::FilePath file,
       int64 file_size,
       int progress_offset,
       int progress_scale,
-      const base::Callback<void(scoped_ptr<std::string>)>& callback);
+      const base::Callback<void(const std::string&)>& callback);
 
   base::WeakPtr<OperationManager> manager_;
   const ExtensionId extension_id_;
 
   base::FilePath image_path_;
-  const std::string storage_unit_id_;
+  base::FilePath device_path_;
 
-  // Whether or not to run the final verification step.
-  bool verify_write_;
+  // Temporary directory to store files as we go.
+  base::ScopedTempDir temp_dir_;
 
  private:
   friend class base::RefCountedThreadSafe<Operation>;
@@ -119,22 +127,25 @@ class Operation : public 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,
-                    scoped_ptr<image_writer_utils::ImageWriter> writer);
-  void WriteComplete();
-
-  void VerifyWriteStage2(scoped_ptr<std::string> image_hash);
-  void VerifyWriteCompare(scoped_ptr<std::string> image_hash,
-                          scoped_ptr<std::string> device_hash);
+  void WriteChunk(base::PlatformFile source,
+                  base::PlatformFile target,
+                  const int64& bytes_written,
+                  const int64& total_size,
+                  const base::Closure& continuation);
+  void WriteComplete(const base::Closure& continuation);
+
+  void VerifyWriteStage2(const base::Closure& continuation,

[tbarzic, 2014/02/07 22:06:42]

'Stage2' is a bit cryptic.. Maybe VerifyWriteGetDeviceHash would be better?

[Drew Haven, 2014/02/11 00:50:15]

Yeah, that's an artifact from the initial prototype.  Also, it's a little silly
that we're using MD5 hashes to verify when we can do an incremental comparison,
so I'm just going to mirror the write behavior here.

+                         const std::string& image_hash);
+  void VerifyWriteCompare(const base::Closure& continuation,

[tbarzic, 2014/02/07 22:06:42]

suggestion: append 'Hashes' to the name.

[Drew Haven, 2014/02/11 00:50:15]

See above

+                          const std::string& image_hash,
+                          const std::string& device_hash);
 #endif
 
 #if defined(OS_CHROMEOS)
-  void StartWriteOnUIThread();
+  void StartWriteOnUIThread(const base::Closure& continuation);
 
-  void OnBurnFinished(const std::string& target_path,
+  void OnBurnFinished(const base::Closure& continuation,
+                      const std::string& target_path,
                       bool success,
                       const std::string& error);
   void OnBurnProgress(const std::string& target_path,
@@ -144,15 +155,15 @@ class Operation : public base::RefCountedThreadSafe<Operation> {
 #endif
 
   // Incrementally calculates the MD5 sum of a file.
-  void MD5Chunk(scoped_ptr<image_writer_utils::ImageReader> reader,
+  void MD5Chunk(base::PlatformFile file,
                 int64 bytes_processed,
                 int64 bytes_total,
                 int progress_offset,
                 int progress_scale,
-                const base::Callback<void(scoped_ptr<std::string>)>& callback);
+                const base::Callback<void(const std::string&)>& callback);
 
   // Callbacks for zip::ZipReader.
-  void OnUnzipSuccess();
+  void OnUnzipSuccess(const base::Closure& continuation);
   void OnUnzipFailure();
   void OnUnzipProgress(int64 total_bytes, int64 progress_bytes);