Change unzipping library to support cross-process delegates.

This prepares for servicification of the unzipping library, where the
library will not have direct access to the filesystem.

Bug: 792066
Change-Id: I696dd8ef0936f22dc637e078bd8bba565e854ead
Reviewed-on: https://chromium-review.googlesource.com/860996
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Satoru Takabayashi <satorux@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#534778}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 5d19ee7988a3f97118f3ff791acf77f3eeed891c

6 files changed, 270 insertions, 280 deletions

diff --git a/google/zip.cc b/google/zip.cc
index 89330d6..8ad2941 100644
--- a/google/zip.cc
+++ b/google/zip.cc
@@ -12,6 +12,7 @@
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "third_party/zlib/google/zip_internal.h"
@@ -33,6 +34,20 @@ bool ExcludeHiddenFilesFilter(const base::FilePath& file_path) {
   return !IsHiddenFile(file_path);
 }
 
+// Creates a directory at |extract_dir|/|entry_path|, including any parents.
+bool CreateDirectory(const base::FilePath& extract_dir,
+                     const base::FilePath& entry_path) {
+  return base::CreateDirectory(extract_dir.Append(entry_path));
+}
+
+// Creates a WriterDelegate that can write a file at |extract_dir|/|entry_path|.
+std::unique_ptr<WriterDelegate> CreateFilePathWriterDelegate(
+    const base::FilePath& extract_dir,
+    const base::FilePath& entry_path) {
+  return std::make_unique<FilePathWriterDelegate>(
+      extract_dir.Append(entry_path));
+}
+
 class DirectFileAccessor : public FileAccessor {
  public:
   explicit DirectFileAccessor(base::FilePath src_dir) : src_dir_(src_dir) {}
@@ -166,30 +181,52 @@ bool UnzipWithFilterCallback(const base::FilePath& src_file,
                              const base::FilePath& dest_dir,
                              const FilterCallback& filter_cb,
                              bool log_skipped_files) {
-  ZipReader reader;
-  if (!reader.Open(src_file)) {
+  base::File file(src_file, base::File::FLAG_OPEN | base::File::FLAG_READ);
+  if (!file.IsValid()) {
     DLOG(WARNING) << "Failed to open " << src_file.value();
     return false;
   }
+  return UnzipWithFilterAndWriters(
+      file.GetPlatformFile(),
+      base::BindRepeating(&CreateFilePathWriterDelegate, dest_dir),
+      base::BindRepeating(&CreateDirectory, dest_dir), filter_cb,
+      log_skipped_files);
+}
+
+bool UnzipWithFilterAndWriters(const base::PlatformFile& src_file,
+                               const WriterFactory& writer_factory,
+                               const DirectoryCreator& directory_creator,
+                               const FilterCallback& filter_cb,
+                               bool log_skipped_files) {
+  ZipReader reader;
+  if (!reader.OpenFromPlatformFile(src_file)) {
+    DLOG(WARNING) << "Failed to open src_file " << src_file;
+    return false;
+  }
   while (reader.HasMore()) {
     if (!reader.OpenCurrentEntryInZip()) {
       DLOG(WARNING) << "Failed to open the current file in zip";
       return false;
     }
+    const base::FilePath& entry_path = reader.current_entry_info()->file_path();
     if (reader.current_entry_info()->is_unsafe()) {
-      DLOG(WARNING) << "Found an unsafe file in zip "
-                    << reader.current_entry_info()->file_path().value();
+      DLOG(WARNING) << "Found an unsafe file in zip " << entry_path;
       return false;
     }
-    if (filter_cb.Run(reader.current_entry_info()->file_path())) {
-      if (!reader.ExtractCurrentEntryIntoDirectory(dest_dir)) {
-        DLOG(WARNING) << "Failed to extract "
-                      << reader.current_entry_info()->file_path().value();
-        return false;
+    if (filter_cb.Run(entry_path)) {
+      if (reader.current_entry_info()->is_directory()) {
+        if (!directory_creator.Run(entry_path))
+          return false;
+      } else {
+        std::unique_ptr<WriterDelegate> writer = writer_factory.Run(entry_path);
+        if (!reader.ExtractCurrentEntry(writer.get(),
+                                        std::numeric_limits<uint64_t>::max())) {
+          DLOG(WARNING) << "Failed to extract " << entry_path;
+          return false;
+        }
       }
     } else if (log_skipped_files) {
-      DLOG(WARNING) << "Skipped file "
-                    << reader.current_entry_info()->file_path().value();
+      DLOG(WARNING) << "Skipped file " << entry_path;
     }
 
     if (!reader.AdvanceToNextEntry()) {
diff --git a/google/zip.h b/google/zip.h
index 77d1e8e..7458be0 100644
--- a/google/zip.h
+++ b/google/zip.h
@@ -19,6 +19,8 @@ class File;
 
 namespace zip {
 
+class WriterDelegate;
+
 // Abstraction for file access operation required by Zip().
 // Can be passed to the ZipParams for providing custom access to the files,
 // for example over IPC.
@@ -156,6 +158,21 @@ bool UnzipWithFilterCallback(const base::FilePath& zip_file,
                              const FilterCallback& filter_cb,
                              bool log_skipped_files);
 
+// Unzip the contents of zip_file, using the writers provided by writer_factory.
+// For each file in zip_file, include it only if the callback |filter_cb|
+// returns true. Otherwise omit it.
+// If |log_skipped_files| is true, files skipped during extraction are printed
+// to debug log.
+typedef base::RepeatingCallback<std::unique_ptr<WriterDelegate>(
+    const base::FilePath&)>
+    WriterFactory;
+typedef base::RepeatingCallback<bool(const base::FilePath&)> DirectoryCreator;
+bool UnzipWithFilterAndWriters(const base::PlatformFile& zip_file,
+                               const WriterFactory& writer_factory,
+                               const DirectoryCreator& directory_creator,
+                               const FilterCallback& filter_cb,
+                               bool log_skipped_files);
+
 // Unzip the contents of zip_file into dest_dir.
 bool Unzip(const base::FilePath& zip_file, const base::FilePath& dest_dir);
 
diff --git a/google/zip_reader.cc b/google/zip_reader.cc
index 08c10b6..cca9c46 100644
--- a/google/zip_reader.cc
+++ b/google/zip_reader.cc
@@ -30,54 +30,6 @@ namespace zip {
 
 namespace {
 
-// FilePathWriterDelegate ------------------------------------------------------
-
-// A writer delegate that writes a file at a given path.
-class FilePathWriterDelegate : public WriterDelegate {
- public:
-  explicit FilePathWriterDelegate(const base::FilePath& output_file_path);
-  ~FilePathWriterDelegate() override;
-
-  // WriterDelegate methods:
-
-  // Creates the output file and any necessary intermediate directories.
-  bool PrepareOutput() override;
-
-  // Writes |num_bytes| bytes of |data| to the file, returning false if not all
-  // bytes could be written.
-  bool WriteBytes(const char* data, int num_bytes) override;
-
- private:
-  base::FilePath output_file_path_;
-  base::File file_;
-
-  DISALLOW_COPY_AND_ASSIGN(FilePathWriterDelegate);
-};
-
-FilePathWriterDelegate::FilePathWriterDelegate(
-    const base::FilePath& output_file_path)
-    : output_file_path_(output_file_path) {
-}
-
-FilePathWriterDelegate::~FilePathWriterDelegate() {
-}
-
-bool FilePathWriterDelegate::PrepareOutput() {
-  // We can't rely on parent directory entries being specified in the
-  // zip, so we make sure they are created.
-  if (!base::CreateDirectory(output_file_path_.DirName()))
-    return false;
-
-  file_.Initialize(output_file_path_,
-                   base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
-  return file_.IsValid();
-}
-
-bool FilePathWriterDelegate::WriteBytes(const char* data, int num_bytes) {
-  return num_bytes == file_.WriteAtCurrentPos(data, num_bytes);
-}
-
-
 // StringWriterDelegate --------------------------------------------------------
 
 // A writer delegate that writes no more than |max_read_bytes| to a given
@@ -96,6 +48,8 @@ class StringWriterDelegate : public WriterDelegate {
   // if |num_bytes| will cause the string to exceed |max_read_bytes|.
   bool WriteBytes(const char* data, int num_bytes) override;
 
+  void SetTimeModified(const base::Time& time) override;
+
  private:
   size_t max_read_bytes_;
   std::string* output_;
@@ -123,6 +77,10 @@ bool StringWriterDelegate::WriteBytes(const char* data, int num_bytes) {
   return true;
 }
 
+void StringWriterDelegate::SetTimeModified(const base::Time& time) {
+  // Do nothing.
+}
+
 }  // namespace
 
 // TODO(satorux): The implementation assumes that file names in zip files
@@ -273,22 +231,6 @@ bool ZipReader::OpenCurrentEntryInZip() {
   return true;
 }
 
-bool ZipReader::LocateAndOpenEntry(const base::FilePath& path_in_zip) {
-  DCHECK(zip_file_);
-
-  current_entry_info_.reset();
-  reached_end_ = false;
-  const int kDefaultCaseSensivityOfOS = 0;
-  const int result = unzLocateFile(zip_file_,
-                                   path_in_zip.AsUTF8Unsafe().c_str(),
-                                   kDefaultCaseSensivityOfOS);
-  if (result != UNZ_OK)
-    return false;
-
-  // Then Open the entry.
-  return OpenCurrentEntryInZip();
-}
-
 bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate,
                                     uint64_t num_bytes_to_extract) const {
   DCHECK(zip_file_);
@@ -331,32 +273,12 @@ bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate,
 
   unzCloseCurrentFile(zip_file_);
 
-  return entire_file_extracted;
-}
-
-bool ZipReader::ExtractCurrentEntryToFilePath(
-    const base::FilePath& output_file_path) const {
-  DCHECK(zip_file_);
-
-  // If this is a directory, just create it and return.
-  if (current_entry_info()->is_directory())
-    return base::CreateDirectory(output_file_path);
-
-  bool success = false;
-  {
-    FilePathWriterDelegate writer(output_file_path);
-    success =
-        ExtractCurrentEntry(&writer, std::numeric_limits<uint64_t>::max());
-  }
-
-  if (success &&
+  if (entire_file_extracted &&
       current_entry_info()->last_modified() != base::Time::UnixEpoch()) {
-    base::TouchFile(output_file_path,
-                    base::Time::Now(),
-                    current_entry_info()->last_modified());
+    delegate->SetTimeModified(current_entry_info()->last_modified());
   }
 
-  return success;
+  return entire_file_extracted;
 }
 
 void ZipReader::ExtractCurrentEntryToFilePathAsync(
@@ -408,27 +330,6 @@ void ZipReader::ExtractCurrentEntryToFilePathAsync(
                  failure_callback, progress_callback, 0 /* initial offset */));
 }
 
-bool ZipReader::ExtractCurrentEntryIntoDirectory(
-    const base::FilePath& output_directory_path) const {
-  DCHECK(current_entry_info_.get());
-
-  base::FilePath output_file_path = output_directory_path.Append(
-      current_entry_info()->file_path());
-  return ExtractCurrentEntryToFilePath(output_file_path);
-}
-
-bool ZipReader::ExtractCurrentEntryToFile(base::File* file) const {
-  DCHECK(zip_file_);
-
-  // If this is a directory, there's nothing to extract to the file, so return
-  // false.
-  if (current_entry_info()->is_directory())
-    return false;
-
-  FileWriterDelegate writer(file);
-  return ExtractCurrentEntry(&writer, std::numeric_limits<uint64_t>::max());
-}
-
 bool ZipReader::ExtractCurrentEntryToString(uint64_t max_read_bytes,
                                             std::string* output) const {
   DCHECK(output);
@@ -533,10 +434,10 @@ void ZipReader::ExtractChunk(base::File output_file,
 
 // FileWriterDelegate ----------------------------------------------------------
 
-FileWriterDelegate::FileWriterDelegate(base::File* file)
-    : file_(file),
-      file_length_(0) {
-}
+FileWriterDelegate::FileWriterDelegate(base::File* file) : file_(file) {}
+
+FileWriterDelegate::FileWriterDelegate(std::unique_ptr<base::File> file)
+    : file_(file.get()), owned_file_(std::move(file)) {}
 
 FileWriterDelegate::~FileWriterDelegate() {
   if (!file_->SetLength(file_length_)) {
@@ -555,4 +456,36 @@ bool FileWriterDelegate::WriteBytes(const char* data, int num_bytes) {
   return bytes_written == num_bytes;
 }
 
+void FileWriterDelegate::SetTimeModified(const base::Time& time) {
+  file_->SetTimes(base::Time::Now(), time);
+}
+
+// FilePathWriterDelegate ------------------------------------------------------
+
+FilePathWriterDelegate::FilePathWriterDelegate(
+    const base::FilePath& output_file_path)
+    : output_file_path_(output_file_path) {}
+
+FilePathWriterDelegate::~FilePathWriterDelegate() {}
+
+bool FilePathWriterDelegate::PrepareOutput() {
+  // We can't rely on parent directory entries being specified in the
+  // zip, so we make sure they are created.
+  if (!base::CreateDirectory(output_file_path_.DirName()))
+    return false;
+
+  file_.Initialize(output_file_path_,
+                   base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+  return file_.IsValid();
+}
+
+bool FilePathWriterDelegate::WriteBytes(const char* data, int num_bytes) {
+  return num_bytes == file_.WriteAtCurrentPos(data, num_bytes);
+}
+
+void FilePathWriterDelegate::SetTimeModified(const base::Time& time) {
+  file_.Close();
+  base::TouchFile(output_file_path_, base::Time::Now(), time);
+}
+
 }  // namespace zip
diff --git a/google/zip_reader.h b/google/zip_reader.h
index 0464acd..cd24075 100644
--- a/google/zip_reader.h
+++ b/google/zip_reader.h
@@ -39,6 +39,9 @@ class WriterDelegate {
   // Invoked to write the next chunk of data. Return false on failure to cancel
   // extraction.
   virtual bool WriteBytes(const char* data, int num_bytes) = 0;
+
+  // Sets the last-modified time of the data.
+  virtual void SetTimeModified(const base::Time& time) = 0;
 };
 
 // This class is used for reading zip files. A typical use case of this
@@ -49,16 +52,16 @@ class WriterDelegate {
 //   reader.Open(zip_file_path);
 //   while (reader.HasMore()) {
 //     reader.OpenCurrentEntryInZip();
-//     reader.ExtractCurrentEntryToDirectory(output_directory_path);
+//     const base::FilePath& entry_path =
+//        reader.current_entry_info()->file_path();
+//     auto writer = CreateFilePathWriterDelegate(extract_dir, entry_path);
+//     reader.ExtractCurrentEntry(writer, std::numeric_limits<uint64_t>::max());
 //     reader.AdvanceToNextEntry();
 //   }
 //
 // For simplicity, error checking is omitted in the example code above. The
 // production code should check return values from all of these functions.
 //
-// This calls can also be used for random access of contents in a zip file
-// using LocateAndOpenEntry().
-//
 class ZipReader {
  public:
   // A callback that is called when the operation is successful.
@@ -154,27 +157,12 @@ class ZipReader {
   // state is reset automatically as needed.
   bool OpenCurrentEntryInZip();
 
-  // Locates an entry in the zip file and opens it. Returns true on
-  // success. This function internally calls OpenCurrentEntryInZip() on
-  // success. On failure, current_entry_info() becomes NULL.
-  bool LocateAndOpenEntry(const base::FilePath& path_in_zip);
-
   // Extracts |num_bytes_to_extract| bytes of the current entry to |delegate|,
   // starting from the beginning of the entry. Return value specifies whether
   // the entire file was extracted.
   bool ExtractCurrentEntry(WriterDelegate* delegate,
                            uint64_t num_bytes_to_extract) const;
 
-  // 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 preserves the timestamp of the original entry. If that
-  // timestamp is not valid, the timestamp will be set to the current time.
-  bool ExtractCurrentEntryToFilePath(
-      const base::FilePath& output_file_path) const;
-
   // Asynchronously extracts the current entry to the given output file path.
   // If the current entry is a directory it just creates the directory
   // synchronously instead.  OpenCurrentEntryInZip() must be called beforehand.
@@ -187,24 +175,6 @@ class ZipReader {
       const FailureCallback& failure_callback,
       const ProgressCallback& progress_callback);
 
-  // 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.
-  //
-  // This function preserves the timestamp of the original entry. If that
-  // timestamp is not valid, the timestamp will be set to the current time.
-  bool ExtractCurrentEntryIntoDirectory(
-      const base::FilePath& output_directory_path) const;
-
-  // Extracts the current entry by writing directly to a platform file.
-  // Does not close the file. Returns true on success.
-  bool ExtractCurrentEntryToFile(base::File* file) const;
-
   // Extracts the current entry into memory. If the current entry is a
   // directory, the |output| parameter is set to the empty string. If the
   // current entry is a file, the |output| parameter is filled with its
@@ -258,8 +228,14 @@ class ZipReader {
 // A writer delegate that writes to a given File.
 class FileWriterDelegate : public WriterDelegate {
  public:
+  // Constructs a FileWriterDelegate that manipulates |file|. The delegate will
+  // not own |file|, therefore the caller must guarantee |file| will outlive the
+  // delegate.
   explicit FileWriterDelegate(base::File* file);
 
+  // Constructs a FileWriterDelegate that takes ownership of |file|.
+  explicit FileWriterDelegate(std::unique_ptr<base::File> file);
+
   // Truncates the file to the number of bytes written.
   ~FileWriterDelegate() override;
 
@@ -272,13 +248,47 @@ class FileWriterDelegate : public WriterDelegate {
   // if not all bytes could be written.
   bool WriteBytes(const char* data, int num_bytes) override;
 
+  // Sets the last-modified time of the data.
+  void SetTimeModified(const base::Time& time) override;
+
  private:
+  // The file the delegate modifies.
   base::File* file_;
-  int64_t file_length_;
+
+  // The delegate can optionally own the file it modifies, in which case
+  // owned_file_ is set and file_ is an alias for owned_file_.
+  std::unique_ptr<base::File> owned_file_;
+
+  int64_t file_length_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(FileWriterDelegate);
 };
 
+// A writer delegate that writes a file at a given path.
+class FilePathWriterDelegate : public WriterDelegate {
+ public:
+  explicit FilePathWriterDelegate(const base::FilePath& output_file_path);
+  ~FilePathWriterDelegate() override;
+
+  // WriterDelegate methods:
+
+  // Creates the output file and any necessary intermediate directories.
+  bool PrepareOutput() override;
+
+  // Writes |num_bytes| bytes of |data| to the file, returning false if not all
+  // bytes could be written.
+  bool WriteBytes(const char* data, int num_bytes) override;
+
+  // Sets the last-modified time of the data.
+  void SetTimeModified(const base::Time& time) override;
+
+ private:
+  base::FilePath output_file_path_;
+  base::File file_;
+
+  DISALLOW_COPY_AND_ASSIGN(FilePathWriterDelegate);
+};
+
 }  // namespace zip
 
 #endif  // THIRD_PARTY_ZLIB_GOOGLE_ZIP_READER_H_
diff --git a/google/zip_reader_unittest.cc b/google/zip_reader_unittest.cc
index 30a56f1..0c74f37 100644
--- a/google/zip_reader_unittest.cc
+++ b/google/zip_reader_unittest.cc
@@ -109,8 +109,30 @@ class MockWriterDelegate : public zip::WriterDelegate {
  public:
   MOCK_METHOD0(PrepareOutput, bool());
   MOCK_METHOD2(WriteBytes, bool(const char*, int));
+  MOCK_METHOD1(SetTimeModified, void(const base::Time&));
 };
 
+bool ExtractCurrentEntryToFilePath(zip::ZipReader* reader,
+                                   base::FilePath path) {
+  zip::FilePathWriterDelegate writer(path);
+  return reader->ExtractCurrentEntry(&writer,
+                                     std::numeric_limits<uint64_t>::max());
+}
+
+bool LocateAndOpenEntry(zip::ZipReader* reader,
+                        const base::FilePath& path_in_zip) {
+  // The underlying library can do O(1) access, but ZipReader does not expose
+  // that. O(N) access is acceptable for these tests.
+  while (reader->HasMore()) {
+    if (!reader->OpenCurrentEntryInZip())
+      return false;
+    if (reader->current_entry_info()->file_path() == path_in_zip)
+      return true;
+    reader->AdvanceToNextEntry();
+  }
+  return false;
+}
+
 }   // namespace
 
 namespace zip {
@@ -250,112 +272,11 @@ TEST_F(ZipReaderTest, PlatformFileIteration) {
   EXPECT_EQ(test_zip_contents_, actual_contents);
 }
 
-TEST_F(ZipReaderTest, LocateAndOpenEntry_ValidFile) {
-  std::set<base::FilePath> actual_contents;
-  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));
-  EXPECT_EQ(target_path, reader.current_entry_info()->file_path());
-}
-
-TEST_F(ZipReaderTest, LocateAndOpenEntry_NonExistentFile) {
-  std::set<base::FilePath> actual_contents;
-  ZipReader reader;
-  ASSERT_TRUE(reader.Open(test_zip_file_));
-  base::FilePath target_path(FILE_PATH_LITERAL("nonexistent.txt"));
-  ASSERT_FALSE(reader.LocateAndOpenEntry(target_path));
-  EXPECT_EQ(NULL, reader.current_entry_info());
-}
-
-TEST_F(ZipReaderTest, ExtractCurrentEntryToFilePath_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));
-  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);
-  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;
-  FileWrapper zip_fd_wrapper(test_zip_file_, FileWrapper::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);
-  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, PlatformFileExtractCurrentEntryToFile_RegularFile) {
-  ZipReader reader;
-  FileWrapper zip_fd_wrapper(test_zip_file_, FileWrapper::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");
-  FileWrapper out_fd_w(out_path, FileWrapper::READ_WRITE);
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
-  ASSERT_TRUE(reader.ExtractCurrentEntryToFile(out_fd_w.file()));
-  // Read the output file and compute the MD5.
-  std::string output;
-  ASSERT_TRUE(base::ReadFileToString(out_path, &output));
-  const std::string md5 = base::MD5String(output);
-  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, ExtractCurrentEntryToFilePath_Directory) {
-  ZipReader reader;
-  ASSERT_TRUE(reader.Open(test_zip_file_));
-  base::FilePath target_path(FILE_PATH_LITERAL("foo/"));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
-  ASSERT_TRUE(reader.ExtractCurrentEntryToFilePath(
-      test_dir_.AppendASCII("foo")));
-  // The directory should be created.
-  ASSERT_TRUE(base::DirectoryExists(test_dir_.AppendASCII("foo")));
-}
-
-TEST_F(ZipReaderTest, ExtractCurrentEntryIntoDirectory_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));
-  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);
-  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));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   ZipReader::EntryInfo* current_entry_info = reader.current_entry_info();
 
   EXPECT_EQ(target_path, current_entry_info->file_path());
@@ -381,7 +302,7 @@ TEST_F(ZipReaderTest, current_entry_info_DotDotFile) {
   ASSERT_TRUE(reader.Open(evil_zip_file_));
   base::FilePath target_path(FILE_PATH_LITERAL(
       "../levilevilevilevilevilevilevilevilevilevilevilevil"));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   ZipReader::EntryInfo* current_entry_info = reader.current_entry_info();
   EXPECT_EQ(target_path, current_entry_info->file_path());
 
@@ -409,7 +330,7 @@ TEST_F(ZipReaderTest, current_entry_info_AbsoluteFile) {
   ZipReader reader;
   ASSERT_TRUE(reader.Open(evil_via_absolute_file_name_zip_file_));
   base::FilePath target_path(FILE_PATH_LITERAL("/evil.txt"));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   ZipReader::EntryInfo* current_entry_info = reader.current_entry_info();
   EXPECT_EQ(target_path, current_entry_info->file_path());
 
@@ -422,7 +343,7 @@ TEST_F(ZipReaderTest, current_entry_info_Directory) {
   ZipReader reader;
   ASSERT_TRUE(reader.Open(test_zip_file_));
   base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/"));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   ZipReader::EntryInfo* current_entry_info = reader.current_entry_info();
 
   EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("foo/bar/")),
@@ -469,9 +390,9 @@ TEST_F(ZipReaderTest, OpenFromString) {
   ZipReader reader;
   ASSERT_TRUE(reader.OpenFromString(data));
   base::FilePath target_path(FILE_PATH_LITERAL("test.txt"));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
-  ASSERT_TRUE(reader.ExtractCurrentEntryToFilePath(
-      test_dir_.AppendASCII("test.txt")));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
+  ASSERT_TRUE(ExtractCurrentEntryToFilePath(&reader,
+                                            test_dir_.AppendASCII("test.txt")));
 
   std::string actual;
   ASSERT_TRUE(base::ReadFileToString(
@@ -487,7 +408,7 @@ TEST_F(ZipReaderTest, ExtractToFileAsync_RegularFile) {
   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));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   reader.ExtractCurrentEntryToFilePathAsync(
       target_file,
       base::Bind(&MockUnzipListener::OnUnzipSuccess,
@@ -527,7 +448,7 @@ TEST_F(ZipReaderTest, ExtractToFileAsync_Directory) {
   base::FilePath target_file = test_dir_.AppendASCII("foo");
   base::FilePath target_path(FILE_PATH_LITERAL("foo/"));
   ASSERT_TRUE(reader.Open(test_zip_file_));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   reader.ExtractCurrentEntryToFilePathAsync(
       target_file,
       base::Bind(&MockUnzipListener::OnUnzipSuccess,
@@ -566,7 +487,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryToString) {
 
     base::FilePath file_name = base::FilePath::FromUTF8Unsafe(
         base::StringPrintf("%d.txt", static_cast<int>(i)));
-    ASSERT_TRUE(reader.LocateAndOpenEntry(file_name));
+    ASSERT_TRUE(LocateAndOpenEntry(&reader, file_name));
 
     if (i > 1) {
       // Off by one byte read limit: must fail.
@@ -598,14 +519,14 @@ TEST_F(ZipReaderTest, ExtractPartOfCurrentEntry) {
   ASSERT_TRUE(reader.Open(test_zip_file));
 
   base::FilePath file_name0 = base::FilePath::FromUTF8Unsafe("0.txt");
-  ASSERT_TRUE(reader.LocateAndOpenEntry(file_name0));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, file_name0));
   EXPECT_TRUE(reader.ExtractCurrentEntryToString(0, &contents));
   EXPECT_EQ("", contents);
   EXPECT_TRUE(reader.ExtractCurrentEntryToString(1, &contents));
   EXPECT_EQ("", contents);
 
   base::FilePath file_name1 = base::FilePath::FromUTF8Unsafe("1.txt");
-  ASSERT_TRUE(reader.LocateAndOpenEntry(file_name1));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, file_name1));
   EXPECT_TRUE(reader.ExtractCurrentEntryToString(0, &contents));
   EXPECT_EQ("", contents);
   EXPECT_TRUE(reader.ExtractCurrentEntryToString(1, &contents));
@@ -614,7 +535,7 @@ TEST_F(ZipReaderTest, ExtractPartOfCurrentEntry) {
   EXPECT_EQ("0", contents);
 
   base::FilePath file_name4 = base::FilePath::FromUTF8Unsafe("4.txt");
-  ASSERT_TRUE(reader.LocateAndOpenEntry(file_name4));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, file_name4));
   EXPECT_TRUE(reader.ExtractCurrentEntryToString(0, &contents));
   EXPECT_EQ("", contents);
   EXPECT_FALSE(reader.ExtractCurrentEntryToString(2, &contents));
@@ -650,7 +571,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryPrepareFailure) {
   ZipReader reader;
 
   ASSERT_TRUE(reader.Open(test_zip_file_));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   ASSERT_FALSE(reader.ExtractCurrentEntry(
       &mock_writer, std::numeric_limits<uint64_t>::max()));
 }
@@ -669,7 +590,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryWriteBytesFailure) {
   ZipReader reader;
 
   ASSERT_TRUE(reader.Open(test_zip_file_));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   ASSERT_FALSE(reader.ExtractCurrentEntry(
       &mock_writer, std::numeric_limits<uint64_t>::max()));
 }
@@ -682,12 +603,13 @@ TEST_F(ZipReaderTest, ExtractCurrentEntrySuccess) {
       .WillOnce(Return(true));
   EXPECT_CALL(mock_writer, WriteBytes(_, _))
       .WillRepeatedly(Return(true));
+  EXPECT_CALL(mock_writer, SetTimeModified(_));
 
   base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt"));
   ZipReader reader;
 
   ASSERT_TRUE(reader.Open(test_zip_file_));
-  ASSERT_TRUE(reader.LocateAndOpenEntry(target_path));
+  ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
   ASSERT_TRUE(reader.ExtractCurrentEntry(&mock_writer,
                                          std::numeric_limits<uint64_t>::max()));
 }
diff --git a/google/zip_unittest.cc b/google/zip_unittest.cc
index d282c57..21e1f71 100644
--- a/google/zip_unittest.cc
+++ b/google/zip_unittest.cc
@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
@@ -338,6 +339,75 @@ TEST_F(ZipTest, UnzipEvil2) {
   ASSERT_FALSE(base::PathExists(evil_file));
 }
 
+TEST_F(ZipTest, UnzipWithFilter) {
+  auto filter = base::BindRepeating([](const base::FilePath& path) {
+    return path.BaseName().MaybeAsASCII() == "foo.txt";
+  });
+  base::FilePath path;
+  ASSERT_TRUE(GetTestDataDirectory(&path));
+  ASSERT_TRUE(zip::UnzipWithFilterCallback(path.AppendASCII("test.zip"),
+                                           test_dir_, filter, false));
+  // Only foo.txt should have been extracted. The following paths should not
+  // be extracted:
+  //   foo/
+  //   foo/bar.txt
+  //   foo/bar/
+  //   foo/bar/.hidden
+  //   foo/bar/baz.txt
+  //   foo/bar/quux.txt
+  ASSERT_TRUE(base::PathExists(test_dir_.AppendASCII("foo.txt")));
+  base::FileEnumerator extractedFiles(
+      test_dir_,
+      false,  // Do not enumerate recursively - the file must be in the root.
+      base::FileEnumerator::FileType::FILES);
+  int extracted_count = 0;
+  while (!extractedFiles.Next().empty())
+    ++extracted_count;
+  ASSERT_EQ(1, extracted_count);
+
+  base::FileEnumerator extractedDirs(
+      test_dir_,
+      false,  // Do not enumerate recursively - we require zero directories.
+      base::FileEnumerator::FileType::DIRECTORIES);
+  extracted_count = 0;
+  while (!extractedDirs.Next().empty())
+    ++extracted_count;
+  ASSERT_EQ(0, extracted_count);
+}
+
+TEST_F(ZipTest, UnzipWithDelegates) {
+  auto filter =
+      base::BindRepeating([](const base::FilePath& path) { return true; });
+  auto dir_creator = base::BindRepeating(
+      [](const base::FilePath& extract_dir, const base::FilePath& entry_path) {
+        return base::CreateDirectory(extract_dir.Append(entry_path));
+      },
+      test_dir_);
+  auto writer = base::BindRepeating(
+      [](const base::FilePath& extract_dir, const base::FilePath& entry_path)
+          -> std::unique_ptr<zip::WriterDelegate> {
+        return std::make_unique<zip::FilePathWriterDelegate>(
+            extract_dir.Append(entry_path));
+      },
+      test_dir_);
+  base::FilePath path;
+  ASSERT_TRUE(GetTestDataDirectory(&path));
+  base::File file(path.AppendASCII("test.zip"),
+                  base::File::Flags::FLAG_OPEN | base::File::Flags::FLAG_READ);
+  ASSERT_TRUE(zip::UnzipWithFilterAndWriters(file.GetPlatformFile(), writer,
+                                             dir_creator, filter, false));
+  base::FilePath dir = test_dir_;
+  base::FilePath dir_foo = dir.AppendASCII("foo");
+  base::FilePath dir_foo_bar = dir_foo.AppendASCII("bar");
+  ASSERT_TRUE(base::PathExists(dir.AppendASCII("foo.txt")));
+  ASSERT_TRUE(base::PathExists(dir_foo));
+  ASSERT_TRUE(base::PathExists(dir_foo.AppendASCII("bar.txt")));
+  ASSERT_TRUE(base::PathExists(dir_foo_bar));
+  ASSERT_TRUE(base::PathExists(dir_foo_bar.AppendASCII(".hidden")));
+  ASSERT_TRUE(base::PathExists(dir_foo_bar.AppendASCII("baz.txt")));
+  ASSERT_TRUE(base::PathExists(dir_foo_bar.AppendASCII("quux.txt")));
+}
+
 TEST_F(ZipTest, Zip) {
   base::FilePath src_dir;
   ASSERT_TRUE(GetTestDataDirectory(&src_dir));
@@ -424,10 +494,11 @@ TEST_F(ZipTest, ZipFiles) {
   EXPECT_TRUE(reader.Open(zip_name));
   EXPECT_EQ(zip_file_list_.size(), static_cast<size_t>(reader.num_entries()));
   for (size_t i = 0; i < zip_file_list_.size(); ++i) {
-    EXPECT_TRUE(reader.LocateAndOpenEntry(zip_file_list_[i]));
-    // Check the path in the entry just in case.
+    EXPECT_TRUE(reader.HasMore());
+    EXPECT_TRUE(reader.OpenCurrentEntryInZip());
     const zip::ZipReader::EntryInfo* entry_info = reader.current_entry_info();
     EXPECT_EQ(entry_info->file_path(), zip_file_list_[i]);
+    reader.AdvanceToNextEntry();
   }
 }
 #endif  // defined(OS_POSIX)