Implements a Zip() utility function. Refactor existing

Unzip-relatedness into shared locations.

Review URL: http://codereview.chromium.org/118028

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

chrome/browser/extensions/extensions_service.cc

@@ -31,7 +31,7 @@
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pref_service.h"
-#include "chrome/common/unzip.h"
+#include "chrome/common/zip.h"
 #include "chrome/common/url_constants.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"

chrome/chrome.gyp

@@ -412,8 +412,6 @@
         'common/transport_dib_linux.cc',
         'common/transport_dib_mac.cc',
         'common/transport_dib_win.cc',
-        'common/unzip.cc',  # Requires zlib directly.
-        'common/unzip.h',
         'common/url_constants.cc',
         'common/url_constants.h',
         'common/visitedlink_common.cc',
@@ -426,6 +424,8 @@
         'common/x11_util.cc',
         'common/x11_util.h',
         'common/x11_util_internal.h',
+        'common/zip.cc',  # Requires zlib directly.
+        'common/zip.h',
         'third_party/xdg_user_dirs/xdg_user_dir_lookup.cc',
 
         'tools/build/win/precompiled.cc',
@@ -3182,8 +3182,8 @@
         'common/property_bag_unittest.cc',
         'common/resource_dispatcher_unittest.cc',
         'common/time_format_unittest.cc',
-        'common/unzip_unittest.cc',
         'common/worker_thread_ticker_unittest.cc',
+        'common/zip_unittest.cc',
         'renderer/extensions/extension_api_client_unittest.cc',
         'renderer/extensions/greasemonkey_api_unittest.cc',
         'renderer/extensions/json_schema_unittest.cc',

chrome/common/common.vcproj

@@ -745,14 +745,6 @@
 			RelativePath=".\transport_dib_win.cc"
 			>
 		</File>
-		<File
-			RelativePath=".\unzip.cc"
-			>
-		</File>
-		<File
-			RelativePath=".\unzip.h"
-			>
-		</File>
 		<File
 			RelativePath=".\url_constants.cc"
 			>
@@ -789,6 +781,14 @@
 			RelativePath=".\worker_thread_ticker.h"
 			>
 		</File>
+		<File
+			RelativePath=".\zip.cc"
+			>
+		</File>
+		<File
+			RelativePath=".\zip.h"
+			>
+		</File>
 	</Files>
 	<Globals>
 	</Globals>

chrome/common/extensions/extension_unpacker.cc

@@ -16,10 +16,10 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/notification_service.h"
-#include "chrome/common/unzip.h"
 #include "chrome/common/url_constants.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "webkit/glue/image_decoder.h"
