Adds asynchronous unzip functions to ZipReader

third_party/zlib/google/zip_reader_unittest.cc

 // 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.
 
 #include "third_party/zlib/google/zip_reader.h"
 
 #include <set>
 #include <string>
 
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
 #include "base/md5.h"
 #include "base/path_service.h"
 #include "base/platform_file.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 #include "third_party/zlib/google/zip_internal.h"
 
 namespace {
 
+const static std::string kQuuxExpectedMD5 = "d1ae4ac8a17a0e09317113ab284b57a6";
+
 // Wrap PlatformFiles in a class so that we don't leak them in tests.
 class PlatformFileWrapper {
  public:
   typedef enum {
     READ_ONLY,
     READ_WRITE
   } AccessMode;
 
   PlatformFileWrapper(const base::FilePath& file, AccessMode mode)
       : file_(base::kInvalidPlatformFileValue) {
@@ skipping 20 matching lines @@
   ~PlatformFileWrapper() {
     base::ClosePlatformFile(file_);
   }
 
   base::PlatformFile platform_file() { return file_; }
 
  private:
   base::PlatformFile file_;
 };
 
+class MockUnzipListener : public base::SupportsWeakPtr<MockUnzipListener> {

[satorux1, 2013/12/11 06:45:11]

Please add a class comment.

[Drew Haven, 2013/12/11 18:20:53]

Done.

+ public:
+  MockUnzipListener() 
+      : success_calls(0),
+        failure_calls(0),
+        progress_calls(0),
+        current_progress(0) {
+  }
+
+  void OnUnzipSuccess() {

[satorux1, 2013/12/11 06:45:11]

function comment is missing. maybe something like

// Used as success_callback for ExtractCurrentEntryToFilePathAsync()

[Drew Haven, 2013/12/11 18:20:53]

Done.

+    success_calls++;
+  }
+
+  void OnUnzipFailure() {
+    failure_calls++;
+  }
+
+  void OnUnzipProgress(int64 progress) {
+    progress_calls++;
+    EXPECT_GT(progress, current_progress);

[satorux1, 2013/12/11 06:45:11]

This may be a matter of taste, but I think EXPECT* should usually only be used
in TEST functions (logic: test failures from EXPECT* in random places are hard
to debug, hence should usually be avoided).

Here, maybe you can make it a DCHECK.

[Drew Haven, 2013/12/11 18:20:53]

True.  At least a DCHECK gives you a nice stack trace.

+    current_progress = progress;
+  }
+
+  int success_calls;

[satorux1, 2013/12/11 06:45:11]

success_calls_

[Drew Haven, 2013/12/11 18:20:53]

Done.

+  int failure_calls;
+  int progress_calls;
+
+  int64 current_progress;

[satorux1, 2013/12/11 06:45:11]

member variables should be private. Please add getters for them.

[Drew Haven, 2013/12/11 18:20:53]

Done.

I'm really confused why this is preferred over an eight-line GMock instance. 
Granted, with the new setup GMock wouldn't work as well, but assuming we were
still making a listener subclass.

[satorux1, 2013/12/12 08:13:01]

Non-GMock code is a bit more verbose but anyone can read. GMock version is more
succinct but harder to read for someone who's not familiar with GMock. I think
it's a reasonable choice not to use GMock in a project where not all folks are
familiar with GMock and some folks feel very strongly about use of GMock. :)

+};
+
 }   // namespace
 
 namespace zip {
 
 // Make the test a PlatformTest to setup autorelease pools properly on Mac.
 class ZipReaderTest : public PlatformTest {
  protected:
   virtual void SetUp() {
     PlatformTest::SetUp();
 
@@ skipping 32 matching lines @@
     if (!success)
       return false;
     *path = path->AppendASCII("third_party");
     *path = path->AppendASCII("zlib");
     *path = path->AppendASCII("google");
     *path = path->AppendASCII("test");
     *path = path->AppendASCII("data");
     return true;
   }
 
+  void CompareFileAndMD5(const base::FilePath& path,
+                         const std::string expected_md5) {
+    // Read the output file and compute the MD5.
+    std::string output;
+    ASSERT_TRUE(base::ReadFileToString(path, &output));
+    const std::string md5 = base::MD5String(output);
+    EXPECT_EQ(expected_md5, md5);

[satorux1, 2013/12/11 06:45:11]

Instead of calling ASSERT and EXPECT here, please return a boolean and do

EXPECT_TRUE(CompareFileAndMD5(...);

This way is nicer because the line number in the test failure report will point
us to the call site of CompareFileAndMD5(). In this file, there is only one call
site of CompareFileAndMD5(), but if there are many, the line number info is very
helpful.

[Drew Haven, 2013/12/11 18:20:53]

Done.

+  }
+
   // The path to temporary directory used to contain the test operations.
   base::FilePath test_dir_;
   // The path to the test data directory where test.zip etc. are located.
   base::FilePath test_data_dir_;
   // The path to test.zip in the test data directory.
   base::FilePath test_zip_file_;
   // The path to evil.zip in the test data directory.
   base::FilePath evil_zip_file_;
   // The path to evil_via_invalid_utf8.zip in the test data directory.
   base::FilePath evil_via_invalid_utf8_zip_file_;
   // The path to evil_via_absolute_file_name.zip in the test data directory.
   base::FilePath evil_via_absolute_file_name_zip_file_;
   std::set<base::FilePath> test_zip_contents_;
 
   base::ScopedTempDir temp_dir_;
+
+  base::MessageLoop message_loop_;
 };
 
 TEST_F(ZipReaderTest, Open_ValidZipFile) {
   ZipReader reader;
   ASSERT_TRUE(reader.Open(test_zip_file_));
 }
 
 TEST_F(ZipReaderTest, Open_ValidZipPlatformFile) {
   ZipReader reader;
   PlatformFileWrapper zip_fd_wrapper(test_zip_file_,
@@ skipping 72 matching lines @@
   ASSERT_TRUE(reader.Open(test_zip_file_));
   base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt"));
   ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
   ASSERT_TRUE(reader.ExtractCurrentEntryToFilePath(
       test_dir_.AppendASCII("quux.txt")));
   // Read the output file ans compute the MD5.
   std::string output;
   ASSERT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("quux.txt"),
                                      &output));
   const std::string md5 = base::MD5String(output);
-  const std::string kExpectedMD5 = "d1ae4ac8a17a0e09317113ab284b57a6";
-  EXPECT_EQ(kExpectedMD5, md5);
+  EXPECT_EQ(kQuuxExpectedMD5, md5);
   // quux.txt should be larger than kZipBufSize so that we can exercise
   // the loop in ExtractCurrentEntry().
   EXPECT_LT(static_cast<size_t>(internal::kZipBufSize), output.size());
 }
 
 TEST_F(ZipReaderTest, PlatformFileExtractCurrentEntryToFilePath_RegularFile) {
   ZipReader reader;
   PlatformFileWrapper zip_fd_wrapper(test_zip_file_,
                                      PlatformFileWrapper::READ_ONLY);
   ASSERT_TRUE(reader.OpenFromPlatformFile(zip_fd_wrapper.platform_file()));
   base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt"));
   ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
   ASSERT_TRUE(reader.ExtractCurrentEntryToFilePath(
       test_dir_.AppendASCII("quux.txt")));
   // Read the output file and compute the MD5.
   std::string output;
   ASSERT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("quux.txt"),
                                      &output));
   const std::string md5 = base::MD5String(output);
