Significantly cleans up the ImageWriter Operation class and subclasses.

* Moved Operations to be in CPS for stage isolation.
* Updates unit tests to use isolated stages.
* Adds unit tests for uncovered stages.
* Cleans up temporary file handling.
* Removes unnecessary and redundant code (CompareFiles, ImageReader).
* Fixes bug with verification progress updates.
* All .zip files now unzipped before burning.
* All non-.zip files not unzipped before burning.
* Removes unnecessary complexity, e.g. unnecessary scoped_ptrs.

BUG=292956
BUG=337883
BUG=335404

Review URL: https://codereview.chromium.org/149313003

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

1 files changed, 44 insertions, 30 deletions

diff --git a/chrome/browser/extensions/api/image_writer_private/operation.h b/chrome/browser/extensions/api/image_writer_private/operation.h
index 58c0814..0ab4c78 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation.h
+++ b/chrome/browser/extensions/api/image_writer_private/operation.h
@@ -6,12 +6,12 @@
 #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/task/cancelable_task_tracker.h"
 #include "base/timer/timer.h"
-#include "chrome/browser/extensions/api/image_writer_private/image_writer_utils.h"
 #include "chrome/common/extensions/api/image_writer_private.h"
 #include "third_party/zlib/google/zip_reader.h"
 
@@ -46,10 +46,10 @@ 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;
+  void Start();
 
   // 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
@@ -66,6 +66,23 @@ class Operation : public base::RefCountedThreadSafe<Operation> {
  protected:
   virtual ~Operation();
 
+  // This function should be overriden by subclasses to set up the work of the
+  // operation.  It will be called from Start().
+  virtual void StartImpl() = 0;
+
+  // 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 +101,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,43 +110,45 @@ 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>;
 
   // 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(const int64& bytes_written,
+                  const int64& total_size,
+                  const base::Closure& continuation);
+  void WriteComplete(const base::Closure& continuation);
+
+  void VerifyWriteChunk(const int64& bytes_written,
+                        const int64& total_size,
+                        const base::Closure& continuation);
+  void VerifyWriteComplete(const base::Closure& continuation);
+
+  base::PlatformFile image_file_;
+  base::PlatformFile device_file_;
 #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 +158,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(const 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);