Deal with themes that don't provide certain sections. Fixes crasher in new themes system.

BUG=30453
TEST=Installing https://chrome.google.com/extensions/detail/immljcnndlfdiajkpggdjbikmnibdhhi doesn't crash chrome.

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

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

2 files changed, 39 insertions, 8 deletions

diff --git a/chrome/browser/browser_theme_pack.cc b/chrome/browser/browser_theme_pack.cc
index 85801ae..78be2f9 100644
--- a/chrome/browser/browser_theme_pack.cc
+++ b/chrome/browser/browser_theme_pack.cc
@@ -222,7 +222,6 @@ BrowserThemePack* BrowserThemePack::BuildFromExtension(Extension* extension) {
   pack->ParseImageNamesFromJSON(extension->GetThemeImages(),
                                 extension->path(),
                                 &file_paths);
-
   pack->LoadRawBitmapsTo(file_paths, &pack->image_cache_);
 
   pack->GenerateFrameImages(&pack->image_cache_);
@@ -452,6 +451,9 @@ void BrowserThemePack::BuildTintsFromJSON(DictionaryValue* tints_value) {
     tints_[i].l = -1;
   }
 
+  if (!tints_value)
+    return;
+
   // Parse the incoming data from |tints_value| into an intermediary structure.
   std::map<int, color_utils::HSL> temp_tints;
   for (DictionaryValue::key_iterator iter(tints_value->begin_keys());
@@ -493,7 +495,8 @@ void BrowserThemePack::BuildColorsFromJSON(DictionaryValue* colors_value) {
   }
 
   std::map<int, SkColor> temp_colors;
-  ReadColorsFromJSON(colors_value, &temp_colors);
+  if (colors_value)
+    ReadColorsFromJSON(colors_value, &temp_colors);
   GenerateMissingColors(&temp_colors);
 
   // Copy data from the intermediary data structure to the array.
@@ -608,6 +611,9 @@ void BrowserThemePack::BuildDisplayPropertiesFromJSON(
     display_properties_[i].property = 0;
   }
 
+  if (!display_properties_value)
+    return;
+
   std::map<int, int> temp_properties;
   for (DictionaryValue::key_iterator iter(
        display_properties_value->begin_keys());
@@ -653,6 +659,9 @@ void BrowserThemePack::ParseImageNamesFromJSON(
     DictionaryValue* images_value,
     FilePath images_path,
     std::map<int, FilePath>* file_paths) const {
+  if (!images_value)
+    return;
+
   for (DictionaryValue::key_iterator iter(images_value->begin_keys());
        iter != images_value->end_keys(); ++iter) {
     std::string val;
diff --git a/chrome/browser/browser_theme_pack_unittest.cc b/chrome/browser/browser_theme_pack_unittest.cc
index b8e5425..77da71b 100644
--- a/chrome/browser/browser_theme_pack_unittest.cc
+++ b/chrome/browser/browser_theme_pack_unittest.cc
@@ -73,22 +73,31 @@ class BrowserThemePackTest : public ::testing::Test {
   void LoadColorJSON(const std::string& json) {
     scoped_ptr<Value> value(base::JSONReader::Read(json, false));
     ASSERT_TRUE(value->IsType(Value::TYPE_DICTIONARY));
-    theme_pack_->BuildColorsFromJSON(
-        static_cast<DictionaryValue*>(value.get()));
+    LoadColorDictionary(static_cast<DictionaryValue*>(value.get()));
+  }
+
+  void LoadColorDictionary(DictionaryValue* value) {
+    theme_pack_->BuildColorsFromJSON(value);
   }
 
   void LoadTintJSON(const std::string& json) {
     scoped_ptr<Value> value(base::JSONReader::Read(json, false));
     ASSERT_TRUE(value->IsType(Value::TYPE_DICTIONARY));
-    theme_pack_->BuildTintsFromJSON(
-        static_cast<DictionaryValue*>(value.get()));
+    LoadTintDictionary(static_cast<DictionaryValue*>(value.get()));
+  }
+
+  void LoadTintDictionary(DictionaryValue* value) {
+    theme_pack_->BuildTintsFromJSON(value);
   }
 
   void LoadDisplayPropertiesJSON(const std::string& json) {
     scoped_ptr<Value> value(base::JSONReader::Read(json, false));
     ASSERT_TRUE(value->IsType(Value::TYPE_DICTIONARY));
-    theme_pack_->BuildDisplayPropertiesFromJSON(
-        static_cast<DictionaryValue*>(value.get()));
+    LoadDisplayPropertiesDictionary(static_cast<DictionaryValue*>(value.get()));
+  }
+
+  void LoadDisplayPropertiesDictionary(DictionaryValue* value) {
+    theme_pack_->BuildDisplayPropertiesFromJSON(value);
   }
 
   void ParseImageNames(const std::string& json,
@@ -293,6 +302,19 @@ TEST_F(BrowserThemePackTest, InvalidDisplayProperties) {
       BrowserThemeProvider::NTP_BACKGROUND_ALIGNMENT, &out_val));
 }
 
+// These three tests should just not cause a segmentation fault.
+TEST_F(BrowserThemePackTest, NullTints) {
+  LoadTintDictionary(NULL);
+}
+
+TEST_F(BrowserThemePackTest, NullColors) {
+  LoadColorDictionary(NULL);
+}
+
+TEST_F(BrowserThemePackTest, NullDisplayProperties) {
+  LoadDisplayPropertiesDictionary(NULL);
+}
+
 // TODO(erg): This test should actually test more of the built resources from
 // the extension data, but for now, exists so valgrind can test some of the
 // tricky memory stuff that BrowserThemePack does.