Skip to content

Cherry pick PR #8050 to main: Reland "feat(cc): Disable resource pool reuse to reduce memory #7853" #11520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
niranjanyardi wants to merge 2 commits into from
Closed

Cherry pick PR #8050 to main: Reland "feat(cc): Disable resource pool reuse to reduce memory #7853" #11520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44829f7
Reland "feat(cc): Disable resource pool reuse to reduce memory #7853"…
sherryzy Nov 17, 2025
0b396c8
Rename OnMemoryPressure test for consistency
sherryzy Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cc/resources/resource_pool.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ ResourcePool::InUsePoolResource ResourcePool::AcquireResource(

if (!resource)
resource = CreateResource(size, format, color_space);

#if BUILDFLAG(IS_COBALT)
if (avoid_cc_reuse_resource) {
resource->mark_avoid_reuse();
Expand Down
64 changes: 64 additions & 0 deletions cc/resources/resource_pool_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ TEST_F(ResourcePoolTest, AcquireRelease) {
}

TEST_F(ResourcePoolTest, EventuallyEvictAndFlush) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
Comment on lines +92 to +94

@gemini-code-assist gemini-code-assist Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since GTEST_SKIP() internally behaves like a return statement, any code following it in the same test function will not be executed. Therefore, wrapping the entire test body in #else and #endif is unnecessary and adds preprocessor boilerplate.

You can simplify this pattern across all modified tests in this PR (including those in cc/resources/resource_pool_unittest.cc, cc/tiles/tile_manager_unittest.cc, and cc/trees/layer_tree_host_impl_unittest.cc) by only wrapping the GTEST_SKIP() call in the #if block, leaving the rest of the test body unwrapped.

For example:

TEST_F(ResourcePoolTest, EventuallyEvictAndFlush) {
#if BUILDFLAG(IS_COBALT)
  GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#endif
  gfx::Size size(100, 100);
  viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
  gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();
  ...
}

This significantly improves readability and maintainability by avoiding nested preprocessor blocks and reducing the diff size.

References
  1. Adhere to upstream Chromium style guides which favor clean, readable code with minimal preprocessor noise. Avoid wrapping entire function bodies in preprocessor conditionals when early returns or macros like GTEST_SKIP() can be used directly. (link)

gfx::Size size(100, 100);
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();
Expand All @@ -102,9 +105,13 @@ TEST_F(ResourcePoolTest, EventuallyEvictAndFlush) {
test_task_runner_->FastForwardBy(ResourcePool::kDefaultExpirationDelay +
ResourcePool::kDefaultMaxFlushDelay);
EXPECT_EQ(0u, resource_pool_->GetTotalResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, FlushEvenIfMoreUnusedToEvict) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
gfx::Size size(100, 100);
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();
Expand Down Expand Up @@ -147,9 +154,13 @@ TEST_F(ResourcePoolTest, FlushEvenIfMoreUnusedToEvict) {
EXPECT_CALL(*context_support_, FlushPendingWork()).Times(testing::AtLeast(1));
test_task_runner_->FastForwardBy(ResourcePool::kDefaultExpirationDelay * 100);
EXPECT_EQ(0u, resource_pool_->GetTotalResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, AccountingSingleResource) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -183,9 +194,13 @@ TEST_F(ResourcePoolTest, AccountingSingleResource) {
EXPECT_EQ(0u, resource_pool_->GetTotalResourceCountForTesting());
EXPECT_EQ(0u, resource_pool_->resource_count());
EXPECT_EQ(0u, resource_pool_->GetBusyResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, SimpleResourceReuse) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -222,9 +237,13 @@ TEST_F(ResourcePoolTest, SimpleResourceReuse) {
CheckAndReturnResource(std::move(resource));
EXPECT_EQ(3u, resource_pool_->GetTotalResourceCountForTesting());
EXPECT_EQ(0u, resource_pool_->GetBusyResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, LostResource) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -255,9 +274,13 @@ TEST_F(ResourcePoolTest, LostResource) {
EXPECT_EQ(1u, resource_pool_->GetTotalResourceCountForTesting());
resource_pool_->ReleaseResource(std::move(resource));
EXPECT_EQ(0u, resource_pool_->GetTotalResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, BusyResourcesNotFreed) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -302,9 +325,13 @@ TEST_F(ResourcePoolTest, BusyResourcesNotFreed) {
EXPECT_EQ(40000u, resource_pool_->GetTotalMemoryUsageForTesting());
EXPECT_EQ(0u, resource_pool_->memory_usage_bytes());
EXPECT_EQ(0u, resource_pool_->GetBusyResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, UnusedResourcesEventuallyFreed) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -349,9 +376,13 @@ TEST_F(ResourcePoolTest, UnusedResourcesEventuallyFreed) {
test_task_runner_->FastForwardBy(ResourcePool::kDefaultExpirationDelay * 10);

EXPECT_EQ(0u, resource_pool_->GetTotalMemoryUsageForTesting());
#endif
}

TEST_F(ResourcePoolTest, UpdateContentId) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
gfx::Size size(100, 100);
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space;
Expand All @@ -374,9 +405,13 @@ TEST_F(ResourcePoolTest, UpdateContentId) {
EXPECT_EQ(original_id, reacquired_resource.unique_id_for_testing());
EXPECT_EQ(new_invalidated_rect, invalidated_rect);
resource_pool_->ReleaseResource(std::move(reacquired_resource));
#endif
}

TEST_F(ResourcePoolTest, UpdateContentIdAndInvalidatedRect) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
gfx::Size size(100, 100);
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space;
Expand Down Expand Up @@ -420,9 +455,13 @@ TEST_F(ResourcePoolTest, UpdateContentIdAndInvalidatedRect) {
EXPECT_EQ(original_id, reacquired_resource.unique_id_for_testing());
EXPECT_EQ(expected_total_invalidated_rect, total_invalidated_rect);
resource_pool_->ReleaseResource(std::move(reacquired_resource));
#endif
}

TEST_F(ResourcePoolTest, LargeInvalidatedRect) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
gfx::Size size(100, 100);
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space;
Expand Down Expand Up @@ -454,9 +493,13 @@ TEST_F(ResourcePoolTest, LargeInvalidatedRect) {
&new_invalidated_rect, color_space);
EXPECT_TRUE(!!resource);
resource_pool_->ReleaseResource(std::move(resource));
#endif
}

TEST_F(ResourcePoolTest, ReuseResource) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();

Expand Down Expand Up @@ -519,9 +562,13 @@ TEST_F(ResourcePoolTest, ReuseResource) {
EXPECT_EQ(nullptr, resource_pool_->ReuseResource(gfx::Size(100, 100), format,
color_space));
CheckAndReturnResource(std::move(resource));
#endif
}

TEST_F(ResourcePoolTest, PurgedMemory) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -574,9 +621,13 @@ TEST_F(ResourcePoolTest, PurgedMemory) {
base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL);
EXPECT_EQ(0u, resource_pool_->GetTotalResourceCountForTesting());
EXPECT_EQ(0u, resource_pool_->GetBusyResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, InvalidateResources) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -648,9 +699,13 @@ TEST_F(ResourcePoolTest, InvalidateResources) {
resource_pool_->ReleaseResource(std::move(in_use_resource));
EXPECT_EQ(0u, resource_pool_->GetTotalResourceCountForTesting());
EXPECT_EQ(0u, resource_pool_->GetBusyResourceCountForTesting());
#endif
}

TEST_F(ResourcePoolTest, ExactRequestsRespected) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();

Expand Down Expand Up @@ -678,9 +733,13 @@ TEST_F(ResourcePoolTest, ExactRequestsRespected) {
EXPECT_EQ(nullptr, resource_pool_->ReuseResource(gfx::Size(100, 100), format,
color_space));
CheckAndReturnResource(std::move(resource));
#endif
}

TEST_F(ResourcePoolTest, MetadataSentToDisplayCompositor) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -728,9 +787,13 @@ TEST_F(ResourcePoolTest, MetadataSentToDisplayCompositor) {
EXPECT_TRUE(transfer[0].is_overlay_candidate);

resource_pool_->ReleaseResource(std::move(resource));
#endif
}

TEST_F(ResourcePoolTest, InvalidResource) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
// Limits high enough to not be hit by this test.
size_t bytes_limit = 10 * 1024 * 1024;
size_t count_limit = 100;
Expand Down Expand Up @@ -761,6 +824,7 @@ TEST_F(ResourcePoolTest, InvalidResource) {
resource = resource_pool_->AcquireResource(size, format, color_space);
EXPECT_FALSE(resource.backing());
resource_pool_->ReleaseResource(std::move(resource));
#endif
}

} // namespace cc
8 changes: 8 additions & 0 deletions cc/tiles/tile_manager_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2544,11 +2544,19 @@ void RunPartialTileDecodeCheck(std::unique_ptr<LayerTreeHostImpl> host_impl,
// Ensures that the tile manager successfully reuses tiles when partial
// raster is enabled.
TEST_F(PartialRasterTileManagerTest, PartialRasterSuccessfullyEnabled) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support partial tile raster.";
#else
RunPartialRasterCheck(TakeHostImpl(), true /* partial_raster_enabled */);
#endif
}

TEST_F(PartialRasterTileManagerTest, PartialTileImageDecode) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support partial tile raster.";
#else
RunPartialTileDecodeCheck(TakeHostImpl(), true /* partial_raster_enabled */);
#endif
}

TEST_F(PartialRasterTileManagerTest, CompleteTileImageDecode) {
Expand Down
4 changes: 4 additions & 0 deletions cc/trees/layer_tree_host_impl_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13093,6 +13093,9 @@ TEST_P(LayerTreeHostImplTest, ExternalTransformSetNeedsRedraw) {
}

TEST_P(LayerTreeHostImplTest, OnMemoryPressure) {
#if BUILDFLAG(IS_COBALT)
GTEST_SKIP() << "Cobalt doesn't support resource reuse.";
#else
gfx::Size size(200, 200);
viz::SharedImageFormat format = viz::SinglePlaneFormat::kRGBA_8888;
gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();
Expand All @@ -13119,6 +13122,7 @@ TEST_P(LayerTreeHostImplTest, OnMemoryPressure) {

// Memory usage after the memory pressure should be less than previous one.
EXPECT_LT(memory_usage_after_memory_pressure, current_memory_usage);
#endif
}

TEST_P(LayerTreeHostImplTest, OnDrawConstraintSetNeedsRedraw) {
Expand Down
Loading