Significantly cleans up the ImageWriter Operation class and subclasses.

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_API_IMAGE_WRITER_PRIVATE_OPERATION_H_
 #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"
 
 namespace image_writer_api = extensions::api::image_writer_private;
 
 namespace base {
 class FilePath;
 }  // namespace base
 
@@ skipping 15 matching lines @@
 // for advancing to the next stage and other UI interaction.  The Run phase does
 // the work on the FILE thread and calls SendProgress or Error as appropriate.
 class Operation : public base::RefCountedThreadSafe<Operation> {
  public:
   typedef base::Callback<void(bool, const std::string&)> StartWriteCallback;
   typedef base::Callback<void(bool, const std::string&)> CancelWriteCallback;
   typedef std::string ExtensionId;
 
   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;
 
   // 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.
   void Cancel();
 
   // Aborts the operation, cancelling it and generating an error.
   void Abort();
 
   // Informational getters.
   int GetProgress();
   image_writer_api::Stage GetStage();
 
  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
   virtual void Error(const std::string& error_message);
 
   // 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.  Triggers a progress
   // event.
   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();
-  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.
   // |progress_scale| is a percentage to which the progress will be scale, e.g.
   // a scale of 50 means it will increment from 0 to 50 over the course of the
   // 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>;
 
   // 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 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(scoped_ptr<std::string> image_hash);
-  void VerifyWriteCompare(scoped_ptr<std::string> image_hash,
-                          scoped_ptr<std::string> device_hash);
+  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,
                       int64 num_bytes_burnt,
                       int64 total_size);
   void OnBurnError();
 #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);
 
   // 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_;
 
   // Zip reader for unzip operations.
   zip::ZipReader zip_reader_;
 
   // 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
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_IMAGE_WRITER_PRIVATE_OPERATION_H_