Adds asynchronous unzip functions to ZipReader

third_party/zlib/google/zip_reader.h

Index: third_party/zlib/google/zip_reader.h
diff --git a/third_party/zlib/google/zip_reader.h b/third_party/zlib/google/zip_reader.h
index a1f470421525197a68b04bc3bd10a2d8257f467c..9cc5e38202c5da117af78446afe671728d6c154a 100644
--- a/third_party/zlib/google/zip_reader.h
+++ b/third_party/zlib/google/zip_reader.h
@@ -10,7 +10,10 @@
 #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"
 
@@ -76,6 +79,24 @@ class ZipReader {
     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();
 
@@ -130,6 +151,13 @@ class ZipReader {
   // 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
@@ -141,10 +169,33 @@ class ZipReader {
   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
@@ -164,11 +215,20 @@ class ZipReader {
   // 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);
 };