+#include "chrome/common/zip.h"
 
 namespace {
 const char kCurrentVersionFileName[] = "Current Version";
@@ -255,7 +255,7 @@ bool ExtensionUnpacker::Run() {
     return false;
   }
 
-  if (!Unzip(extension_path_, temp_install_dir_, NULL)) {
+  if (!Unzip(extension_path_, temp_install_dir_)) {
     SetError("Couldn't unzip extension.");
     return false;
   }

chrome/common/unzip.h

@@ -1,19 +0,0 @@
-// Copyright (c) 2006-2008 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 CHROME_COMMON_UNZIP_H_
-#define CHROME_COMMON_UNZIP_H_
-
-#include <vector>
-
-#include "base/file_path.h"
-
-// Unzip the contents of zip_file into dest_dir.  The complete paths of all
-// created files and directories are added to files if it is non-NULL.
-// Returns true on success.  Does not clean up dest_dir on failure.
-// Does not support encrypted or password protected zip files.
-bool Unzip(const FilePath& zip_file, const FilePath& dest_dir,
-           std::vector<FilePath>* files);
-
-#endif  // CHROME_COMMON_UNZIP_H_

chrome/common/zip.cc

@@ -1,25 +1,25 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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 "chrome/common/unzip.h"
+#include "chrome/common/zip.h"
 
 #include "base/file_util.h"
 #include "base/string_util.h"
 #include "net/base/file_stream.h"
 #include "third_party/zlib/contrib/minizip/unzip.h"
+#include "third_party/zlib/contrib/minizip/zip.h"
 #if defined(OS_WIN)
 #include "third_party/zlib/contrib/minizip/iowin32.h"
 #endif
 
 static const int kZipMaxPath = 256;
-static const int kUnzipBufSize = 8192;
+static const int kZipBufSize = 8192;
 
 // Extract the 'current' selected file from the zip into dest_dir.
 // Output filename is stored in out_file.  Returns true on success.
 static bool ExtractCurrentFile(unzFile zip_file,
-                               const FilePath& dest_dir,
-                               FilePath* out_file) {
+                               const FilePath& dest_dir) {
   char filename_inzip[kZipMaxPath] = {0};
   unz_file_info file_info;
   int err = unzGetCurrentFileInfo(zip_file, &file_info, filename_inzip,
@@ -46,16 +46,16 @@ static bool ExtractCurrentFile(unzFile zip_file,
   std::vector<FilePath::StringType>::iterator iter;
   for (iter = filename_parts.begin(); iter != filename_parts.end(); ++iter)
     dest_file = dest_file.Append(*iter);
-  if (out_file)
-    *out_file = dest_file;
+
   // If this is a directory, just create it and return.
   if (filename_inzip[strlen(filename_inzip) - 1] == '/') {
     if (!file_util::CreateDirectory(dest_file))
       return false;
     return true;
   }
-  // TODO(erikkay): Can we always count on the directory entry coming before a
-  // file in that directory?  If so, then these three lines can be removed.
+
+  // We can't rely on parent directory entries being specified in the zip, so we
+  // make sure they are created.
   FilePath dir = dest_file.DirName();
   if (!file_util::CreateDirectory(dir))
     return false;
@@ -67,9 +67,9 @@ static bool ExtractCurrentFile(unzFile zip_file,
 
   bool ret = true;
   int num_bytes = 0;
-  char buf[kUnzipBufSize];
+  char buf[kZipBufSize];
   do {
-    num_bytes = unzReadCurrentFile(zip_file, buf, kUnzipBufSize);
+    num_bytes = unzReadCurrentFile(zip_file, buf, kZipBufSize);
     if (num_bytes < 0) {
       // If num_bytes < 0, then it's a specific UNZ_* error code.
       // While we're not currently handling these codes specifically, save
@@ -104,7 +104,7 @@ typedef struct {
 // This function is derived from third_party/minizip/iowin32.c.
 // Its only difference is that it treats the char* as UTF8 and
 // uses the Unicode version of CreateFile.
-static void* UnzipOpenFunc(void *opaque, const char* filename, int mode) {
+static void* ZipOpenFunc(void *opaque, const char* filename, int mode) {
   DWORD desired_access, creation_disposition;
   DWORD share_mode, flags_and_attributes;
   HANDLE file = 0;
@@ -147,44 +147,37 @@ static void* UnzipOpenFunc(void *opaque, const char* filename, int mode) {
 }
 #endif
 
-// TODO(erikkay): Make this function asynchronous so that a large zip file
-// won't starve the thread it's running on.  This won't be entirely possible
-// since reads need to be synchronous, but we can at least make writes async.
-bool Unzip(const FilePath& zip_path, const FilePath& dest_dir,
-           std::vector<FilePath>* files) {
+bool Unzip(const FilePath& src_file, const FilePath& dest_dir) {
 #if defined(OS_WIN)
-  zlib_filefunc_def unzip_funcs;
-  fill_win32_filefunc(&unzip_funcs);
-  unzip_funcs.zopen_file = UnzipOpenFunc;
+  zlib_filefunc_def zip_funcs;
+  fill_win32_filefunc(&zip_funcs);
+  zip_funcs.zopen_file = ZipOpenFunc;
 #endif
 
 #if defined(OS_POSIX)
-  std::string zip_file_str = zip_path.value();
-  unzFile zip_file = unzOpen(zip_file_str.c_str());
+  std::string src_file_str = src_file.value();
+  unzFile zip_file = unzOpen(src_file_str.c_str());
 #elif defined(OS_WIN)
-  std::string zip_file_str = WideToUTF8(zip_path.value());
-  unzFile zip_file = unzOpen2(zip_file_str.c_str(), &unzip_funcs);
+  std::string src_file_str = WideToUTF8(src_file.value());
+  unzFile zip_file = unzOpen2(src_file_str.c_str(), &zip_funcs);
 #endif
   if (!zip_file) {
-    LOG(WARNING) << "couldn't open extension file " << zip_file_str;
+    LOG(WARNING) << "couldn't create file " << src_file_str;
     return false;
   }
   unz_global_info zip_info;
   int err;
   err = unzGetGlobalInfo(zip_file, &zip_info);
   if (err != UNZ_OK) {
-    LOG(WARNING) << "couldn't open extension file " << zip_file_str;
+    LOG(WARNING) << "couldn't open zip " << src_file_str;
     return false;
   }
   bool ret = true;
   for (unsigned int i = 0; i < zip_info.number_entry; ++i) {
-    FilePath dest_file;
-    if (!ExtractCurrentFile(zip_file, dest_dir, &dest_file)) {
+    if (!ExtractCurrentFile(zip_file, dest_dir)) {
       ret = false;
       break;
     }
-    if (files)
-      files->push_back(dest_file);
 
     if (i + 1 < zip_info.number_entry) {
       err = unzGoToNextFile(zip_file);
@@ -198,3 +191,109 @@ bool Unzip(const FilePath& zip_path, const FilePath& dest_dir,
   unzClose(zip_file);
   return ret;
 }
+
+static bool AddFileToZip(zipFile zip_file, const FilePath& src_dir) {
+  net::FileStream stream;
+  int flags = base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ;
+  if (stream.Open(src_dir, flags) != 0) {
+    LOG(ERROR) << "Could not open stream for path "
+               << WideToASCII(src_dir.ToWStringHack());
+    return false;
+  }
+
+  int num_bytes;
+  char buf[kZipBufSize];
+  do {
+    num_bytes = stream.Read(buf, kZipBufSize, NULL);
+    if (num_bytes > 0) {
+      if (ZIP_OK != zipWriteInFileInZip(zip_file, buf, num_bytes)) {
+        LOG(ERROR) << "Could not write data to zip for path "
+                   << WideToASCII(src_dir.ToWStringHack());
+        return false;
+      }
+    }
+  } while (num_bytes > 0);
+
+  return true;
+}
+
+static bool AddEntryToZip(zipFile zip_file, const FilePath& path,
+                          const FilePath& root_path) {
+#if defined(OS_WIN)
+  std::string str_path =
+      WideToUTF8(path.value().substr(root_path.value().length() + 1));
+  ReplaceSubstringsAfterOffset(&str_path, 0u, "\\", "/");
+#else
+  std::string str_path = path.value().substr(root_path.value().length() + 1);
+#endif
+
+  bool is_directory = file_util::DirectoryExists(path);
+  if (is_directory)
+    str_path += "/";
+
+  if (ZIP_OK != zipOpenNewFileInZip(
+      zip_file, str_path.c_str(),
+      NULL, NULL, 0u, NULL, 0u, NULL,  // file info, extrafield local, length,
+                                       // extrafield global, length, comment
+      Z_DEFLATED, Z_DEFAULT_COMPRESSION)) {
+    LOG(ERROR) << "Could not open zip file entry " << str_path;
+    return false;
+  }
+
+  bool success = true;
+  if (!is_directory) {
+    success = AddFileToZip(zip_file, path);
+  }
+
+  if (ZIP_OK != zipCloseFileInZip(zip_file)) {
+    LOG(ERROR) << "Could not close zip file entry " << str_path;
+    return false;
+  }
+
+  return success;
+}
+
+bool Zip(const FilePath& src_dir, const FilePath& dest_file) {
+  DCHECK(file_util::DirectoryExists(src_dir));
+
+#if defined(OS_WIN)
+  zlib_filefunc_def zip_funcs;
+  fill_win32_filefunc(&zip_funcs);
+  zip_funcs.zopen_file = ZipOpenFunc;
+#endif
+
+#if defined(OS_POSIX)
+  std::string dest_file_str = dest_file.value();
+  std::string src_dir_str = src_dir.value();
+  zipFile zip_file = zipOpen(src_dir_str.c_str(), APPEND_STATUS_CREATE);
+#elif defined(OS_WIN)
+  std::string dest_file_str = WideToUTF8(dest_file.value());
+  zipFile zip_file = zipOpen2(dest_file_str.c_str(), APPEND_STATUS_CREATE,
+                              NULL,  // global comment
+                              &zip_funcs);
+#endif
+
+  if (!zip_file) {
+    LOG(WARNING) << "couldn't create file " << dest_file_str;
+    return false;
+  }
+
+  bool success = true;
+  file_util::FileEnumerator file_enumerator(
+      src_dir, true,  // recursive
+      file_util::FileEnumerator::FILES_AND_DIRECTORIES);
+  for (FilePath path = file_enumerator.Next(); !path.value().empty();
+       path = file_enumerator.Next()) {
+    if (!AddEntryToZip(zip_file, path, src_dir)) {
+      success = false;
+      return false;
+    }
+  }
+
+  if (ZIP_OK != zipClose(zip_file, NULL)) {  // global comment
+    LOG(ERROR) << "Error closing zip file " << dest_file_str;
+    return false;
+  }
+
+  return success;
+}

chrome/common/zip.h

@@ -0,0 +1,20 @@
+// Copyright (c) 2009 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 CHROME_COMMON_ZIP_H_
+#define CHROME_COMMON_ZIP_H_
+
+#include <vector>
+
+#include "base/file_path.h"
+
+// Zip the contents of src_dir into dest_file. src_path must be a directory.
+// An entry will *not* be created in the zip for the root folder -- children
+// of src_dir will be at the root level of the created zip.
+bool Zip(const FilePath& src_dir, const FilePath& dest_file);
+
+// Unzip the contents of zip_file into dest_dir.
+bool Unzip(const FilePath& zip_file, const FilePath& dest_dir);
+
+#endif  // CHROME_COMMON_ZIP_H_

chrome/common/zip_unittest.cc

@@ -9,14 +9,14 @@
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "chrome/common/chrome_paths.h"
-#include "chrome/common/unzip.h"
+#include "chrome/common/zip.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 namespace {
 
 // Make the test a PlatformTest to setup autorelease pools properly on Mac.
-class UnzipTest : public PlatformTest {
+class ZipTest : public PlatformTest {
  protected:
   virtual void SetUp() {
     PlatformTest::SetUp();
@@ -24,8 +24,7 @@ class UnzipTest : public PlatformTest {
     ASSERT_TRUE(file_util::CreateNewTempDirectory(
         FILE_PATH_LITERAL("unzip_unittest_"), &test_dir_));
 
-    FilePath zip_path(test_dir_.AppendASCII("test"));
-    zip_contents_.insert(zip_path);
+    FilePath zip_path(test_dir_);
     zip_contents_.insert(zip_path.AppendASCII("foo.txt"));
     zip_path = zip_path.AppendASCII("foo");
     zip_contents_.insert(zip_path);
@@ -45,15 +44,16 @@ class UnzipTest : public PlatformTest {
     ASSERT_FALSE(file_util::PathExists(test_dir_));
   }
 
-  void TestZipFile(const FilePath::StringType& filename) {
+  void TestUnzipFile(const FilePath::StringType& filename) {
     FilePath test_dir;
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
-    test_dir = test_dir.AppendASCII("unzip");
-    FilePath path = test_dir.Append(filename);
+    test_dir = test_dir.AppendASCII("zip");
+    TestUnzipFile(test_dir.Append(filename));
+  }
 
+  void TestUnzipFile(const FilePath& path) {
     ASSERT_TRUE(file_util::PathExists(path)) << "no file " << path.value();
-    std::vector<FilePath> out_files;
-    ASSERT_TRUE(Unzip(path, test_dir_, &out_files));
+    ASSERT_TRUE(Unzip(path, test_dir_));
 
     file_util::FileEnumerator files(test_dir_, true,
         file_util::FileEnumerator::FILES_AND_DIRECTORIES);
@@ -66,12 +66,6 @@ class UnzipTest : public PlatformTest {
       next_path = files.Next();
     }
     EXPECT_EQ(count, zip_contents_.size());
-    EXPECT_EQ(count, out_files.size());
-    std::vector<FilePath>::iterator iter;
-    for (iter = out_files.begin(); iter != out_files.end(); ++iter) {
-      EXPECT_EQ(zip_contents_.count(*iter), 1U) <<
-          "Couldn't find " << (*iter).value();
-    }
   }
 
   // the path to temporary directory used to contain the test operations
@@ -82,12 +76,30 @@ class UnzipTest : public PlatformTest {
 };
 
 
-TEST_F(UnzipTest, Unzip) {
-  TestZipFile(FILE_PATH_LITERAL("test.zip"));
+TEST_F(ZipTest, Unzip) {
+  TestUnzipFile(FILE_PATH_LITERAL("test.zip"));
 }
 
-TEST_F(UnzipTest, UnzipUncompressed) {
-  TestZipFile(FILE_PATH_LITERAL("test_nocompress.zip"));
+TEST_F(ZipTest, UnzipUncompressed) {
+  TestUnzipFile(FILE_PATH_LITERAL("test_nocompress.zip"));
+}
+
+
+TEST_F(ZipTest, Zip) {
+  FilePath src_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &src_dir));
+  src_dir = src_dir.AppendASCII("zip").AppendASCII("test");
+
+  FilePath zip_file;
+  ASSERT_TRUE(file_util::CreateNewTempDirectory(
+      FILE_PATH_LITERAL("unzip_unittest_"), &zip_file));
+  zip_file = zip_file.AppendASCII("out.zip");
+
+  EXPECT_TRUE(Zip(src_dir, zip_file));
+
+  TestUnzipFile(zip_file);
+
+  EXPECT_TRUE(file_util::Delete(zip_file, false));
 }
 
 }  // namespace

chrome/test/unit/unittests.vcproj

@@ -1081,10 +1081,6 @@
 				RelativePath="..\..\common\time_format_unittest.cc"
 				>
 			</File>
-			<File
-				RelativePath="..\..\common\unzip_unittest.cc"
-				>
-			</File>
 			<File
 				RelativePath="..\..\common\extensions\user_script_unittest.cc"
 				>
@@ -1093,6 +1089,10 @@
 				RelativePath="..\..\common\worker_thread_ticker_unittest.cc"
 				>
 			</File>
+			<File
+				RelativePath="..\..\common\zip_unittest.cc"
+				>
+			</File>
 		</Filter>
 		<Filter
 			Name="views"

third_party/zlib/zlib.gyp

@@ -18,6 +18,8 @@
         'contrib/minizip/iowin32.h',
         'contrib/minizip/unzip.c',
         'contrib/minizip/unzip.h',
+        'contrib/minizip/zip.c',
+        'contrib/minizip/zip.h',
         'adler32.c',
         'compress.c',
         'crc32.c',