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

4 files changed, 21 insertions, 16 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.