Skip to content

Commit

Permalink
fix: dangling raw_ptr in OSRWHV destructor (#41117)
Browse files Browse the repository at this point in the history
`delegated_frame_host_` holds a pointer to `delegated_frame_host_client_`.
Since `delegated_frame_host_client_` was being destroyed first, that
pointer was dangling in the OSRWHV destructor.

Also, make these two unique_ptr fields `const` since they point to the
same objects for the lifespan of the OSRWHV.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
  • Loading branch information
trop[bot] and ckerr committed Jan 25, 2024
1 parent 3f85bd5 commit 370a2c7
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 11 deletions.
13 changes: 6 additions & 7 deletions shell/browser/osr/osr_render_widget_host_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ OffScreenRenderWidgetHostView::OffScreenRenderWidgetHostView(
frame_rate_(frame_rate),
size_(initial_size),
painting_(painting),
delegated_frame_host_client_{
std::make_unique<ElectronDelegatedFrameHostClient>(this)},
delegated_frame_host_{std::make_unique<content::DelegatedFrameHost>(
AllocateFrameSinkId(),
delegated_frame_host_client_.get(),
true /* should_register_frame_sink_id */)},
cursor_manager_(std::make_unique<content::CursorManager>(this)),
mouse_wheel_phase_handler_(this),
backing_(std::make_unique<SkBitmap>()) {
Expand All @@ -205,12 +211,6 @@ OffScreenRenderWidgetHostView::OffScreenRenderWidgetHostView(
compositor_allocator_.GenerateId();
compositor_surface_id_ = compositor_allocator_.GetCurrentLocalSurfaceId();

delegated_frame_host_client_ =
std::make_unique<ElectronDelegatedFrameHostClient>(this);
delegated_frame_host_ = std::make_unique<content::DelegatedFrameHost>(
AllocateFrameSinkId(), delegated_frame_host_client_.get(),
true /* should_register_frame_sink_id */);

root_layer_ = std::make_unique<ui::Layer>(ui::LAYER_SOLID_COLOR);

bool opaque = SkColorGetA(background_color_) == SK_AlphaOPAQUE;
Expand Down Expand Up @@ -247,7 +247,6 @@ OffScreenRenderWidgetHostView::~OffScreenRenderWidgetHostView() {
content::DelegatedFrameHost::HiddenCause::kOther);
delegated_frame_host_->DetachFromCompositor();

delegated_frame_host_.reset();
compositor_.reset();
root_layer_.reset();
}
Expand Down
13 changes: 9 additions & 4 deletions shell/browser/osr/osr_render_widget_host_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,22 @@ class OffScreenRenderWidgetHostView : public content::RenderWidgetHostViewBase,
viz::ParentLocalSurfaceIdAllocator compositor_allocator_;

std::unique_ptr<ui::Layer> root_layer_;

// depends-on: root_layer_
std::unique_ptr<ui::Compositor> compositor_;
std::unique_ptr<content::DelegatedFrameHost> delegated_frame_host_;

// depends-on: render_widget_host_, root_layer_
const std::unique_ptr<ElectronDelegatedFrameHostClient>
delegated_frame_host_client_;

// depends-on: delegated_frame_host_client_
const std::unique_ptr<content::DelegatedFrameHost> delegated_frame_host_;

std::unique_ptr<content::CursorManager> cursor_manager_;

raw_ptr<OffScreenHostDisplayClient> host_display_client_;
std::unique_ptr<OffScreenVideoConsumer> video_consumer_;

std::unique_ptr<ElectronDelegatedFrameHostClient>
delegated_frame_host_client_;

content::MouseWheelPhaseHandler mouse_wheel_phase_handler_;

// Latest capture sequence number which is incremented when the caller
Expand Down

0 comments on commit 370a2c7

Please sign in to comment.