Skip to content

Commit

Permalink
Add AutoDisallowEvents to PaintOpBuffer and fix up and remove Ass…
Browse files Browse the repository at this point in the history
…erts (#792)
  • Loading branch information
Domiii committed Jul 6, 2023
1 parent b9e52b8 commit c275c6e
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 49 deletions.
5 changes: 5 additions & 0 deletions cc/layers/picture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ void PictureLayer::PushPropertiesTo(
// TODO(enne): http://crbug.com/918126 debugging
CHECK(this);
if (!recording_source_.Read(*this)) {
recordreplay::Assert(
"[RUN-2104-2296] PictureLayer::PushPropertiesTo A");
bool valid_host = layer_tree_host();
bool has_parent = parent();
bool parent_has_host = parent() && parent()->layer_tree_host();
Expand All @@ -90,9 +92,12 @@ void PictureLayer::PushPropertiesTo(
base::debug::DumpWithoutCrashing();
}

recordreplay::Assert("[RUN-2104-2296] PictureLayer::PushPropertiesTo B");
layer_impl->UpdateRasterSource(
recording_source_.Read(*this)->CreateRasterSource(),
&last_updated_invalidation_.Write(*this), nullptr, nullptr);
recordreplay::Assert("[RUN-2104-2296] PictureLayer::PushPropertiesTo C");

DCHECK(last_updated_invalidation_.Read(*this).IsEmpty());
}

Expand Down
23 changes: 10 additions & 13 deletions cc/layers/picture_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,6 @@ PictureLayerImpl::PictureLayerImpl(LayerTreeImpl* tree_impl, int id)
}

PictureLayerImpl::~PictureLayerImpl() {
if (!recordreplay::AreEventsDisallowed()) {
recordreplay::Assert("[RUN-2104-2266] ~PictureLayerImpl %d %d %llu",
id(), recordreplay::PointerId(this),
element_id().GetStableId());
}
if (twin_layer_)
twin_layer_->twin_layer_ = nullptr;

Expand Down Expand Up @@ -755,10 +750,6 @@ void PictureLayerImpl::UpdateRasterSource(
Region* new_invalidation,
const PictureLayerTilingSet* pending_set,
const PaintWorkletRecordMap* pending_paint_worklet_records) {
// https://linear.app/replay/issue/RUN-885
recordreplay::Assert("PictureLayerImpl::UpdateRasterSource %d %d",
raster_source->GetSize().width(), raster_source->GetSize().height());

// The bounds and the pile size may differ if the pile wasn't updated (ie.
// PictureLayer::Update didn't happen). In that case the pile will be empty.
DCHECK(raster_source->GetSize().IsEmpty() ||
Expand Down Expand Up @@ -826,10 +817,6 @@ void PictureLayerImpl::UpdateRasterSource(

raster_source_->set_debug_name(DebugName());

// https://linear.app/replay/issue/RUN-885
recordreplay::Assert("PictureLayerImpl::UpdateRasterSource #5 %d %d",
raster_source_->GetSize().width(), raster_source_->GetSize().height());

// Register images from the new raster source, if the recording was updated.
// TODO(khushalsagar): UMA the number of animated images in layer?
if (recording_updated)
Expand Down Expand Up @@ -861,9 +848,15 @@ void PictureLayerImpl::UpdateRasterSource(
// this ends up running with the old LayerTreeFrameSink, or possibly with a
// null LayerTreeFrameSink, which can give incorrect results or maybe crash.
if (pending_set) {
recordreplay::Assert(
"[RUN-2104-2296] PictureLayerImpl::UpdateRasterSource C %d",
raster_source_->HasOneRef());
tilings_->UpdateTilingsToCurrentRasterSourceForActivation(
raster_source_, pending_set, invalidation_, MinimumContentsScale(),
MaximumContentsScale());
recordreplay::Assert(
"[RUN-2104-2296] PictureLayerImpl::UpdateRasterSource D %d",
raster_source_->HasOneRef());
} else {
tilings_->UpdateTilingsToCurrentRasterSourceForCommit(
raster_source_, invalidation_, MinimumContentsScale(),
Expand All @@ -873,6 +866,10 @@ void PictureLayerImpl::UpdateRasterSource(
layer_tree_impl()->UpdateImageDecodingHints(
raster_source_->TakeDecodingModeMap());
}

recordreplay::Assert(
"[RUN-2104-2296] PictureLayerImpl::UpdateRasterSource E %d",
raster_source_->HasOneRef());
}

void PictureLayerImpl::UpdateCanUseLCDText(
Expand Down
6 changes: 1 addition & 5 deletions cc/paint/display_item_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,7 @@ DisplayItemList::DisplayItemList(UsageHint usage_hint)
}
}

DisplayItemList::~DisplayItemList() {
if (!recordreplay::AreEventsDisallowed())
recordreplay::Assert("[RUN-2104-2266] ~DisplayItemList %d",
paint_op_buffer_.unique());
}
DisplayItemList::~DisplayItemList() = default;

void DisplayItemList::Raster(SkCanvas* canvas,
ImageProvider* image_provider) const {
Expand Down
7 changes: 1 addition & 6 deletions cc/paint/paint_op_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2921,10 +2921,7 @@ PaintOpBuffer::PaintOpBuffer(PaintOpBuffer&& other) {
}

PaintOpBuffer::~PaintOpBuffer() {
if (!recordreplay::AreEventsDisallowed()) {
recordreplay::Assert("[RUN-2104-2228] ~PaintOpBuffer %d",
are_ops_destroyed_);
}
recordreplay::AutoDisallowEvents disallow("~PaintOpBuffer");
Reset();
}

Expand Down Expand Up @@ -2956,11 +2953,9 @@ PaintOpBuffer& PaintOpBuffer::operator=(PaintOpBuffer&& other) {
void PaintOpBuffer::Reset() {
if (!are_ops_destroyed_) {
for (PaintOp& op : Iterator(this)) {
recordreplay::Assert("[RUN-2104-2266] PaintOpBuffer::Reset A %d", (int)op.GetType());
op.DestroyThis();
}
}
recordreplay::Assert("[RUN-2104-2266] PaintOpBuffer::Reset B");

// Leave data_ allocated, reserved_ unchanged. ShrinkToFit will take care of
// that if called.
Expand Down
4 changes: 1 addition & 3 deletions cc/paint/paint_shader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,7 @@ size_t PaintShader::GetSerializedSize(const PaintShader* shader) {
}

PaintShader::PaintShader(Type type) : shader_type_(type) {}
PaintShader::~PaintShader() {
recordreplay::AssertMaybeEventsDisallowed("[RUN-2104-2266] ~PaintShader %u", id_);
}
PaintShader::~PaintShader() = default;

bool PaintShader::has_discardable_images() const {
return (image_ && !image_.IsTextureBacked()) ||
Expand Down
7 changes: 1 addition & 6 deletions cc/raster/raster_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,7 @@ RasterSource::RasterSource(const RecordingSource* other)
other->slow_down_raster_scale_factor_for_debug_),
recording_scale_factor_(other->recording_scale_factor_) {}

RasterSource::~RasterSource() {
recordreplay::AssertMaybeEventsDisallowed(
"[RUN-2104-2266] ~RasterSource %s %d %d %d", debug_name_.c_str(),
!!display_list_, display_list_ && display_list_->HasOneRef(),
display_list_ && display_list_->HasAtLeastOneRef());
}
RasterSource::~RasterSource() = default;

void RasterSource::ClearForOpaqueRaster(
SkCanvas* raster_canvas,
Expand Down
2 changes: 2 additions & 0 deletions cc/resources/resource_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,8 @@ bool ResourcePool::OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,

void ResourcePool::OnMemoryPressure(
base::MemoryPressureListener::MemoryPressureLevel level) {
recordreplay::AutoDisallowEvents diallow("ResourcePool::OnMemoryPressure");

switch (level) {
case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:
case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
Expand Down
8 changes: 1 addition & 7 deletions cc/tiles/picture_layer_tiling_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,7 @@ PictureLayerTilingSet::PictureLayerTilingSet(
recordreplay::RegisterPointer("PictureLayerTilingSet", this);
}

PictureLayerTilingSet::~PictureLayerTilingSet() {
recordreplay::AssertMaybeEventsDisallowed(
"[RUN-2104-2266] ~PictureLayerTilingSet %d %d %d",
recordreplay::PointerId(this), !!raster_source_,
raster_source_ && raster_source_->HasOneRef());
recordreplay::UnregisterPointer(this);
}
PictureLayerTilingSet::~PictureLayerTilingSet() = default;

void PictureLayerTilingSet::CopyTilingsAndPropertiesFromPendingTwin(
const PictureLayerTilingSet* pending_twin_set,
Expand Down
3 changes: 0 additions & 3 deletions cc/tiles/tile_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1522,9 +1522,6 @@ void TileManager::CheckRasterFinishedQueries() {
}

void TileManager::FlushAndIssueSignals() {
if (!recordreplay::AreEventsDisallowed()) {
recordreplay::Assert("[RUN-2104-2228] TileManager::FlushAndIssueSignals");
}
TRACE_EVENT0("cc", "TileManager::FlushAndIssueSignals");
tile_task_manager_->CheckForCompletedTasks();
did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3748,6 +3748,9 @@ void LocalFrameView::DetachFromLayout() {
SetParentVisible(false);
SetAttached(false);

recordreplay::AssertMaybeEventsDisallowed(
"[RUN-2104-2296] LocalFrameView::DetachFromLayout");

// We may need update paint properties in detached frame subtree for printing.
// See UpdateLifecyclePhasesForPrinting().
if (auto* layout_view = GetLayoutView()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,9 @@
namespace blink {

DisplayItemList::~DisplayItemList() {
recordreplay::AssertMaybeEventsDisallowed(
"[RUN-2104-2266] ~DisplayItemList A %zu", MemoryUsageInBytes());
for (auto& item : *this) {
recordreplay::AssertMaybeEventsDisallowed(
"[RUN-2104-2266] ~DisplayItemList B %s",
item.GetId().ToString().Utf8().c_str());
item.Destruct();
}
recordreplay::AssertMaybeEventsDisallowed("[RUN-2104-2266] ~DisplayItemList C");
}

#if DCHECK_IS_ON()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ bool PaintController::UseCachedSubsequenceIfPossible(
}

num_cached_new_items_ += end_item_index - start_item_index;

recordreplay::AssertMaybeEventsDisallowed(
"[RUN-2104-2296] PaintController::UseCachedSubsequenceIfPossible %u %u",
(unsigned)num_cached_new_items_, (unsigned)num_cached_new_subsequences_);

++num_cached_new_subsequences_;

if (RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled()) {
Expand Down Expand Up @@ -430,6 +435,10 @@ void PaintController::UpdateCurrentPaintChunkProperties(

bool PaintController::ClientCacheIsValid(
const DisplayItemClient& client) const {
recordreplay::AssertMaybeEventsDisallowed(
"[RUN-2104-2296] PaintController::ClientCacheIsValid %u %d %d %d",
(unsigned)num_cached_new_items_, IsSkippingCache(), cache_is_all_invalid_,
client.IsValid());
if (IsSkippingCache() || cache_is_all_invalid_)
return false;
return client.IsValid();
Expand Down Expand Up @@ -638,6 +647,14 @@ void PaintController::CommitNewDisplayItems() {
num_cached_new_subsequences_;
}

recordreplay::Assert(
"[RUN-2104-2296] PaintController::CommitNewDisplayItems %u %u %u %u",
(unsigned)num_cached_new_items_, (unsigned)num_cached_new_subsequences_,
(unsigned)new_paint_artifact_->GetDisplayItemList().size(),
current_paint_artifact_
? (unsigned)current_paint_artifact_->GetDisplayItemList().size()
: 0);

num_cached_new_items_ = 0;
num_cached_new_subsequences_ = 0;
#if DCHECK_IS_ON()
Expand Down

0 comments on commit c275c6e

Please sign in to comment.