diff options
| -rw-r--r-- | chrome/browser/themes/browser_theme_pack.cc | 147 | ||||
| -rw-r--r-- | chrome/browser/themes/browser_theme_pack.h | 19 | ||||
| -rw-r--r-- | chrome/browser/themes/browser_theme_pack_unittest.cc | 225 | ||||
| -rw-r--r-- | chrome/common/extensions/manifest_handlers/theme_handler.cc | 19 | ||||
| -rw-r--r-- | chrome/test/data/extensions/theme_hidpi/manifest.json | 1 |
5 files changed, 349 insertions, 62 deletions
diff --git a/chrome/browser/themes/browser_theme_pack.cc b/chrome/browser/themes/browser_theme_pack.cc index bf2c6a7..977b8e9 100644 --- a/chrome/browser/themes/browser_theme_pack.cc +++ b/chrome/browser/themes/browser_theme_pack.cc @@ -10,6 +10,7 @@ #include "base/memory/scoped_ptr.h" #include "base/stl_util.h" #include "base/string_util.h" +#include "base/strings/string_number_conversions.h" #include "base/threading/sequenced_worker_pool.h" #include "base/threading/thread_restrictions.h" #include "base/utf_string_conversions.h" @@ -567,6 +568,20 @@ scoped_refptr<BrowserThemePack> BrowserThemePack::BuildFromExtension( image_skia->MakeThreadSafe(); } + // Set ThemeImageSource on |images_on_ui_thread_| to resample the source + // image if a caller of BrowserThemePack::GetImageNamed() requests an + // ImageSkiaRep for a scale factor not specified by the theme author. + // Callers of BrowserThemePack::GetImageNamed() to be able to retrieve + // ImageSkiaReps for all supported scale factors. + for (ImageCache::iterator it = pack->images_on_ui_thread_.begin(); + it != pack->images_on_ui_thread_.end(); ++it) { + const gfx::ImageSkia source_image_skia = it->second.AsImageSkia(); + ThemeImageSource* source = new ThemeImageSource(source_image_skia); + // image_skia takes ownership of source. + gfx::ImageSkia image_skia(source, source_image_skia.size()); + it->second = gfx::Image(image_skia); + } + // The BrowserThemePack is now in a consistent state. return pack; } @@ -729,6 +744,7 @@ gfx::Image BrowserThemePack::GetImageNamed(int idr_id) { // TODO(pkotwicz): Do something better than loading the bitmaps // for all the scale factors associated with |idr_id|. + // See crbug.com/243831. gfx::ImageSkia source_image_skia; for (size_t i = 0; i < scale_factors_.size(); ++i) { scoped_refptr<base::RefCountedMemory> memory = @@ -1062,22 +1078,52 @@ void BrowserThemePack::ParseImageNamesFromJSON( for (DictionaryValue::Iterator iter(*images_value); !iter.IsAtEnd(); iter.Advance()) { - std::string val; - if (iter.value().GetAsString(&val)) { - int id = GetPersistentIDByName(iter.key()); - if (id != -1) - (*file_paths)[id] = images_path.AppendASCII(val); -#if defined(OS_WIN) && defined(USE_AURA) - id = GetPersistentIDByNameHelper(iter.key(), - kPersistingImagesWinDesktopAura, - kPersistingImagesWinDesktopAuraLength); - if (id != -1) - (*file_paths)[id] = images_path.AppendASCII(val); -#endif + if (iter.value().IsType(Value::TYPE_DICTIONARY)) { + const DictionaryValue* inner_value = NULL; + if (iter.value().GetAsDictionary(&inner_value)) { + for (DictionaryValue::Iterator inner_iter(*inner_value); + !inner_iter.IsAtEnd(); + inner_iter.Advance()) { + std::string name; + ui::ScaleFactor scale_factor = ui::SCALE_FACTOR_NONE; + if (GetScaleFactorFromManifestKey(inner_iter.key(), &scale_factor) && + inner_iter.value().IsType(Value::TYPE_STRING) && + inner_iter.value().GetAsString(&name)) { + AddFileAtScaleToMap(iter.key(), + scale_factor, + images_path.AppendASCII(name), + file_paths); + } + } + } + } else if (iter.value().IsType(Value::TYPE_STRING)) { + std::string name; + if (iter.value().GetAsString(&name)) { + AddFileAtScaleToMap(iter.key(), + ui::SCALE_FACTOR_100P, + images_path.AppendASCII(name), + file_paths); + } } } } +void BrowserThemePack::AddFileAtScaleToMap(const std::string& image_name, + ui::ScaleFactor scale_factor, + const base::FilePath& image_path, + FilePathMap* file_paths) const { + int id = GetPersistentIDByName(image_name); + if (id != -1) + (*file_paths)[id][scale_factor] = image_path; +#if defined(OS_WIN) && defined(USE_AURA) + id = GetPersistentIDByNameHelper(image_name, + kPersistingImagesWinDesktopAura, + kPersistingImagesWinDesktopAuraLength); + if (id != -1) + (*file_paths)[id][scale_factor] = image_path; +#endif +} + void BrowserThemePack::BuildSourceImagesArray(const FilePathMap& file_paths) { std::vector<int> ids; for (FilePathMap::const_iterator it = file_paths.begin(); @@ -1099,14 +1145,7 @@ bool BrowserThemePack::LoadRawBitmapsTo( for (FilePathMap::const_iterator it = file_paths.begin(); it != file_paths.end(); ++it) { - scoped_refptr<base::RefCountedMemory> raw_data(ReadFileData(it->second)); - if (!raw_data.get()) { - LOG(ERROR) << "Could not load theme image"; - return false; - } - int prs_id = it->first; - // Some images need to go directly into |image_memory_|. No modification is // necessary or desirable. bool is_copyable = false; @@ -1116,21 +1155,48 @@ bool BrowserThemePack::LoadRawBitmapsTo( break; } } - - if (is_copyable) { - int raw_id = GetRawIDByPersistentID(prs_id, ui::SCALE_FACTOR_100P); - image_memory_[raw_id] = raw_data; - } else if (raw_data.get() && raw_data->size()) { - // Decode the PNG. - SkBitmap bitmap; - if (gfx::PNGCodec::Decode(raw_data->front(), raw_data->size(), - &bitmap)) { - (*image_cache)[prs_id] = - gfx::Image(gfx::ImageSkia::CreateFrom1xBitmap(bitmap)); - } else { - NOTREACHED() << "Unable to decode theme image resource " << it->first; + gfx::ImageSkia image_skia; + for (int pass = 0; pass < 2; ++pass) { + // Two passes: In the first pass, we process only scale factor + // 100% and in the second pass all other scale factors. We + // process scale factor 100% first because the first image added + // in image_skia.AddRepresentation() determines the DIP size for + // all representations. + for (ScaleFactorToFileMap::const_iterator s2f = it->second.begin(); + s2f != it->second.end(); ++s2f) { + ui::ScaleFactor scale_factor = s2f->first; + if ((pass == 0 && scale_factor != ui::SCALE_FACTOR_100P) || + (pass == 1 && scale_factor == ui::SCALE_FACTOR_100P)) { + continue; + } + scoped_refptr<base::RefCountedMemory> raw_data( + ReadFileData(s2f->second)); + if (!raw_data.get() || !raw_data->size()) { + LOG(ERROR) << "Could not load theme image" + << " prs_id=" << prs_id + << " scale_factor_enum=" << scale_factor + << " file=" << s2f->second.value() + << (raw_data.get() ? " (zero size)" : " (read error)"); + return false; + } + if (is_copyable) { + int raw_id = GetRawIDByPersistentID(prs_id, scale_factor); + image_memory_[raw_id] = raw_data; + } else { + SkBitmap bitmap; + if (gfx::PNGCodec::Decode(raw_data->front(), raw_data->size(), + &bitmap)) { + image_skia.AddRepresentation( + gfx::ImageSkiaRep(bitmap, scale_factor)); + } else { + NOTREACHED() << "Unable to decode theme image resource " + << it->first; + } + } } } + if (!is_copyable && !image_skia.isNull()) + (*image_cache)[prs_id] = gfx::Image(image_skia); } return true; @@ -1361,3 +1427,20 @@ int BrowserThemePack::GetRawIDByPersistentID( } return -1; } + +bool BrowserThemePack::GetScaleFactorFromManifestKey( + const std::string& key, + ui::ScaleFactor* scale_factor) const { + int percent = 0; + if (base::StringToInt(key, &percent)) { + float scale = static_cast<float>(percent) / 100.0f; + ui::ScaleFactor factor = ui::GetScaleFactorFromScale(scale); + // To be valid the scale factor must be in use. + if (std::find(scale_factors_.begin(), scale_factors_.end(), factor) + != scale_factors_.end()) { + *scale_factor = factor; + return true; + } + } + return false; +} diff --git a/chrome/browser/themes/browser_theme_pack.h b/chrome/browser/themes/browser_theme_pack.h index bf9414f..8e02d7b 100644 --- a/chrome/browser/themes/browser_theme_pack.h +++ b/chrome/browser/themes/browser_theme_pack.h @@ -112,8 +112,11 @@ class BrowserThemePack : public base::RefCountedThreadSafe< // The type passed to ui::DataPack::WritePack. typedef std::map<uint16, base::StringPiece> RawDataForWriting; - // An association between an id and the base::FilePath that has the image data. - typedef std::map<int, base::FilePath> FilePathMap; + // Maps scale factors (enum values) to file paths. + typedef std::map<ui::ScaleFactor, base::FilePath> ScaleFactorToFileMap; + + // Maps image ids to maps of scale factors to file paths. + typedef std::map<int, ScaleFactorToFileMap> FilePathMap; // Default. Everything is empty. BrowserThemePack(); @@ -145,6 +148,12 @@ class BrowserThemePack : public base::RefCountedThreadSafe< const base::FilePath& images_path, FilePathMap* file_paths) const; + // Helper function to populate the FilePathMap. + void AddFileAtScaleToMap(const std::string& image_name, + ui::ScaleFactor scale_factor, + const base::FilePath& image_path, + FilePathMap* file_paths) const; + // Creates the data for |source_images_| from |file_paths|. void BuildSourceImagesArray(const FilePathMap& file_paths); @@ -203,6 +212,12 @@ class BrowserThemePack : public base::RefCountedThreadSafe< // |scale_factor| in memory. int GetRawIDByPersistentID(int prs_id, ui::ScaleFactor scale_factor) const; + // Returns true if the |key| specifies a valid scale (e.g. "100") and + // the corresponding scale factor is currently in use. If true, returns + // the scale factor in |scale_factor|. + bool GetScaleFactorFromManifestKey(const std::string& key, + ui::ScaleFactor* scale_factor) const; + // Data pack, if we have one. scoped_ptr<ui::DataPack> data_pack_; diff --git a/chrome/browser/themes/browser_theme_pack_unittest.cc b/chrome/browser/themes/browser_theme_pack_unittest.cc index 39fedaf..1c8bd78 100644 --- a/chrome/browser/themes/browser_theme_pack_unittest.cc +++ b/chrome/browser/themes/browser_theme_pack_unittest.cc @@ -16,10 +16,20 @@ #include "grit/theme_resources.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/gfx/color_utils.h" +#include "ui/gfx/image/image_skia.h" +#include "ui/gfx/image/image_skia_rep.h" using content::BrowserThread; using extensions::Extension; +// Maps scale factors (enum values) to file path. +// A similar typedef in BrowserThemePack is private. +typedef std::map<ui::ScaleFactor, base::FilePath> TestScaleFactorToFileMap; + +// Maps image ids to maps of scale factors to file paths. +// A similar typedef in BrowserThemePack is private. +typedef std::map<int, TestScaleFactorToFileMap> TestFilePathMap; + class BrowserThemePackTest : public ::testing::Test { public: BrowserThemePackTest() @@ -108,7 +118,7 @@ class BrowserThemePackTest : public ::testing::Test { } void ParseImageNamesJSON(const std::string& json, - std::map<int, base::FilePath>* out_file_paths) { + TestFilePathMap* out_file_paths) { scoped_ptr<Value> value(base::JSONReader::Read(json)); ASSERT_TRUE(value->IsType(Value::TYPE_DICTIONARY)); ParseImageNamesDictionary(static_cast<DictionaryValue*>(value.get()), @@ -117,7 +127,7 @@ class BrowserThemePackTest : public ::testing::Test { void ParseImageNamesDictionary( DictionaryValue* value, - std::map<int, base::FilePath>* out_file_paths) { + TestFilePathMap* out_file_paths) { theme_pack_->ParseImageNamesFromJSON(value, base::FilePath(), out_file_paths); @@ -125,11 +135,36 @@ class BrowserThemePackTest : public ::testing::Test { theme_pack_->BuildSourceImagesArray(*out_file_paths); } - bool LoadRawBitmapsTo(const std::map<int, base::FilePath>& out_file_paths) { + bool LoadRawBitmapsTo(const TestFilePathMap& out_file_paths) { return theme_pack_->LoadRawBitmapsTo(out_file_paths, &theme_pack_->images_on_ui_thread_); } + // This function returns void in order to be able use ASSERT_... + // The BrowserThemePack is returned in |pack|. + void BuildFromUnpackedExtension(const base::FilePath& extension_path, + scoped_refptr<BrowserThemePack>& pack) { + base::FilePath manifest_path = + extension_path.AppendASCII("manifest.json"); + std::string error; + JSONFileValueSerializer serializer(manifest_path); + scoped_ptr<DictionaryValue> valid_value( + static_cast<DictionaryValue*>(serializer.Deserialize(NULL, &error))); + EXPECT_EQ("", error); + ASSERT_TRUE(valid_value.get()); + scoped_refptr<Extension> extension( + Extension::Create( + extension_path, + extensions::Manifest::INVALID_LOCATION, + *valid_value, + Extension::REQUIRE_KEY, + &error)); + ASSERT_TRUE(extension.get()); + ASSERT_EQ("", error); + pack = BrowserThemePack::BuildFromExtension(extension.get()); + ASSERT_TRUE(pack.get()); + } + base::FilePath GetStarGazingPath() { base::FilePath test_path; if (!PathService::Get(chrome::DIR_TEST_DATA, &test_path)) { @@ -146,6 +181,17 @@ class BrowserThemePackTest : public ::testing::Test { return base::FilePath(test_path); } + base::FilePath GetHiDpiThemePath() { + base::FilePath test_path; + if (!PathService::Get(chrome::DIR_TEST_DATA, &test_path)) { + NOTREACHED(); + return test_path; + } + test_path = test_path.AppendASCII("extensions"); + test_path = test_path.AppendASCII("theme_hidpi"); + return base::FilePath(test_path); + } + // Verifies the data in star gazing. We do this multiple times for different // BrowserThemePack objects to make sure it works in generated and mmapped // mode correctly. @@ -194,6 +240,116 @@ class BrowserThemePackTest : public ::testing::Test { EXPECT_FALSE(pack->GetTint(ThemeProperties::TINT_FRAME, &actual)); } + void VerifyHiDpiTheme(BrowserThemePack* pack) { + // The high DPI theme defines the following images: + 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_TOOLBAR)); + + // The high DPI theme does not define the following images: + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_TAB_BACKGROUND)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_TAB_BACKGROUND)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_TAB_BACKGROUND_INCOGNITO)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_TAB_BACKGROUND_V)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_NTP_BACKGROUND)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_FRAME_OVERLAY)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_FRAME_OVERLAY_INACTIVE)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_BUTTON_BACKGROUND)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_NTP_ATTRIBUTION)); + EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_WINDOW_CONTROL_BACKGROUND)); + + // Compare some known pixel colors at know locations for a theme + // image where two different PNG files were specified for scales 100% + // and 200% respectively. + int idr = IDR_THEME_FRAME; + gfx::Image image = pack->GetImageNamed(idr); + EXPECT_FALSE(image.IsEmpty()); + const gfx::ImageSkia* image_skia = image.ToImageSkia(); + ASSERT_TRUE(image_skia); + // Scale 100%. + const gfx::ImageSkiaRep& rep1 = image_skia->GetRepresentation( + ui::SCALE_FACTOR_100P); + ASSERT_FALSE(rep1.is_null()); + EXPECT_EQ(80, rep1.sk_bitmap().width()); + EXPECT_EQ(80, rep1.sk_bitmap().height()); + rep1.sk_bitmap().lockPixels(); + EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep1.sk_bitmap().getColor( 4, 4)); + EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep1.sk_bitmap().getColor( 8, 8)); + EXPECT_EQ(SkColorSetRGB( 0, 241, 237), rep1.sk_bitmap().getColor(16, 16)); + EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep1.sk_bitmap().getColor(24, 24)); + EXPECT_EQ(SkColorSetRGB( 0, 241, 237), rep1.sk_bitmap().getColor(32, 32)); + rep1.sk_bitmap().unlockPixels(); + // Scale 200%. + const gfx::ImageSkiaRep& rep2 = image_skia->GetRepresentation( + ui::SCALE_FACTOR_200P); + ASSERT_FALSE(rep2.is_null()); + EXPECT_EQ(160, rep2.sk_bitmap().width()); + EXPECT_EQ(160, rep2.sk_bitmap().height()); + rep2.sk_bitmap().lockPixels(); + EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep2.sk_bitmap().getColor( 4, 4)); + EXPECT_EQ(SkColorSetRGB(223, 42, 0), rep2.sk_bitmap().getColor( 8, 8)); + EXPECT_EQ(SkColorSetRGB(223, 42, 0), rep2.sk_bitmap().getColor(16, 16)); + EXPECT_EQ(SkColorSetRGB(223, 42, 0), rep2.sk_bitmap().getColor(24, 24)); + EXPECT_EQ(SkColorSetRGB(255, 255, 255), rep2.sk_bitmap().getColor(32, 32)); + rep2.sk_bitmap().unlockPixels(); + + // TODO(sschmitz): I plan to remove the following (to the end of the fct) + // Reason: this test may be brittle. It depends on details of how we scale + // an 100% image to an 200% image. If there's filtering etc, this test would + // break. Also High DPI is new, but scaling from 100% to 200% is not new + // and need not be tested here. But in the interrim it is useful to verify + // that this image was scaled and did not appear in the input. + + // Compare pixel colors and locations for a theme image that had + // only one PNG file specified (for scale 100%). The representation + // for scale of 200% was computed. + idr = IDR_THEME_FRAME_INCOGNITO_INACTIVE; + image = pack->GetImageNamed(idr); + EXPECT_FALSE(image.IsEmpty()); + image_skia = image.ToImageSkia(); + ASSERT_TRUE(image_skia); + // Scale 100%. + const gfx::ImageSkiaRep& rep3 = image_skia->GetRepresentation( + ui::SCALE_FACTOR_100P); + ASSERT_FALSE(rep3.is_null()); + EXPECT_EQ(80, rep3.sk_bitmap().width()); + EXPECT_EQ(80, rep3.sk_bitmap().height()); + rep3.sk_bitmap().lockPixels(); + // We take samples of colors and locations along the diagonal whenever + // the color changes. Note these colors are slightly different from + // the input PNG file due to input processing. + std::vector<std::pair<int, SkColor> > normal; + int xy = 0; + SkColor color = rep3.sk_bitmap().getColor(xy, xy); + normal.push_back(std::make_pair(xy, color)); + for (int xy = 0; xy < 40; ++xy) { + SkColor next_color = rep3.sk_bitmap().getColor(xy, xy); + if (next_color != color) { + color = next_color; + normal.push_back(std::make_pair(xy, color)); + } + } + EXPECT_EQ(static_cast<size_t>(9), normal.size()); + rep3.sk_bitmap().unlockPixels(); + // Scale 200%. + const gfx::ImageSkiaRep& rep4 = image_skia->GetRepresentation( + ui::SCALE_FACTOR_200P); + ASSERT_FALSE(rep4.is_null()); + EXPECT_EQ(160, rep4.sk_bitmap().width()); + EXPECT_EQ(160, rep4.sk_bitmap().height()); + rep4.sk_bitmap().lockPixels(); + // We expect the same colors and at locations scaled by 2 + // since this bitmap was scaled by 2. + for (size_t i = 0; i < normal.size(); ++i) { + int xy = 2 * normal[i].first; + SkColor color = normal[i].second; + EXPECT_EQ(color, rep4.sk_bitmap().getColor(xy, xy)); + } + rep4.sk_bitmap().unlockPixels(); + } + base::MessageLoop message_loop; content::TestBrowserThread fake_ui_thread; content::TestBrowserThread fake_file_thread; @@ -302,7 +458,7 @@ TEST_F(BrowserThemePackTest, CanReadDisplayProperties) { TEST_F(BrowserThemePackTest, CanParsePaths) { std::string path_json = "{ \"theme_button_background\": \"one\", " " \"theme_toolbar\": \"two\" }"; - std::map<int, base::FilePath> out_file_paths; + TestFilePathMap out_file_paths; ParseImageNamesJSON(path_json, &out_file_paths); size_t expected_file_paths = 2u; @@ -315,15 +471,18 @@ TEST_F(BrowserThemePackTest, CanParsePaths) { // "12" and "5" are internal constants to BrowserThemePack and are // PRS_THEME_BUTTON_BACKGROUND and PRS_THEME_TOOLBAR, but they are // implementation details that shouldn't be exported. - EXPECT_TRUE(base::FilePath(FILE_PATH_LITERAL("one")) == out_file_paths[12]); - EXPECT_TRUE(base::FilePath(FILE_PATH_LITERAL("two")) == out_file_paths[5]); + // By default the scale factor is for 100%. + EXPECT_TRUE(base::FilePath(FILE_PATH_LITERAL("one")) == + out_file_paths[12][ui::SCALE_FACTOR_100P]); + EXPECT_TRUE(base::FilePath(FILE_PATH_LITERAL("two")) == + out_file_paths[5][ui::SCALE_FACTOR_100P]); } TEST_F(BrowserThemePackTest, InvalidPathNames) { std::string path_json = "{ \"wrong\": [1], " " \"theme_button_background\": \"one\", " " \"not_a_thing\": \"blah\" }"; - std::map<int, base::FilePath> out_file_paths; + TestFilePathMap out_file_paths; ParseImageNamesJSON(path_json, &out_file_paths); // We should have only parsed one valid path out of that mess above. @@ -361,7 +520,7 @@ TEST_F(BrowserThemePackTest, InvalidDisplayProperties) { // These three tests should just not cause a segmentation fault. TEST_F(BrowserThemePackTest, NullPaths) { - std::map<int, base::FilePath> out_file_paths; + TestFilePathMap out_file_paths; ParseImageNamesDictionary(NULL, &out_file_paths); } @@ -381,7 +540,7 @@ 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, base::FilePath> out_file_paths; + TestFilePathMap out_file_paths; ParseImageNamesJSON(images, &out_file_paths); EXPECT_TRUE(theme_pack_->HasCustomImage(IDR_THEME_FRAME)); @@ -390,7 +549,7 @@ TEST_F(BrowserThemePackTest, TestHasCustomImage) { TEST_F(BrowserThemePackTest, TestNonExistantImages) { std::string images = "{ \"theme_frame\": \"does_not_exist\" }"; - std::map<int, base::FilePath> out_file_paths; + TestFilePathMap out_file_paths; ParseImageNamesJSON(images, &out_file_paths); EXPECT_FALSE(LoadRawBitmapsTo(out_file_paths)); @@ -407,23 +566,8 @@ TEST_F(BrowserThemePackTest, CanBuildAndReadPack) { // Part 1: Build the pack from an extension. { base::FilePath star_gazing_path = GetStarGazingPath(); - base::FilePath manifest_path = - star_gazing_path.AppendASCII("manifest.json"); - std::string error; - JSONFileValueSerializer serializer(manifest_path); - scoped_ptr<DictionaryValue> valid_value( - static_cast<DictionaryValue*>(serializer.Deserialize(NULL, &error))); - EXPECT_EQ("", error); - ASSERT_TRUE(valid_value.get()); - scoped_refptr<Extension> extension(Extension::Create( - star_gazing_path, extensions::Manifest::INVALID_LOCATION, *valid_value, - Extension::REQUIRE_KEY, &error)); - ASSERT_TRUE(extension.get()); - ASSERT_EQ("", error); - - scoped_refptr<BrowserThemePack> pack( - BrowserThemePack::BuildFromExtension(extension.get())); - ASSERT_TRUE(pack.get()); + scoped_refptr<BrowserThemePack> pack; + BuildFromUnpackedExtension(star_gazing_path, pack); ASSERT_TRUE(pack->WriteToDisk(file)); VerifyStarGazing(pack.get()); } @@ -437,3 +581,30 @@ TEST_F(BrowserThemePackTest, CanBuildAndReadPack) { VerifyStarGazing(pack.get()); } } + +TEST_F(BrowserThemePackTest, HiDpiThemeTest) { + std::vector<ui::ScaleFactor> scale_factors; + scale_factors.push_back(ui::SCALE_FACTOR_100P); + scale_factors.push_back(ui::SCALE_FACTOR_200P); + ui::test::ScopedSetSupportedScaleFactors test_scale_factors(scale_factors); + base::ScopedTempDir dir; + ASSERT_TRUE(dir.CreateUniqueTempDir()); + base::FilePath file = dir.path().AppendASCII("theme_data.pak"); + + // Part 1: Build the pack from an extension. + { + base::FilePath hidpi_path = GetHiDpiThemePath(); + scoped_refptr<BrowserThemePack> pack; + BuildFromUnpackedExtension(hidpi_path, pack); + ASSERT_TRUE(pack->WriteToDisk(file)); + VerifyHiDpiTheme(pack.get()); + } + + // Part 2: Try to read back the data pack that we just wrote to disk. + { + scoped_refptr<BrowserThemePack> pack = + BrowserThemePack::BuildFromDataPack(file, "gllekhaobjnhgeag"); + ASSERT_TRUE(pack.get()); + VerifyHiDpiTheme(pack.get()); + } +} diff --git a/chrome/common/extensions/manifest_handlers/theme_handler.cc b/chrome/common/extensions/manifest_handlers/theme_handler.cc index ec2842f..e9a9a1e 100644 --- a/chrome/common/extensions/manifest_handlers/theme_handler.cc +++ b/chrome/common/extensions/manifest_handlers/theme_handler.cc @@ -30,7 +30,24 @@ bool LoadImages(const DictionaryValue* theme_value, // Validate that the images are all strings. for (DictionaryValue::Iterator iter(*images_value); !iter.IsAtEnd(); iter.Advance()) { - if (!iter.value().IsType(Value::TYPE_STRING)) { + // The value may be a dictionary of scales and files paths. + // Or the value may be a file path, in which case a scale + // of 100% is assumed. + if (iter.value().IsType(Value::TYPE_DICTIONARY)) { + const DictionaryValue* inner_value = NULL; + if (iter.value().GetAsDictionary(&inner_value)) { + for (DictionaryValue::Iterator inner_iter(*inner_value); + !inner_iter.IsAtEnd(); inner_iter.Advance()) { + if (!inner_iter.value().IsType(Value::TYPE_STRING)) { + *error = ASCIIToUTF16(errors::kInvalidThemeImages); + return false; + } + } + } else { + *error = ASCIIToUTF16(errors::kInvalidThemeImages); + return false; + } + } else if (!iter.value().IsType(Value::TYPE_STRING)) { *error = ASCIIToUTF16(errors::kInvalidThemeImages); return false; } diff --git a/chrome/test/data/extensions/theme_hidpi/manifest.json b/chrome/test/data/extensions/theme_hidpi/manifest.json index 01609de..d1cd16c 100644 --- a/chrome/test/data/extensions/theme_hidpi/manifest.json +++ b/chrome/test/data/extensions/theme_hidpi/manifest.json @@ -1,5 +1,6 @@ { "name" : "HighDPI", + "key" : "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDh+ndFpTRdmV69ZOwIFiHaPRSij7jDI/010cI1wqxHOZYc9TK5LdWbuPkQV1dGMNhfIltfYfEjcRxHerJKU72RZSyMpGd8Kvw2vnqws9pfjTMBGD9U0J+kNx3S+tawjM1jRkx0+WGSTISTd92hudOdrZcrtjeetixSiQDJu0++MQIDAQAB", "version" : "1.0", "manifest_version" : 2, "theme": { |