Commit 30a0cda6 authored by Adrienne Walker's avatar Adrienne Walker Committed by Commit Bot

cc: Fix bug in ClientPaintCache::Purge

ClientPaintCache::Purge was not modifying the map that it was generating
PurgeData from.  This meant that to generate a budget to remove, it
would loop over the same element repeatedly and generate a large list
of the same item to purge.  (This is why this is intermintent and hard
to repro.)  However, it would then inform the service that this one
entry was removed, but the client would still think it was there.
If anything used it at that point, the client would assume it was
cached, but the service would fail because it would not be cached.

Bug: 906510
Change-Id: I71bf22833649875ea2486f5e8518b897f354b6c1
Reviewed-on: https://chromium-review.googlesource.com/c/1345418Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609932}
parent 60b97267
...@@ -48,6 +48,7 @@ void ClientPaintCache::Purge(PurgedData* purged_data) { ...@@ -48,6 +48,7 @@ void ClientPaintCache::Purge(PurgedData* purged_data) {
(*purged_data)[static_cast<uint32_t>(type)].push_back(id); (*purged_data)[static_cast<uint32_t>(type)].push_back(id);
DCHECK_GE(bytes_used_, it->second); DCHECK_GE(bytes_used_, it->second);
bytes_used_ -= it->second; bytes_used_ -= it->second;
cache_map_.Erase(it);
} }
} }
......
...@@ -47,14 +47,20 @@ TEST_P(PaintCacheTest, ClientBasic) { ...@@ -47,14 +47,20 @@ TEST_P(PaintCacheTest, ClientBasic) {
TEST_P(PaintCacheTest, ClientPurgeForBudgeting) { TEST_P(PaintCacheTest, ClientPurgeForBudgeting) {
ClientPaintCache client_cache(kDefaultBudget); ClientPaintCache client_cache(kDefaultBudget);
client_cache.Put(GetType(), 1u, kDefaultBudget); client_cache.Put(GetType(), 1u, kDefaultBudget - 100);
client_cache.Put(GetType(), 2u, kDefaultBudget); client_cache.Put(GetType(), 2u, kDefaultBudget);
client_cache.Put(GetType(), 3u, kDefaultBudget);
ClientPaintCache::PurgedData purged_data; ClientPaintCache::PurgedData purged_data;
client_cache.Purge(&purged_data); client_cache.Purge(&purged_data);
const auto& ids = purged_data[static_cast<uint32_t>(GetType())]; const auto& ids = purged_data[static_cast<uint32_t>(GetType())];
ASSERT_EQ(ids.size(), 1u); ASSERT_EQ(ids.size(), 2u);
EXPECT_EQ(ids[0], 1u); EXPECT_EQ(ids[0], 1u);
EXPECT_EQ(ids[1], 2u);
EXPECT_FALSE(client_cache.Get(GetType(), 1u));
EXPECT_FALSE(client_cache.Get(GetType(), 2u));
EXPECT_TRUE(client_cache.Get(GetType(), 3u));
} }
TEST_P(PaintCacheTest, ClientPurgeAll) { TEST_P(PaintCacheTest, ClientPurgeAll) {
......
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