-  const std::string kExpectedMD5 = "d1ae4ac8a17a0e09317113ab284b57a6";
-  EXPECT_EQ(kExpectedMD5, md5);
+  EXPECT_EQ(kQuuxExpectedMD5, md5);
   // quux.txt should be larger than kZipBufSize so that we can exercise
   // the loop in ExtractCurrentEntry().
   EXPECT_LT(static_cast<size_t>(internal::kZipBufSize), output.size());
 }
 
 #if defined(OS_POSIX)
 TEST_F(ZipReaderTest, PlatformFileExtractCurrentEntryToFd_RegularFile) {
   ZipReader reader;
   PlatformFileWrapper zip_fd_wrapper(test_zip_file_,
                                      PlatformFileWrapper::READ_ONLY);
   ASSERT_TRUE(reader.OpenFromPlatformFile(zip_fd_wrapper.platform_file()));
   base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt"));
   base::FilePath out_path = test_dir_.AppendASCII("quux.txt");
   PlatformFileWrapper out_fd_w(out_path, PlatformFileWrapper::READ_WRITE);
   ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
   ASSERT_TRUE(reader.ExtractCurrentEntryToFd(out_fd_w.platform_file()));
   // Read the output file and compute the MD5.
   std::string output;
   ASSERT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("quux.txt"),
                                      &output));
   const std::string md5 = base::MD5String(output);
-  const std::string kExpectedMD5 = "d1ae4ac8a17a0e09317113ab284b57a6";
-  EXPECT_EQ(kExpectedMD5, md5);
+  EXPECT_EQ(kQuuxExpectedMD5, md5);
   // quux.txt should be larger than kZipBufSize so that we can exercise
   // the loop in ExtractCurrentEntry().
   EXPECT_LT(static_cast<size_t>(internal::kZipBufSize), output.size());
 }
 #endif
 
 TEST_F(ZipReaderTest, ExtractCurrentEntryToFilePath_Directory) {
   ZipReader reader;
   ASSERT_TRUE(reader.Open(test_zip_file_));
   base::FilePath target_path(FILE_PATH_LITERAL("foo/"));
@@ skipping 10 matching lines @@
   base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt"));
   ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
   ASSERT_TRUE(reader.ExtractCurrentEntryIntoDirectory(test_dir_));
   // Sub directories should be created.
   ASSERT_TRUE(base::DirectoryExists(test_dir_.AppendASCII("foo/bar")));
   // And the file should be created.
   std::string output;
   ASSERT_TRUE(base::ReadFileToString(
       test_dir_.AppendASCII("foo/bar/quux.txt"), &output));
   const std::string md5 = base::MD5String(output);
-  const std::string kExpectedMD5 = "d1ae4ac8a17a0e09317113ab284b57a6";
-  EXPECT_EQ(kExpectedMD5, md5);
+  EXPECT_EQ(kQuuxExpectedMD5, md5);
 }
 
 TEST_F(ZipReaderTest, current_entry_info_RegularFile) {
   ZipReader reader;
   ASSERT_TRUE(reader.Open(test_zip_file_));
   base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt"));
   ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
   ZipReader::EntryInfo* current_entry_info = reader.current_entry_info();
 
   EXPECT_EQ(target_path, current_entry_info->file_path());
@@ skipping 110 matching lines @@
   ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
   ASSERT_TRUE(reader.ExtractCurrentEntryToFilePath(
       test_dir_.AppendASCII("test.txt")));
 
   std::string actual;
   ASSERT_TRUE(base::ReadFileToString(
       test_dir_.AppendASCII("test.txt"), &actual));
   EXPECT_EQ(std::string("This is a test.\n"), actual);
 }
 
+// Verifies that the asynchronous extraction to a file works.
+TEST_F(ZipReaderTest, ExtractToFileAsync_RegularFile) {
+  MockUnzipListener listener;
+
+  ZipReader reader;
+  base::FilePath target_file = test_dir_.AppendASCII("quux.txt");
+  base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt"));
+  ASSERT_TRUE(reader.Open(test_zip_file_));
+  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  reader.ExtractCurrentEntryToFilePathAsync(
+      target_file,
+      base::Bind(&MockUnzipListener::OnUnzipSuccess,
+                 listener.AsWeakPtr()),
+      base::Bind(&MockUnzipListener::OnUnzipFailure,
+                 listener.AsWeakPtr()),
+      base::Bind(&MockUnzipListener::OnUnzipProgress,
+                 listener.AsWeakPtr()));
+
+  EXPECT_EQ(0, listener.success_calls);
+  EXPECT_EQ(0, listener.failure_calls);
+  EXPECT_EQ(0, listener.progress_calls);
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(1, listener.success_calls);
+  EXPECT_EQ(0, listener.failure_calls);
+  EXPECT_GE(listener.progress_calls, 1);
+
+  CompareFileAndMD5(target_file, kQuuxExpectedMD5);
+
+  int64 file_size;

[satorux1, 2013/12/11 06:45:11]

= 0;

to make it initialized. doesn't matter here, but it's a good practice to avoid
uninitialized variables 

[Drew Haven, 2013/12/11 18:20:53]

Done.

+  ASSERT_TRUE(base::GetFileSize(target_file, &file_size));
+
+  EXPECT_EQ(listener.current_progress, file_size);
+}
+
+// Verifies that the asynchronous extraction to a file works.
+TEST_F(ZipReaderTest, ExtractToFileAsync_Directory) {
+  MockUnzipListener listener;
+
+  ZipReader reader;
+  base::FilePath target_path(FILE_PATH_LITERAL("foo/"));
+  ASSERT_TRUE(reader.Open(test_zip_file_));
+  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  reader.ExtractCurrentEntryToFilePathAsync(
+      target_path,
+      base::Bind(&MockUnzipListener::OnUnzipSuccess,
+                 listener.AsWeakPtr()),
+      base::Bind(&MockUnzipListener::OnUnzipFailure,
+                 listener.AsWeakPtr()),
+      base::Bind(&MockUnzipListener::OnUnzipProgress,
+                 listener.AsWeakPtr()));
+
+  EXPECT_EQ(1, listener.success_calls);
+  EXPECT_EQ(0, listener.failure_calls);
+  EXPECT_GE(0, listener.progress_calls);
+
+  ASSERT_TRUE(base::DirectoryExists(target_path));
+}
+
 }  // namespace zip