This reverts r38541, which reverts previous reverts.

This will re-introduce a mac performance regression but will make the code
correct.

R=mirandac (over shoulder)
BUG=34775,34078
TEST=none

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

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

4 files changed, 72 insertions, 14 deletions

diff --git a/chrome/browser/browser_theme_pack.cc b/chrome/browser/browser_theme_pack.cc
index a3418ad..9f54abc 100644
--- a/chrome/browser/browser_theme_pack.cc
+++ b/chrome/browser/browser_theme_pack.cc
@@ -30,7 +30,7 @@ namespace {
 
 // Version number of the current theme pack. We just throw out and rebuild
 // theme packs that aren't int-equal to this.
-const int kThemePackVersion = 2;
+const int kThemePackVersion = 3;
 
 // IDs that are in the DataPack won't clash with the positive integer
 // int32_t. kHeaderID should always have the maximum value because we want the
@@ -41,6 +41,7 @@ const int kHeaderID = UINT_MAX - 1;
 const int kTintsID = UINT_MAX - 2;
 const int kColorsID = UINT_MAX - 3;
 const int kDisplayPropertiesID = UINT_MAX - 4;
+const int kSourceImagesID = UINT_MAX - 5;
 
 // Static size of the tint/color/display property arrays that are mmapped.
 const int kTintArraySize = 6;
@@ -336,6 +337,7 @@ BrowserThemePack::~BrowserThemePack() {
     delete [] tints_;
     delete [] colors_;
     delete [] display_properties_;
+    delete [] source_images_;
   }
 
   STLDeleteValues(&prepared_images_);
@@ -359,6 +361,8 @@ BrowserThemePack* BrowserThemePack::BuildFromExtension(Extension* extension) {
   pack->ParseImageNamesFromJSON(extension->GetThemeImages(),
                                 extension->path(),
                                 &file_paths);
+  pack->BuildSourceImagesArray(file_paths);
+
   if (!pack->LoadRawBitmapsTo(file_paths, &pack->prepared_images_))
     return NULL;
 
@@ -424,6 +428,11 @@ scoped_refptr<BrowserThemePack> BrowserThemePack::BuildFromDataPack(
   pack->display_properties_ = reinterpret_cast<DisplayPropertyPair*>(
       const_cast<char*>(pointer.data()));
 
+  if (!pack->data_pack_->GetStringPiece(kSourceImagesID, &pointer))
+    return NULL;
+  pack->source_images_ = reinterpret_cast<int*>(
+      const_cast<char*>(pointer.data()));
+
   return pack;
 }
 
@@ -442,6 +451,14 @@ bool BrowserThemePack::WriteToDisk(FilePath path) const {
       reinterpret_cast<const char*>(display_properties_),
       sizeof(DisplayPropertyPair[kDisplayPropertySize]));
 
+  int source_count = 1;
+  int* end = source_images_;
+  for (; *end != -1 ; end++)
+    source_count++;
+  resources[kSourceImagesID] = base::StringPiece(
+      reinterpret_cast<const char*>(source_images_),
+      source_count * sizeof(int));
+
   AddRawImagesTo(image_memory_, &resources);
 
   RawImages reencoded_images;
@@ -557,13 +574,13 @@ bool BrowserThemePack::HasCustomImage(int idr_id) const {
   if (prs_id == -1)
     return false;
 
-  if (data_pack_.get()) {
-    base::StringPiece ignored;
-    return data_pack_->GetStringPiece(prs_id, &ignored);
-  } else {
-    return prepared_images_.count(prs_id) > 0 ||
-        image_memory_.count(prs_id) > 0;
+  int* img = source_images_;
+  for (; *img != -1; ++img) {
+    if (*img == prs_id)
+      return true;
   }
+
+  return false;
 }
 
 // private:
@@ -572,7 +589,8 @@ BrowserThemePack::BrowserThemePack()
     : header_(NULL),
       tints_(NULL),
       colors_(NULL),
-      display_properties_(NULL) {
+      display_properties_(NULL),
+      source_images_(NULL) {
 }
 
 void BrowserThemePack::BuildHeader(Extension* extension) {
@@ -826,6 +844,18 @@ void BrowserThemePack::ParseImageNamesFromJSON(
   }
 }
 
+void BrowserThemePack::BuildSourceImagesArray(const FilePathMap& file_paths) {
+  std::vector<int> ids;
+  for (FilePathMap::const_iterator it = file_paths.begin();
+       it != file_paths.end(); ++it) {
+    ids.push_back(it->first);
+  }
+
+  source_images_ = new int[ids.size() + 1];
+  std::copy(ids.begin(), ids.end(), source_images_);
+  source_images_[ids.size()] = -1;
+}
+
 bool BrowserThemePack::LoadRawBitmapsTo(
     const FilePathMap& file_paths,
     ImageCache* raw_bitmaps) {
diff --git a/chrome/browser/browser_theme_pack.h b/chrome/browser/browser_theme_pack.h
index b97680a..e9327e4 100644
--- a/chrome/browser/browser_theme_pack.h
+++ b/chrome/browser/browser_theme_pack.h
@@ -116,6 +116,9 @@ class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
                                FilePath images_path,
                                FilePathMap* file_paths) const;
 
+  // Creates the data for |source_images_| from |file_paths|.
+  void BuildSourceImagesArray(const FilePathMap& file_paths);
+
   // Loads the unmodified bitmaps packed in the extension to SkBitmaps. Returns
   // true if all images loaded.
   bool LoadRawBitmapsTo(const FilePathMap& file_paths,
@@ -189,6 +192,10 @@ class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
     int32 id;
     int32 property;
   } *display_properties_;
+
+  // A list of included source images. A pointer to a -1 terminated array of
+  // our persistent IDs.
+  int* source_images_;
 #pragma pack(pop)
 
   // References to raw PNG data. This map isn't touched when |data_pack_| is
diff --git a/chrome/browser/browser_theme_pack_unittest.cc b/chrome/browser/browser_theme_pack_unittest.cc
index fdaea84..3068248 100644
--- a/chrome/browser/browser_theme_pack_unittest.cc
+++ b/chrome/browser/browser_theme_pack_unittest.cc
@@ -118,6 +118,9 @@ class BrowserThemePackTest : public ::testing::Test {
   void ParseImageNamesDictionary(DictionaryValue* value,
                                  std::map<int, FilePath>* out_file_paths) {
     theme_pack_->ParseImageNamesFromJSON(value, FilePath(), out_file_paths);
+
+    // Build the source image list for HasCustomImage().
+    theme_pack_->BuildSourceImagesArray(*out_file_paths);
   }
 
   bool LoadRawBitmapsTo(const std::map<int, FilePath>& out_file_paths) {
@@ -167,14 +170,21 @@ class BrowserThemePackTest : public ::testing::Test {
         BrowserThemeProvider::NTP_BACKGROUND_ALIGNMENT, &val));
     EXPECT_EQ(BrowserThemeProvider::ALIGN_TOP, val);
 
-    // Every theme should have the following images, because they need to be
-    // tinted.
+    // The stargazing theme defines the following images:
+    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_BUTTON_BACKGROUND));
     EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_FRAME));
-    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_FRAME_INACTIVE));
-    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_FRAME_INCOGNITO));
-    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_FRAME_INCOGNITO_INACTIVE));
+    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_NTP_BACKGROUND));
     EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_TAB_BACKGROUND));
