From 0decd626b2d57a363bba18f65b4ae151aaf403be Mon Sep 17 00:00:00 2001 From: Xianzhu Wang Date: Fri, 2 Jun 2023 03:52:04 +0000 Subject: [PATCH] Gracefully handle duplicated entries in PictureLayerImpl::SetPaintWorkletInputs() Clear the cached entry to nullopt so that the next duplicated entry will get a nullopt instead of an invalid PaintRecord. (cherry picked from commit 5c807677a349096ad0730b3d663e8c2b28b3ca59) Bug: 1442690 Change-Id: Id5089f36820d9deffba464f2b18594fb86123528 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4570147 Reviewed-by: Justin Novosad Commit-Queue: Xianzhu Wang Cr-Original-Commit-Position: refs/heads/main@{#1150362} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583309 Auto-Submit: Xianzhu Wang Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/branch-heads/5790@{#250} Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114} --- cc/layers/picture_layer_impl.cc | 3 +++ cc/layers/picture_layer_impl_unittest.cc | 31 ++++++++++++++++++++++++ cc/paint/paint_record.h | 7 +++++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc index 313eff53c35af..4018eb46f3c4f 100644 --- a/cc/layers/picture_layer_impl.cc +++ b/cc/layers/picture_layer_impl.cc @@ -2114,6 +2114,9 @@ void PictureLayerImpl::SetPaintWorkletInputs( // Attempt to re-use an existing PaintRecord if possible. new_records[input] = std::make_pair( paint_image_id, std::move(paint_worklet_records_[input].second)); + // The move constructor of absl::optional does not clear the source to + // nullopt. + paint_worklet_records_[input].second = absl::nullopt; } paint_worklet_records_.swap(new_records); diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc index d0891de2576e7..32d776234bd45 100644 --- a/cc/layers/picture_layer_impl_unittest.cc +++ b/cc/layers/picture_layer_impl_unittest.cc @@ -6260,6 +6260,37 @@ TEST_F(LegacySWPictureLayerImplTest, PaintWorkletInputs) { EXPECT_TRUE(pending_layer()->GetPaintWorkletRecordMap().contains(input3)); } +TEST_F(LegacySWPictureLayerImplTest, PaintWorkletInputsIdenticalEntries) { + gfx::Size layer_bounds(1000, 1000); + + auto recording_source = FakeRecordingSource::CreateRecordingSource( + gfx::Rect(layer_bounds), layer_bounds); + scoped_refptr input = + base::MakeRefCounted(gfx::SizeF(100, 100)); + PaintImage image = CreatePaintWorkletPaintImage(input); + recording_source->add_draw_image(image, gfx::Point(100, 100)); + recording_source->add_draw_image(image, gfx::Point(100, 100)); + recording_source->Rerecord(); + + // All inputs should be registered on the pending layer. + SetupPendingTree(recording_source->CreateRasterSource(), gfx::Size(), + Region(gfx::Rect(layer_bounds))); + EXPECT_EQ(pending_layer()->GetPaintWorkletRecordMap().size(), 1u); + EXPECT_TRUE(pending_layer()->GetPaintWorkletRecordMap().contains(input)); + + PaintRecord record; + pending_layer()->SetPaintWorkletRecord(input, record); + pending_layer()->picture_layer_tiling_set()->RemoveAllTiles(); + recording_source->Rerecord(); + pending_layer()->SetRasterSource(recording_source->CreateRasterSource(), + Region()); + EXPECT_EQ(pending_layer()->GetPaintWorkletRecordMap().size(), 1u); + auto it = pending_layer()->GetPaintWorkletRecordMap().find(input); + ASSERT_NE(it, pending_layer()->GetPaintWorkletRecordMap().end()); + // For now the paint record is cleared for multiple identical inputs. + EXPECT_FALSE(it->second.second.has_value()); +} + TEST_F(LegacySWPictureLayerImplTest, NoTilingsUsesScaleOne) { auto render_pass = viz::CompositorRenderPass::Create(); diff --git a/cc/paint/paint_record.h b/cc/paint/paint_record.h index 7afa6def956ab..9eaca49bd3638 100644 --- a/cc/paint/paint_record.h +++ b/cc/paint/paint_record.h @@ -25,6 +25,8 @@ class CC_PAINT_EXPORT PaintRecord { public: PaintRecord(); ~PaintRecord(); + // After move, the source is invalid (with nullptr buffer_) and can't be + // used anymore. PaintRecord(PaintRecord&&); PaintRecord& operator=(PaintRecord&&); PaintRecord(const PaintRecord&); @@ -34,7 +36,10 @@ class CC_PAINT_EXPORT PaintRecord { return buffer_->EqualsForTesting(*other.buffer_); } - const PaintOpBuffer& buffer() const { return *buffer_; } + const PaintOpBuffer& buffer() const { + CHECK(buffer_); + return *buffer_; + } size_t size() const { return buffer_->size(); } bool empty() const { return buffer_->empty(); }