Fix an issue where themes would sporadically fail to install.

Trying to send decoded images over IPC didn't work too well.  Instead, we'll
write them to a file and have the browser slurp them in from there.  My first
instinct was to use SharedMemory, but that would require us to impose a limit
on the size of the decoded image data.

Also made sure that the undecoded images are deleted when we install.

BUG=13455
TEST=Try the repro steps in bug 13455 several times and make sure it works
every time.
Review URL: http://codereview.chromium.org/119255

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

4 files changed, 77 insertions, 18 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index c641afc..ffed0d5 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -805,3 +805,25 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
 
   return true;
 }
+
+std::set<FilePath> Extension::GetBrowserImages() {
+  std::set<FilePath> image_paths;
+
+  DictionaryValue* theme_images = GetThemeImages();
+  if (theme_images) {
+    for (DictionaryValue::key_iterator it = theme_images->begin_keys();
+         it != theme_images->end_keys(); ++it) {
+      std::wstring val;
+      if (theme_images->GetString(*it, &val)) {
+        image_paths.insert(FilePath::FromWStringHack(val));
+      }
+    }
+  }
+
+  for (PageActionMap::const_iterator it = page_actions().begin();
+       it != page_actions().end(); ++it) {
+    image_paths.insert(it->second->icon_path());
+  }
+
+  return image_paths;
+}
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 9b79f30..245d7b6 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_COMMON_EXTENSIONS_EXTENSION_H_
 #define CHROME_COMMON_EXTENSIONS_EXTENSION_H_
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -202,6 +203,10 @@ class Extension {
   }
   bool IsTheme() { return is_theme_; }
 
+  // Returns a list of paths (relative to the extension dir) for images that
+  // the browser might load (like themes and page action icons).
+  std::set<FilePath> GetBrowserImages();
+
  private:
   // Helper method that loads a UserScript object from a
   // dictionary in the content_script list of the manifest.
diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc
index 916b048..bc7b25a 100644
--- a/chrome/common/extensions/extension_unpacker.cc
+++ b/chrome/common/extensions/extension_unpacker.cc
@@ -14,12 +14,13 @@
 #include "base/values.h"
 #include "net/base/file_stream.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/ipc_message_utils.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/common/zip.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";
@@ -28,6 +29,9 @@ const char kCurrentVersionFileName[] = "Current Version";
 // validation before finalizing install.
 const char kTempExtensionName[] = "TEMP_INSTALL";
 
+// The file to write our decoded images to, relative to the extension_path.
+const char kDecodedImagesFilename[] = "DECODED_IMAGES";
+
 // Chromium Extension magic number
 const char kExtensionFileMagic[] = "Cr24";
 
@@ -262,27 +266,43 @@ bool ExtensionUnpacker::Run() {
   }
 
   // Decode any images that the browser needs to display.
-  DictionaryValue* images = extension.GetThemeImages();
-  if (images) {
-    for (DictionaryValue::key_iterator it = images->begin_keys();
-         it != images->end_keys(); ++it) {
-      std::wstring val;
-      if (images->GetString(*it, &val)) {
-        if (!AddDecodedImage(FilePath::FromWStringHack(val)))
-          return false;  // Error was already reported.
-      }
-    }
+  std::set<FilePath> image_paths = extension.GetBrowserImages();
+  for (std::set<FilePath>::iterator it = image_paths.begin();
+       it != image_paths.end(); ++it) {
+    if (!AddDecodedImage(*it))
+      return false;  // Error was already reported.
   }
 
-  for (PageActionMap::const_iterator it = extension.page_actions().begin();
-       it != extension.page_actions().end(); ++it) {
-    if (!AddDecodedImage(it->second->icon_path()))
-      return false;  // Error was already reported.
+  return true;
+}
+
+bool ExtensionUnpacker::DumpImagesToFile() {
+  IPC::Message pickle;  // We use a Message so we can use WriteParam.
+  IPC::WriteParam(&pickle, decoded_images_);
+
+  FilePath path = extension_path_.DirName().AppendASCII(kDecodedImagesFilename);
+  if (!file_util::WriteFile(path, static_cast<const char*>(pickle.data()),
+                            pickle.size())) {
+    SetError("Could not write image data to disk.");
+    return false;
   }
 
   return true;
 }
 
+// static
+bool ExtensionUnpacker::ReadImagesFromFile(const FilePath& extension_path,
+                                           DecodedImages* images) {
+  FilePath path = extension_path.DirName().AppendASCII(kDecodedImagesFilename);
+  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, images);
+}
+
 bool ExtensionUnpacker::AddDecodedImage(const FilePath& path) {
   // Make sure it's not referencing a file outside the extension's subdir.
   if (path.IsAbsolute() || PathContainsParentDirectory(path)) {
diff --git a/chrome/common/extensions/extension_unpacker.h b/chrome/common/extensions/extension_unpacker.h
index 7e83363..65af750 100644
--- a/chrome/common/extensions/extension_unpacker.h
+++ b/chrome/common/extensions/extension_unpacker.h
@@ -15,9 +15,10 @@
 class DictionaryValue;
 class SkBitmap;
 
-// Implements IO for the ExtensionsService.
-// TODO(aa): Extract an interface out of this for testing the frontend, once the
-// frontend has significant logic to test.
+// This class unpacks an extension.  It is designed to be used in a sandboxed
+// child process.  We unpack and parse various bits of the extension, then
+// report back to the browser process, who then transcodes the pre-parsed bits
+// and writes them back out to disk for later use.
 class ExtensionUnpacker {
  public:
   typedef std::vector< Tuple2<SkBitmap, FilePath> > DecodedImages;
@@ -29,6 +30,17 @@ class ExtensionUnpacker {
   // Otherwise, error_message will contain a string explaining what went wrong.
   bool Run();
 
+  // Write the decoded images to kDecodedImagesFilename.  We do this instead
+  // of sending them over IPC, since they are so large.  Returns true on
+  // success.
+  bool DumpImagesToFile();
+
+  // 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);
+
   const std::string& error_message() { return error_message_; }
   DictionaryValue* parsed_manifest() {
     return parsed_manifest_.get();