Commit 2ce3871f authored by Stephen Chenney's avatar Stephen Chenney Committed by Commit Bot

[CI] Fix caching of image size in CSSGeneratedImageValue

The CSSGeneratedImageValue image and client cache has only
one slot for the size of the image associated with any given
client, but the client could be registered with an empty size
and a non-empty size. In such cases we would not over-write
the non-empty size with the empty size, despite having deleted
the non-empty image. Subsequently removing the client again
would try to remove the non-empty one instead.

This patch adds DCHECKs to catch these cases, and fixes the
logic to only store non-empty sizes. Comments are added explaining
the invariant that the client only be present at one non-empty size
at any given time.

R=fs@opera.com
BUG=803224

Change-Id: Ie5601aaffe6e3eace4ff4a989dc892bea4fd381b
Reviewed-on: https://chromium-review.googlesource.com/876900Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530674}
parent 073b0641
...@@ -45,18 +45,19 @@ Image* GeneratedImageCache::GetImage(const LayoutSize& size) const { ...@@ -45,18 +45,19 @@ Image* GeneratedImageCache::GetImage(const LayoutSize& size) const {
void GeneratedImageCache::PutImage(const LayoutSize& size, void GeneratedImageCache::PutImage(const LayoutSize& size,
scoped_refptr<Image> image) { scoped_refptr<Image> image) {
DCHECK(!size.IsEmpty());
images_.insert(size, std::move(image)); images_.insert(size, std::move(image));
} }
void GeneratedImageCache::AddSize(const LayoutSize& size) { void GeneratedImageCache::AddSize(const LayoutSize& size) {
if (size.IsEmpty()) DCHECK(!size.IsEmpty());
return;
sizes_.insert(size); sizes_.insert(size);
} }
void GeneratedImageCache::RemoveSize(const LayoutSize& size) { void GeneratedImageCache::RemoveSize(const LayoutSize& size) {
if (size.IsEmpty()) DCHECK(!size.IsEmpty());
return; DCHECK(sizes_.Contains(size));
DCHECK(images_.Contains(size));
if (sizes_.erase(size)) if (sizes_.erase(size))
images_.erase(size); images_.erase(size);
} }
...@@ -90,7 +91,10 @@ void CSSImageGeneratorValue::RemoveClient(const ImageResourceObserver* client) { ...@@ -90,7 +91,10 @@ void CSSImageGeneratorValue::RemoveClient(const ImageResourceObserver* client) {
SECURITY_DCHECK(it != clients_.end()); SECURITY_DCHECK(it != clients_.end());
SizeAndCount& size_count = it->value; SizeAndCount& size_count = it->value;
cached_images_.RemoveSize(size_count.size); if (!size_count.size.IsEmpty()) {
cached_images_.RemoveSize(size_count.size);
size_count.size = LayoutSize();
}
if (!--size_count.count) if (!--size_count.count)
clients_.erase(client); clients_.erase(client);
...@@ -108,12 +112,15 @@ Image* CSSImageGeneratorValue::GetImage(const ImageResourceObserver* client, ...@@ -108,12 +112,15 @@ Image* CSSImageGeneratorValue::GetImage(const ImageResourceObserver* client,
DCHECK(keep_alive_); DCHECK(keep_alive_);
SizeAndCount& size_count = it->value; SizeAndCount& size_count = it->value;
if (size_count.size != size) { if (size_count.size != size) {
cached_images_.RemoveSize(size_count.size); if (!size_count.size.IsEmpty()) {
cached_images_.AddSize(size); cached_images_.RemoveSize(size_count.size);
size_count.size = LayoutSize();
}
// If there's only one use for this client, then update the size. if (!size.IsEmpty()) {
if (size_count.count == 1) cached_images_.AddSize(size);
size_count.size = size; size_count.size = size;
}
} }
} }
return cached_images_.GetImage(size); return cached_images_.GetImage(size);
......
...@@ -61,7 +61,11 @@ struct SizeAndCount { ...@@ -61,7 +61,11 @@ struct SizeAndCount {
DISALLOW_NEW(); DISALLOW_NEW();
SizeAndCount() : size(), count(0) {} SizeAndCount() : size(), count(0) {}
// The non-zero size associated with this client. A client must only
// ever be present at one non-zero size, with as many zero sizes as it wants.
LayoutSize size; LayoutSize size;
// The net number of times this client has been added.
int count; int count;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment