Commit 0868b9fb authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

viz/mac: Fix surface invariants violation by recycled compositors

RecyclableCompositorMac owns a ui::Compositor, but the surface id
generation was owned by BrowserCompositorMac.

A RecyclableCompositorMac will be recycled by many different
BrowserCompositorMac, and as a result, it may have a non-monotonic-
increasing surface id.

Scope the surface id information to RecyclableCompositorMac, and
throw some of the related logic into a helper function.

Bug: 772576, 817827
Change-Id: Ic4299afa5670c96b51260917a3fafb2b692b3465
Reviewed-on: https://chromium-review.googlesource.com/1046045Reviewed-by: default avatarFady Samuel <fsamuel@chromium.org>
Reviewed-by: default avatarccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556342}
parent 690f3329
...@@ -197,12 +197,6 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient { ...@@ -197,12 +197,6 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient {
gfx::Size dfh_size_dip_; gfx::Size dfh_size_dip_;
display::Display dfh_display_; display::Display dfh_display_;
// The viz::ParentLocalSurfaceIdAllocator for the ui::Compositor dispenses
// viz::LocalSurfaceIds that are renderered into by the ui::Compositor.
viz::ParentLocalSurfaceIdAllocator compositor_local_surface_id_allocator_;
gfx::Size compositor_size_pixels_;
float compositor_scale_factor_ = 1.f;
// Used to disable screen updates while resizing (because frames are drawn in // Used to disable screen updates while resizing (because frames are drawn in
// the GPU process, they can end up appearing on-screen before our window // the GPU process, they can end up appearing on-screen before our window
// resizes). // resizes).
......
...@@ -75,6 +75,8 @@ class RecyclableCompositorMac : public ui::CompositorObserver { ...@@ -75,6 +75,8 @@ class RecyclableCompositorMac : public ui::CompositorObserver {
ui::AcceleratedWidgetMac* accelerated_widget_mac() { ui::AcceleratedWidgetMac* accelerated_widget_mac() {
return accelerated_widget_mac_.get(); return accelerated_widget_mac_.get();
} }
const gfx::Size pixel_size() const { return size_pixels_; }
float scale_factor() const { return scale_factor_; }
// Suspend will prevent the compositor from producing new frames. This should // Suspend will prevent the compositor from producing new frames. This should
// be called to avoid creating spurious frames while changing state. // be called to avoid creating spurious frames while changing state.
...@@ -82,6 +84,20 @@ class RecyclableCompositorMac : public ui::CompositorObserver { ...@@ -82,6 +84,20 @@ class RecyclableCompositorMac : public ui::CompositorObserver {
void Suspend(); void Suspend();
void Unsuspend(); void Unsuspend();
// Update the compositor's surface information, set |root_layer| to be the
// compositor root layer, and resize |root_layer|.
void UpdateSurfaceAndUnsuspend(const gfx::Size& size_pixels,
float scale_factor,
ui::Layer* root_layer);
// Invalidate the compositor's surface information.
void InvalidateSurface();
// The viz::ParentLocalSurfaceIdAllocator for the ui::Compositor dispenses
// viz::LocalSurfaceIds that are renderered into by the ui::Compositor.
viz::ParentLocalSurfaceIdAllocator local_surface_id_allocator_;
gfx::Size size_pixels_;
float scale_factor_ = 1.f;
private: private:
RecyclableCompositorMac(); RecyclableCompositorMac();
...@@ -129,6 +145,32 @@ void RecyclableCompositorMac::Unsuspend() { ...@@ -129,6 +145,32 @@ void RecyclableCompositorMac::Unsuspend() {
compositor_suspended_lock_ = nullptr; compositor_suspended_lock_ = nullptr;
} }
void RecyclableCompositorMac::UpdateSurfaceAndUnsuspend(
const gfx::Size& size_pixels,
float scale_factor,
ui::Layer* root_layer) {
if (size_pixels != size_pixels_ || scale_factor != scale_factor_) {
size_pixels_ = size_pixels;
scale_factor_ = scale_factor;
compositor()->SetScaleAndSize(scale_factor_, size_pixels_,
local_surface_id_allocator_.GenerateId());
}
compositor()->SetRootLayer(root_layer);
root_layer->SetBounds(
gfx::Rect(gfx::ConvertSizeToDIP(scale_factor, size_pixels)));
Unsuspend();
}
void RecyclableCompositorMac::InvalidateSurface() {
size_pixels_ = gfx::Size();
scale_factor_ = 1.f;
local_surface_id_allocator_.Invalidate();
compositor()->SetScaleAndSize(
scale_factor_, size_pixels_,
local_surface_id_allocator_.GetCurrentLocalSurfaceId());
compositor()->SetRootLayer(nullptr);
}
void RecyclableCompositorMac::OnCompositingDidCommit( void RecyclableCompositorMac::OnCompositingDidCommit(
ui::Compositor* compositor_that_did_commit) { ui::Compositor* compositor_that_did_commit) {
DCHECK_EQ(compositor_that_did_commit, compositor()); DCHECK_EQ(compositor_that_did_commit, compositor());
...@@ -353,7 +395,6 @@ void BrowserCompositorMac::TransitionToState(State new_state) { ...@@ -353,7 +395,6 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
// Transition HasNoCompositor -> HasDetachedCompositor. // Transition HasNoCompositor -> HasDetachedCompositor.
if (state_ == HasNoCompositor && new_state != HasNoCompositor) { if (state_ == HasNoCompositor && new_state != HasNoCompositor) {
recyclable_compositor_ = RecyclableCompositorMac::Create(); recyclable_compositor_ = RecyclableCompositorMac::Create();
recyclable_compositor_->compositor()->SetRootLayer(root_layer_.get());
recyclable_compositor_->compositor()->SetBackgroundColor(background_color_); recyclable_compositor_->compositor()->SetBackgroundColor(background_color_);
recyclable_compositor_->compositor()->SetDisplayColorSpace( recyclable_compositor_->compositor()->SetDisplayColorSpace(
dfh_display_.color_space()); dfh_display_.color_space());
...@@ -372,17 +413,9 @@ void BrowserCompositorMac::TransitionToState(State new_state) { ...@@ -372,17 +413,9 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
// now (if one is not ready, the compositor will unsuspend on first surface // now (if one is not ready, the compositor will unsuspend on first surface
// activation). // activation).
if (delegated_frame_host_->HasSavedFrame()) { if (delegated_frame_host_->HasSavedFrame()) {
if (compositor_scale_factor_ != dfh_display_.device_scale_factor() || recyclable_compositor_->UpdateSurfaceAndUnsuspend(
compositor_size_pixels_ != dfh_size_pixels_) { dfh_size_pixels_, dfh_display_.device_scale_factor(),
compositor_scale_factor_ = dfh_display_.device_scale_factor(); root_layer_.get());
compositor_size_pixels_ = dfh_size_pixels_;
root_layer_->SetBounds(gfx::Rect(gfx::ConvertSizeToDIP(
compositor_scale_factor_, compositor_size_pixels_)));
recyclable_compositor_->compositor()->SetScaleAndSize(
compositor_scale_factor_, compositor_size_pixels_,
compositor_local_surface_id_allocator_.GenerateId());
}
recyclable_compositor_->Unsuspend();
} }
state_ = HasAttachedCompositor; state_ = HasAttachedCompositor;
...@@ -403,13 +436,7 @@ void BrowserCompositorMac::TransitionToState(State new_state) { ...@@ -403,13 +436,7 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
// Transition HasDetachedCompositor -> HasNoCompositor. // Transition HasDetachedCompositor -> HasNoCompositor.
if (state_ == HasDetachedCompositor && new_state == HasNoCompositor) { if (state_ == HasDetachedCompositor && new_state == HasNoCompositor) {
recyclable_compositor_->accelerated_widget_mac()->ResetNSView(); recyclable_compositor_->accelerated_widget_mac()->ResetNSView();
compositor_scale_factor_ = 1.f; recyclable_compositor_->InvalidateSurface();
compositor_size_pixels_ = gfx::Size();
compositor_local_surface_id_allocator_.Invalidate();
recyclable_compositor_->compositor()->SetScaleAndSize(
compositor_scale_factor_, compositor_size_pixels_,
compositor_local_surface_id_allocator_.GetCurrentLocalSurfaceId());
recyclable_compositor_->compositor()->SetRootLayer(nullptr);
RecyclableCompositorMac::Recycle(std::move(recyclable_compositor_)); RecyclableCompositorMac::Recycle(std::move(recyclable_compositor_));
state_ = HasNoCompositor; state_ = HasNoCompositor;
} }
...@@ -469,27 +496,17 @@ void BrowserCompositorMac::OnFirstSurfaceActivation( ...@@ -469,27 +496,17 @@ void BrowserCompositorMac::OnFirstSurfaceActivation(
if (!recyclable_compositor_) if (!recyclable_compositor_)
return; return;
recyclable_compositor_->Unsuspend(); recyclable_compositor_->UpdateSurfaceAndUnsuspend(
surface_info.size_in_pixels(), surface_info.device_scale_factor(),
// Resize the compositor to match the current frame size, if needed. root_layer_.get());
if (compositor_size_pixels_ == surface_info.size_in_pixels() &&
compositor_scale_factor_ == surface_info.device_scale_factor()) {
return;
}
compositor_size_pixels_ = surface_info.size_in_pixels();
compositor_scale_factor_ = surface_info.device_scale_factor();
root_layer_->SetBounds(gfx::Rect(gfx::ConvertSizeToDIP(
compositor_scale_factor_, compositor_size_pixels_)));
recyclable_compositor_->compositor()->SetScaleAndSize(
compositor_scale_factor_, compositor_size_pixels_,
compositor_local_surface_id_allocator_.GenerateId());
// Disable screen updates until the frame of the new size appears (because the // Disable screen updates until the frame of the new size appears (because the
// content is drawn in the GPU process, it may change before we want it to). // content is drawn in the GPU process, it may change before we want it to).
if (repaint_state_ == RepaintState::Paused) { if (repaint_state_ == RepaintState::Paused) {
bool compositor_is_nsview_size = bool compositor_is_nsview_size =
compositor_size_pixels_ == dfh_size_pixels_ && recyclable_compositor_->pixel_size() == dfh_size_pixels_ &&
compositor_scale_factor_ == dfh_display_.device_scale_factor(); recyclable_compositor_->scale_factor() ==
dfh_display_.device_scale_factor();
if (compositor_is_nsview_size || repaint_auto_resize_enabled_) { if (compositor_is_nsview_size || repaint_auto_resize_enabled_) {
NSDisableScreenUpdates(); NSDisableScreenUpdates();
repaint_state_ = RepaintState::ScreenUpdatesDisabled; repaint_state_ = RepaintState::ScreenUpdatesDisabled;
......
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