Adds asynchronous unzip functions to ZipReader

third_party/zlib/google/zip_reader.h

 // Copyright (c) 2011 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 THIRD_PARTY_ZLIB_GOOGLE_ZIP_READER_H_
 #define THIRD_PARTY_ZLIB_GOOGLE_ZIP_READER_H_
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/platform_file.h"
 #include "base/time/time.h"
 
 #if defined(USE_SYSTEM_MINIZIP)
 #include <minizip/unzip.h>
 #else
 #include "third_party/zlib/contrib/minizip/unzip.h"
 #endif
 
 namespace zip {
@@ skipping 45 matching lines @@
 
    private:
     const base::FilePath file_path_;
     int64 original_size_;
     base::Time last_modified_;
     bool is_directory_;
     bool is_unsafe_;
     DISALLOW_COPY_AND_ASSIGN(EntryInfo);
   };
 
+  class Listener : public base::RefCountedThreadSafe<Listener> {

[satorux1, 2013/12/05 04:39:37]

ref counted should usually be avoided, as it makes ownership of object unclear.

I think you wanted to use RefCounted so you can call the functions of Listener
without worrying about the Listener object is alive. Looking at operations.h,
Operation class is already RefCounted, which is IMOH unfortunate...

Here's what I'd do:

typedef base::Callback<void(int progress)> OnProgressCallback;

ExtractCurrentEntryToFilePathAsync(
  const base::FilePath& output_file_path,
  const base::Closure& on_success,
  const base::Closure& on_failure,
  const OnProgressCallback& on_progress);

This way, clients of the function can create callbacks with WeakPtr. These
callbacks are voided once the associated object is gone.

Taking three callbacks is somewhat awkward, but taking a delegate class
(Listener class in this case), tends to introduce a tricky object lifetime
issue, so I think this approach is reasonable.

[Drew Haven, 2013/12/09 23:33:12]

I really wasn't sure how to handle this one.  Explicit callback functions was my
initial thought, but I felt it would be nicer to bundle them together.  However,
then the object lifetime is a bit odd.  We don't really have a convention of
bundling together a set of callbacks?  I'd basically rather just make a named
tuple of callbacks.

I've split them out into individual arguments.

+   public:
+    // Reports progress information back to the caller.  It may or may not be
+    // called, but should be called approximately once per write-chunk or once
+    // per percent, whichever is less.  The progress is posted as an integer
+    // percentage.
+    virtual void OnUnzipProgress(int progress) = 0;
+
+    // Called when the operation completes.
+    virtual void OnUnzipSuccess() = 0;
+
+    // Called if there is an error.
+    virtual void OnUnzipFailed() = 0;
+   protected:
+    virtual ~Listener() {};
+    friend class base::RefCountedThreadSafe<Listener>;
+  };
+
   ZipReader();
   ~ZipReader();
 
   // Opens the zip file specified by |zip_file_path|. Returns true on
   // success.
   bool Open(const base::FilePath& zip_file_path);
 
   // Opens the zip file referred to by the platform file |zip_fd|.
   // Returns true on success.
   bool OpenFromPlatformFile(base::PlatformFile zip_fd);
@@ skipping 34 matching lines @@
   bool LocateAndOpenEntry(const base::FilePath& path_in_zip);
 
   // Extracts the current entry to the given output file path. If the
   // current file is a directory, just creates a directory
   // instead. Returns true on success. OpenCurrentEntryInZip() must be
   // called beforehand.
   //
   // This function does not preserve the timestamp of the original entry.
   bool ExtractCurrentEntryToFilePath(const base::FilePath& output_file_path);
 
+  // Asynchronously extracts the current entry to the given output file path.
+  // See ExtractCurrentEntryToFilePath for more information.
+  void ExtractCurrentEntryToFilePathAsync(
+      const base::FilePath& output_file_path,
+      scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
+      scoped_refptr<Listener> listener);
+
   // Extracts the current entry to the given output directory path using
   // ExtractCurrentEntryToFilePath(). Sub directories are created as needed
   // based on the file path of the current entry. For example, if the file
   // path in zip is "foo/bar.txt", and the output directory is "output",
   // "output/foo/bar.txt" will be created.
   //
   // Returns true on success. OpenCurrentEntryInZip() must be called
   // beforehand.
   bool ExtractCurrentEntryIntoDirectory(
       const base::FilePath& output_directory_path);
 
+  // Asynchronously extracts the current entry to the given output directory.
+  // See ExtractCurrentEntryIntoDirectory for more information.
+  void ExtractCurrentEntryIntoDirectoryAsync(
+      const base::FilePath& output_directory_path,
+      scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
+      scoped_refptr<Listener> listener);
+
+  // Asynchronously extracts the current entry to the given PlatformFile.  If
+  // the current entry is a directory this function will fail.  Does not close
+  // the PlatformFile.
+  void ExtractCurrentEntryToPlatformFileAsync(
+      base::PlatformFile output_file,
+      scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
+      scoped_refptr<Listener> listener);
+
 #if defined(OS_POSIX)
   // Extracts the current entry by writing directly to a file descriptor.
   // Does not close the file descriptor. Returns true on success.
   bool ExtractCurrentEntryToFd(int fd);
+
+  // Asynchronously extracts the current entry by writing directly to a file
+  // descriptor.
+  // See ExtractCurrentEntryToFd for more information.
+  void ExtractCurrentEntryToFdAsync(
+      int fd,
+      scoped_refptr<base::MessageLoopProxy> message_loop_proxy,

[satorux1, 2013/12/05 04:39:37]

Three functions were added:

ExtractCurrentEntryToFilePathAsync()
ExtractCurrentEntryIntoDirectoryAsync()
ExtractCurrentEntryToFdAsync()

Do you need all of them? Otherwise, please remove ones you don't need in the
spirit of YAGNI. Less things to maintain and test is nicer.

[Drew Haven, 2013/12/09 23:33:12]

A lot of them were basically "free" so I put them into mimic the existing
interface.  I took them out again, which does reduce the amount of code. 
Hopefully anyone in the future who needs a special version can figure out
quickly how to implement it.

+      scoped_refptr<Listener> listener);
 #endif
 
   // Returns the current entry info. Returns NULL if the current entry is
   // not yet opened. OpenCurrentEntryInZip() must be called beforehand.
   EntryInfo* current_entry_info() const {
     return current_entry_info_.get();
   }
 
   // Returns the number of entries in the zip file.
   // Open() must be called beforehand.
   int num_entries() const { return num_entries_; }
 
  private:
   // Common code used both in Open and OpenFromFd.
   bool OpenInternal();
 
   // Resets the internal state.
   void Reset();
 
+  // Unzip loop for asynchronous code.
+  void ExtractChunk(base::PlatformFile target_file,
+                    scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
+                    scoped_refptr<Listener> listener,
+                    int offset,
+                    int size);
+
   unzFile zip_file_;
   int num_entries_;
   bool reached_end_;
   scoped_ptr<EntryInfo> current_entry_info_;
 
+  base::WeakPtrFactory<ZipReader> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ZipReader);
 };
 
 }  // namespace zip
 
 #endif  // THIRD_PARTY_ZLIB_GOOGLE_ZIP_READER_H_