-    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_TAB_BACKGROUND_INCOGNITO));
+    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_TOOLBAR));
+    EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_WINDOW_CONTROL_BACKGROUND));
+
+    // Here are a few images that we shouldn't expect because even though
+    // they're included in the theme pack, they were autogenerated and
+    // therefore shouldn't show up when calling HasCustomImage().
+    EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_FRAME_INACTIVE));
+    EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_FRAME_INCOGNITO));
+    EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_FRAME_INCOGNITO_INACTIVE));
+    EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_TAB_BACKGROUND_INCOGNITO));
 
     // Make sure we don't have phantom data.
     EXPECT_FALSE(pack->GetColor(BrowserThemeProvider::COLOR_CONTROL_BACKGROUND,
@@ -359,6 +369,17 @@ TEST_F(BrowserThemePackTest, NullDisplayProperties) {
   LoadDisplayPropertiesDictionary(NULL);
 }
 
+TEST_F(BrowserThemePackTest, TestHasCustomImage) {
+  // HasCustomImage should only return true for images that exist in the
+  // extension and not for autogenerated images.
+  std::string images = "{ \"theme_frame\": \"one\" }";
+  std::map<int, FilePath> out_file_paths;
+  ParseImageNamesJSON(images, &out_file_paths);
+
+  EXPECT_TRUE(theme_pack_->HasCustomImage(IDR_THEME_FRAME));
+  EXPECT_FALSE(theme_pack_->HasCustomImage(IDR_THEME_FRAME_INCOGNITO));
+}
+
 TEST_F(BrowserThemePackTest, TestNonExistantImages) {
   std::string images = "{ \"theme_frame\": \"does_not_exist\" }";
   std::map<int, FilePath> out_file_paths;
diff --git a/chrome/test/data/profiles/complex_theme/Default/Extensions/mblmlcbknbnfebdfjnolmcapmdofhmme/1.1/Cached Theme.pak b/chrome/test/data/profiles/complex_theme/Default/Extensions/mblmlcbknbnfebdfjnolmcapmdofhmme/1.1/Cached Theme.pak
index 228bb2b..07c8c66 100644
--- a/chrome/test/data/profiles/complex_theme/Default/Extensions/mblmlcbknbnfebdfjnolmcapmdofhmme/1.1/Cached Theme.pak
+++ b/chrome/test/data/profiles/complex_theme/Default/Extensions/mblmlcbknbnfebdfjnolmcapmdofhmme/1.1/Cached Theme.pak