This looks like it was causing the pipe to sometimes overfill.

BUG=38220
TEST=See bug

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

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

8 files changed, 78 insertions, 29 deletions

diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index a9d6966..4021dec 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -76,11 +76,12 @@ void SandboxedExtensionUnpacker::Start() {
   } else {
     // Otherwise, unpack the extension in this process.
     ExtensionUnpacker unpacker(temp_crx_path);
-    if (unpacker.Run() && unpacker.DumpImagesToFile())
-      OnUnpackExtensionSucceeded(*unpacker.parsed_manifest(),
-                                 *unpacker.parsed_catalogs());
-    else
+    if (unpacker.Run() && unpacker.DumpImagesToFile() &&
+        unpacker.DumpMessageCatalogsToFile()) {
+      OnUnpackExtensionSucceeded(*unpacker.parsed_manifest());
+    } else {
       OnUnpackExtensionFailed(unpacker.error_message());
+    }
   }
 }
 
@@ -92,8 +93,7 @@ void SandboxedExtensionUnpacker::StartProcessOnIOThread(
 }
 
 void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
-    const DictionaryValue& manifest,
-    const DictionaryValue& catalogs) {
+    const DictionaryValue& manifest) {
   // Skip check for unittests.
   if (thread_identifier_ != ChromeThread::ID_COUNT)
     DCHECK(ChromeThread::CurrentlyOn(thread_identifier_));
@@ -126,7 +126,7 @@ void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
   if (!RewriteImageFiles())
     return;
 
-  if (!RewriteCatalogFiles(catalogs))
+  if (!RewriteCatalogFiles())
     return;
 
   ReportSuccess();
@@ -334,8 +334,14 @@ bool SandboxedExtensionUnpacker::RewriteImageFiles() {
   return true;
 }
 
-bool SandboxedExtensionUnpacker::RewriteCatalogFiles(
-    const DictionaryValue& catalogs) {
+bool SandboxedExtensionUnpacker::RewriteCatalogFiles() {
+  DictionaryValue catalogs;
+  if (!ExtensionUnpacker::ReadMessageCatalogsFromFile(temp_dir_.path(),
+                                                      &catalogs)) {
+    ReportFailure("Could not read catalog data from disk.");
+    return false;
+  }
+
   // Write our parsed catalogs back to disk.
   for (DictionaryValue::key_iterator key_it = catalogs.begin_keys();
        key_it != catalogs.end_keys(); ++key_it) {
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.h b/chrome/browser/extensions/sandboxed_extension_unpacker.h
index 9c04c9f..fd44a6a 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.h
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.h
@@ -121,8 +121,7 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
   void StartProcessOnIOThread(const FilePath& temp_crx_path);
 
   // SandboxedExtensionUnpacker
-  void OnUnpackExtensionSucceeded(const DictionaryValue& manifest,
-                                  const DictionaryValue& catalogs);
+  void OnUnpackExtensionSucceeded(const DictionaryValue& manifest);
   void OnUnpackExtensionFailed(const std::string& error_message);
   void OnProcessCrashed();
 
@@ -136,7 +135,7 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
   // Overwrites original files with safe results from utility process.
   // Reports error and returns false if it fails.
   bool RewriteImageFiles();
-  bool RewriteCatalogFiles(const DictionaryValue& parsed_catalogs);
+  bool RewriteCatalogFiles();
 
   FilePath crx_path_;
   ChromeThread::ID thread_identifier_;
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
index 3212080..f320cc0 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
@@ -104,8 +104,7 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
 
   void OnUnpackSucceeded() {
     sandboxed_unpacker_->OnUnpackExtensionSucceeded(
-        *unpacker_->parsed_manifest(),
-        *unpacker_->parsed_catalogs());
+        *unpacker_->parsed_manifest());
   }
 
   FilePath GetInstallPath() {
@@ -125,6 +124,7 @@ TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
   SetupUnpacker("no_l10n.crx");
   ASSERT_TRUE(unpacker_->Run());
   ASSERT_TRUE(unpacker_->DumpImagesToFile());
+  ASSERT_TRUE(unpacker_->DumpMessageCatalogsToFile());
 
   // Check that there is no _locales folder.
   FilePath install_path =
@@ -143,6 +143,7 @@ TEST_F(SandboxedExtensionUnpackerTest, WithCatalogsSuccess) {
   SetupUnpacker("good_l10n.crx");
   ASSERT_TRUE(unpacker_->Run());
   ASSERT_TRUE(unpacker_->DumpImagesToFile());
+  ASSERT_TRUE(unpacker_->DumpMessageCatalogsToFile());
 
   // Check timestamp on _locales/en_US/messages.json.
   FilePath messages_file;
diff --git a/chrome/browser/utility_process_host.h b/chrome/browser/utility_process_host.h
index 8448318..ab326e8 100644
--- a/chrome/browser/utility_process_host.h
+++ b/chrome/browser/utility_process_host.h
@@ -38,8 +38,7 @@ class UtilityProcessHost : public ChildProcessHost {
     // parsed manifest.json file.  |catalogs| contains list of all parsed
     // message catalogs.  |images| contains a list of decoded images and the
     // associated paths where those images live on disk.
-    virtual void OnUnpackExtensionSucceeded(const DictionaryValue& manifest,
-                                            const DictionaryValue& catalogs) {}
+    virtual void OnUnpackExtensionSucceeded(const DictionaryValue& manifest) {}
 
     // Called when an error occurred while unpacking the extension.
     // |error_message| contains a description of the problem.
diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc
index f79395c..8e5e6b6 100644
--- a/chrome/common/extensions/extension_unpacker.cc
+++ b/chrome/common/extensions/extension_unpacker.cc
@@ -35,6 +35,10 @@ const char kTempExtensionName[] = "TEMP_INSTALL";
 // The file to write our decoded images to, relative to the extension_path.
 const char kDecodedImagesFilename[] = "DECODED_IMAGES";
 
+// The file to write our decoded message catalogs to, relative to the
+// extension_path.
+const char kDecodedMessageCatalogsFilename[] = "DECODED_MESSAGE_CATALOGS";
+
 // Errors
 const char* kCouldNotCreateDirectoryError =
     "Could not create directory for unzipping.";
@@ -210,6 +214,21 @@ bool ExtensionUnpacker::DumpImagesToFile() {
   return true;
 }
 
+bool ExtensionUnpacker::DumpMessageCatalogsToFile() {
+  IPC::Message pickle;
+  IPC::WriteParam(&pickle, *parsed_catalogs_.get());
+
+  FilePath path = extension_path_.DirName().AppendASCII(
+      kDecodedMessageCatalogsFilename);
+  if (!file_util::WriteFile(path, static_cast<const char*>(pickle.data()),
+                            pickle.size())) {
+    SetError("Could not write message catalogs to disk.");
+    return false;
+  }
+
+  return true;
+}
+
 // static
 bool ExtensionUnpacker::ReadImagesFromFile(const FilePath& extension_path,
                                            DecodedImages* images) {
@@ -223,6 +242,19 @@ bool ExtensionUnpacker::ReadImagesFromFile(const FilePath& extension_path,
   return IPC::ReadParam(&pickle, &iter, images);
 }
 
+// static
+bool ExtensionUnpacker::ReadMessageCatalogsFromFile(
+    const FilePath& extension_path, DictionaryValue* catalogs) {
+  FilePath path = extension_path.AppendASCII(kDecodedMessageCatalogsFilename);
+  std::string file_str;
+  if (!file_util::ReadFileToString(path, &file_str))
+    return false;
+
+  IPC::Message pickle(file_str.data(), file_str.size());
+  void* iter = NULL;
+  return IPC::ReadParam(&pickle, &iter, catalogs);
+}
+
 bool ExtensionUnpacker::AddDecodedImage(const FilePath& path) {
   // Make sure it's not referencing a file outside the extension's subdir.
   if (path.IsAbsolute() || PathContainsParentDirectory(path)) {
@@ -243,9 +275,9 @@ bool ExtensionUnpacker::AddDecodedImage(const FilePath& path) {
 bool ExtensionUnpacker::ReadMessageCatalog(const FilePath& message_path) {
   std::string error;
   JSONFileValueSerializer serializer(message_path);
-  DictionaryValue* root =
-    static_cast<DictionaryValue*>(serializer.Deserialize(&error));
-  if (!root) {
+  scoped_ptr<DictionaryValue> root(
+      static_cast<DictionaryValue*>(serializer.Deserialize(&error)));
+  if (!root.get()) {
     std::string messages_file = WideToASCII(message_path.ToWStringHack());
     if (error.empty()) {
       // If file is missing, Deserialize will fail with empty error.
@@ -262,7 +294,8 @@ bool ExtensionUnpacker::ReadMessageCatalog(const FilePath& message_path) {
   if (!temp_install_dir_.AppendRelativePath(message_path, &relative_path))
     NOTREACHED();
 
-  parsed_catalogs_->Set(relative_path.DirName().ToWStringHack(), root);
+  parsed_catalogs_->Set(relative_path.DirName().ToWStringHack(),
+                        root.release());
 
   return true;
 }
diff --git a/chrome/common/extensions/extension_unpacker.h b/chrome/common/extensions/extension_unpacker.h
index c59262d..b0a4073 100644
--- a/chrome/common/extensions/extension_unpacker.h
+++ b/chrome/common/extensions/extension_unpacker.h
@@ -35,12 +35,23 @@ class ExtensionUnpacker {
   // success.
   bool DumpImagesToFile();
 
+  // Write the decoded messages to kDecodedMessageCatalogsFilename.  We do this
+  // instead of sending them over IPC, since they are so large.  Returns true on
+  // success.
+  bool DumpMessageCatalogsToFile();
+
   // Read the decoded images back from the file we saved them to.
   // |extension_path| is the path to the extension we unpacked that wrote the
   // data. Returns true on success.
   static bool ReadImagesFromFile(const FilePath& extension_path,
                                  DecodedImages* images);
 
+  // Read the decoded message catalogs back from the file we saved them to.
+  // |extension_path| is the path to the extension we unpacked that wrote the
+  // data. Returns true on success.
+  static bool ReadMessageCatalogsFromFile(const FilePath& extension_path,
+                                          DictionaryValue* catalogs);
+
   const std::string& error_message() { return error_message_; }
   DictionaryValue* parsed_manifest() {
     return parsed_manifest_.get();
diff --git a/chrome/common/utility_messages_internal.h b/chrome/common/utility_messages_internal.h
index 4933742..46da384 100644
--- a/chrome/common/utility_messages_internal.h
+++ b/chrome/common/utility_messages_internal.h
@@ -37,13 +37,12 @@ IPC_END_MESSAGES(Utility)
 IPC_BEGIN_MESSAGES(UtilityHost)
 
   // Reply when the utility process is done unpacking an extension.  |manifest|
-  // is the parsed manifest.json file.  |catalogs| is the list of all parsed
-  // message catalogs and relative paths to them.
-  // The unpacker should also have written out a file containing decoded images
-  // from the extension.  See ExtensionUnpacker for details.
-  IPC_MESSAGE_CONTROL2(UtilityHostMsg_UnpackExtension_Succeeded,
-                       DictionaryValue /* manifest */,
-                       DictionaryValue /* catalogs */)
+  // is the parsed manifest.json file.
+  // The unpacker should also have written out files containing the decoded
+  // images and message catalogs from the extension. See ExtensionUnpacker for
+  // details.
+  IPC_MESSAGE_CONTROL1(UtilityHostMsg_UnpackExtension_Succeeded,
+                       DictionaryValue /* manifest */)
 
   // Reply when the utility process has failed while unpacking an extension.
   // |error_message| is a user-displayable explanation of what went wrong.
diff --git a/chrome/utility/utility_thread.cc b/chrome/utility/utility_thread.cc
index 69f0790..0615efb 100644
--- a/chrome/utility/utility_thread.cc
+++ b/chrome/utility/utility_thread.cc
@@ -30,9 +30,10 @@ void UtilityThread::OnControlMessageReceived(const IPC::Message& msg) {
 
 void UtilityThread::OnUnpackExtension(const FilePath& extension_path) {
   ExtensionUnpacker unpacker(extension_path);
-  if (unpacker.Run() && unpacker.DumpImagesToFile()) {
+  if (unpacker.Run() && unpacker.DumpImagesToFile() &&
+      unpacker.DumpMessageCatalogsToFile()) {
     Send(new UtilityHostMsg_UnpackExtension_Succeeded(
-      *unpacker.parsed_manifest(), *unpacker.parsed_catalogs()));
+       *unpacker.parsed_manifest()));
   } else {
     Send(new UtilityHostMsg_UnpackExtension_Failed(unpacker.error_message()));
   }