summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--app/theme_provider.h5
-rw-r--r--chrome/browser/browser_theme_pack.cc68
-rw-r--r--chrome/browser/browser_theme_pack.h33
-rw-r--r--chrome/browser/browser_theme_pack_unittest.cc11
-rw-r--r--chrome/browser/browser_theme_provider.cc2
5 files changed, 81 insertions, 38 deletions
diff --git a/app/theme_provider.h b/app/theme_provider.h
index 410e699..0bd7ae1 100644
--- a/app/theme_provider.h
+++ b/app/theme_provider.h
@@ -61,8 +61,9 @@ class ThemeProvider {
// doesn't provide a certain image, but custom themes might (badges, etc).
virtual bool HasCustomImage(int id) const = 0;
- // Reads the image data from the theme file into the specified
- // vector. Returns NULL on error.
+ // Reads the image data from the theme file into the specified vector. Only
+ // valid for un-themed resources and the themed IDR_THEME_NTP_* in most
+ // implementations of ThemeProvider. Returns NULL on error.
virtual RefCountedMemory* GetRawData(int id) const = 0;
#if defined(OS_LINUX) && !defined(TOOLKIT_VIEWS)
diff --git a/chrome/browser/browser_theme_pack.cc b/chrome/browser/browser_theme_pack.cc
index 78be2f9..638cca2 100644
--- a/chrome/browser/browser_theme_pack.cc
+++ b/chrome/browser/browser_theme_pack.cc
@@ -203,11 +203,13 @@ BrowserThemePack::~BrowserThemePack() {
delete [] display_properties_;
}
- STLDeleteValues(&image_cache_);
+ STLDeleteValues(&prepared_images_);
+ STLDeleteValues(&loaded_images_);
}
// static
BrowserThemePack* BrowserThemePack::BuildFromExtension(Extension* extension) {
+ DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
DCHECK(extension);
DCHECK(extension->IsTheme());
@@ -222,23 +224,19 @@ BrowserThemePack* BrowserThemePack::BuildFromExtension(Extension* extension) {
pack->ParseImageNamesFromJSON(extension->GetThemeImages(),
extension->path(),
&file_paths);
- pack->LoadRawBitmapsTo(file_paths, &pack->image_cache_);
+ pack->LoadRawBitmapsTo(file_paths, &pack->prepared_images_);
- pack->GenerateFrameImages(&pack->image_cache_);
+ pack->GenerateFrameImages(&pack->prepared_images_);
#if !defined(OS_MACOSX)
// OSX uses its own special buttons that are PDFs that do odd sorts of vector
// graphics tricks. Other platforms use bitmaps and we must pre-tint them.
pack->GenerateTintedButtons(
pack->GetTintInternal(BrowserThemeProvider::TINT_BUTTONS),
- &pack->image_cache_);
+ &pack->prepared_images_);
#endif
- pack->GenerateTabBackgroundImages(&pack->image_cache_);
-
- // Repack all the images from |image_cache_| into |image_memory_| for
- // writing to the data pack.
- pack->RepackImageCacheToImageMemory();
+ pack->GenerateTabBackgroundImages(&pack->prepared_images_);
// The BrowserThemePack is now in a consistent state.
return pack;
@@ -247,6 +245,7 @@ BrowserThemePack* BrowserThemePack::BuildFromExtension(Extension* extension) {
// static
scoped_refptr<BrowserThemePack> BrowserThemePack::BuildFromDataPack(
FilePath path, const std::string& expected_id) {
+ DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
scoped_refptr<BrowserThemePack> pack = new BrowserThemePack;
pack->data_pack_.reset(new base::DataPack);
@@ -291,8 +290,9 @@ scoped_refptr<BrowserThemePack> BrowserThemePack::BuildFromDataPack(
}
bool BrowserThemePack::WriteToDisk(FilePath path) const {
- std::map<uint32, base::StringPiece> resources;
-
+ DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+ // Add resources for each of the property arrays.
+ RawDataForWriting resources;
resources[kHeaderID] = base::StringPiece(
reinterpret_cast<const char*>(header_), sizeof(BrowserThemePackHeader));
resources[kTintsID] = base::StringPiece(
@@ -304,11 +304,11 @@ bool BrowserThemePack::WriteToDisk(FilePath path) const {
reinterpret_cast<const char*>(display_properties_),
sizeof(DisplayPropertyPair[kDisplayPropertySize]));
- for (RawImages::const_iterator it = image_memory_.begin();
- it != image_memory_.end(); ++it) {
- resources[it->first] = base::StringPiece(
- reinterpret_cast<const char*>(it->second->front()), it->second->size());
- }
+ AddRawImagesTo(image_memory_, &resources);
+
+ RawImages reencoded_images;
+ RepackImages(prepared_images_, &reencoded_images);
+ AddRawImagesTo(reencoded_images, &resources);
return base::DataPack::WritePack(path, resources);
}
@@ -356,8 +356,13 @@ bool BrowserThemePack::GetDisplayProperty(int id, int* result) const {
SkBitmap* BrowserThemePack::GetBitmapNamed(int id) const {
// Check our cache of prepared images, first.
- ImageCache::const_iterator image_iter = image_cache_.find(id);
- if (image_iter != image_cache_.end())
+ ImageCache::const_iterator image_iter = prepared_images_.find(id);
+ if (image_iter != prepared_images_.end())
+ return image_iter->second;
+
+ // Check if we've already loaded this image.
+ image_iter = loaded_images_.find(id);
+ if (image_iter != loaded_images_.end())
return image_iter->second;
scoped_refptr<RefCountedMemory> memory;
@@ -381,7 +386,7 @@ SkBitmap* BrowserThemePack::GetBitmapNamed(int id) const {
}
SkBitmap* ret = new SkBitmap(bitmap);
- image_cache_[id] = ret;
+ loaded_images_[id] = ret;
return ret;
}
@@ -409,7 +414,8 @@ bool BrowserThemePack::HasCustomImage(int id) const {
base::StringPiece ignored;
return data_pack_->GetStringPiece(id, &ignored);
} else {
- return image_memory_.count(id) > 0;
+ return prepared_images_.count(id) > 0 ||
+ image_memory_.count(id) > 0;
}
}
@@ -810,17 +816,17 @@ void BrowserThemePack::GenerateTabBackgroundImages(ImageCache* bitmaps) const {
MergeImageCaches(temp_output, bitmaps);
}
-void BrowserThemePack::RepackImageCacheToImageMemory() {
- // TODO(erg): This shouldn't be done on the main thread. This should be done
- // during the WriteToDisk() method, but will require some tricky locking.
- for (ImageCache::const_iterator it = image_cache_.begin();
- it != image_cache_.end(); ++it) {
+void BrowserThemePack::RepackImages(const ImageCache& images,
+ RawImages* reencoded_images) const {
+ DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+ for (ImageCache::const_iterator it = images.begin();
+ it != images.end(); ++it) {
std::vector<unsigned char> image_data;
if (!gfx::PNGCodec::EncodeBGRASkBitmap(*(it->second), false, &image_data)) {
NOTREACHED() << "Image file for resource " << it->first
<< " could not be encoded.";
} else {
- image_memory_[it->first] = RefCountedBytes::TakeVector(&image_data);
+ (*reencoded_images)[it->first] = RefCountedBytes::TakeVector(&image_data);
}
}
}
@@ -838,6 +844,16 @@ void BrowserThemePack::MergeImageCaches(
}
}
+void BrowserThemePack::AddRawImagesTo(const RawImages& images,
+ RawDataForWriting* out) const {
+ DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+ for (RawImages::const_iterator it = images.begin(); it != images.end();
+ ++it) {
+ (*out)[it->first] = base::StringPiece(
+ reinterpret_cast<const char*>(it->second->front()), it->second->size());
+ }
+}
+
color_utils::HSL BrowserThemePack::GetTintInternal(int id) const {
if (tints_) {
for (int i = 0; i < kTintArraySize; ++i) {
diff --git a/chrome/browser/browser_theme_pack.h b/chrome/browser/browser_theme_pack.h
index 9b30872..8e4bff1 100644
--- a/chrome/browser/browser_theme_pack.h
+++ b/chrome/browser/browser_theme_pack.h
@@ -32,7 +32,9 @@ class RefCountedMemory;
// A note on const-ness. All public, non-static methods are const. We do this
// because once we've constructed a BrowserThemePack through the
// BuildFromExtension() interface, we WriteToDisk() on a thread other than the
-// UI thread that consumes a BrowserThemePack.
+// UI thread that consumes a BrowserThemePack. There is no locking; thread
+// safety between the writing thread and the UI thread is ensured by having the
+// data be immutable.
class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
public:
~BrowserThemePack();
@@ -61,6 +63,9 @@ class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
bool GetColor(int id, SkColor* color) const;
bool GetDisplayProperty(int id, int* result) const;
SkBitmap* GetBitmapNamed(int id) const;
+
+ // Returns the raw PNG encoded data for IDR_THEME_NTP_*. This method is only
+ // supposed to work for the NTP attribution and background resources.
RefCountedMemory* GetRawData(int id) const;
// Whether this theme provides an image for |id|.
@@ -77,6 +82,9 @@ class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
// The raw PNG memory associated with a certain id.
typedef std::map<int, scoped_refptr<RefCountedMemory> > RawImages;
+ // The type passed to base::DataPack::WritePack.
+ typedef std::map<uint32, base::StringPiece> RawDataForWriting;
+
// Default. Everything is empty.
BrowserThemePack();
@@ -121,15 +129,19 @@ class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
// in |bitmaps|. Must be called after GenerateFrameImages().
void GenerateTabBackgroundImages(ImageCache* bitmaps) const;
- // Takes all the SkBitmaps in |image_cache_|, encodes them as PNGs and places
- // them in |image_memory_|.
- void RepackImageCacheToImageMemory();
+ // Takes all the SkBitmaps in |images|, encodes them as PNGs and places
+ // them in |reencoded_images|.
+ void RepackImages(const ImageCache& images,
+ RawImages* reencoded_images) const;
// Takes all images in |source| and puts them in |destination|, freeing any
// image already in |destination| that |source| would overwrite.
void MergeImageCaches(const ImageCache& source,
ImageCache* destination) const;
+ // Changes the RefCountedMemory based |images| into StringPiece data in |out|.
+ void AddRawImagesTo(const RawImages& images, RawDataForWriting* out) const;
+
// Retrieves the tint OR the default tint. Unlike the public interface, we
// always need to return a reasonable tint here, instead of partially
// querying if the tint exists.
@@ -180,10 +192,15 @@ class BrowserThemePack : public base::RefCountedThreadSafe<BrowserThemePack> {
// needs to be in |image_memory_|.
RawImages image_memory_;
- // Tinted (or otherwise prepared) images for passing out. BrowserThemePack
- // owns all these images. This cache isn't touched when we write our data to
- // a DataPack.
- mutable ImageCache image_cache_;
+ // An immutable cache of images generated in BuildFromExtension(). When this
+ // BrowserThemePack is generated from BuildFromDataPack(), this cache is
+ // empty. We separate the images from the images loaded from disk so that
+ // WriteToDisk()'s implementation doesn't need locks. There should be no IDs
+ // in |image_memory_| that are in |prepared_images_| or vice versa.
+ ImageCache prepared_images_;
+
+ // Loaded images. These are loaded from |image_memory_| or the |data_pack_|.
+ mutable ImageCache loaded_images_;
DISALLOW_COPY_AND_ASSIGN(BrowserThemePack);
};
diff --git a/chrome/browser/browser_theme_pack_unittest.cc b/chrome/browser/browser_theme_pack_unittest.cc
index 77da71b..7d780f2 100644
--- a/chrome/browser/browser_theme_pack_unittest.cc
+++ b/chrome/browser/browser_theme_pack_unittest.cc
@@ -20,7 +20,12 @@
class BrowserThemePackTest : public ::testing::Test {
public:
- BrowserThemePackTest() : theme_pack_(new BrowserThemePack) { }
+ BrowserThemePackTest()
+ : message_loop(),
+ fake_ui_thread(ChromeThread::UI, &message_loop),
+ fake_file_thread(ChromeThread::FILE, &message_loop),
+ theme_pack_(new BrowserThemePack) {
+ }
// Transformation for link underline colors.
SkColor BuildThirdOpacity(SkColor color_link) {
@@ -159,6 +164,10 @@ class BrowserThemePackTest : public ::testing::Test {
EXPECT_FALSE(pack->GetTint(BrowserThemeProvider::TINT_FRAME, &actual));
}
+ MessageLoop message_loop;
+ ChromeThread fake_ui_thread;
+ ChromeThread fake_file_thread;
+
scoped_refptr<BrowserThemePack> theme_pack_;
};
diff --git a/chrome/browser/browser_theme_provider.cc b/chrome/browser/browser_theme_provider.cc
index 3887c2f..338e42e 100644
--- a/chrome/browser/browser_theme_provider.cc
+++ b/chrome/browser/browser_theme_provider.cc
@@ -566,7 +566,7 @@ void BrowserThemeProvider::BuildFromExtension(Extension* extension) {
// Write the packed file to disk.
FilePath pack_path = extension->path().Append(chrome::kThemePackFilename);
- ChromeThread::PostTask(ChromeThread::IO, FROM_HERE,
+ ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
new WritePackToDiskTask(pack, pack_path));
SavePackName(pack_path);