Skip to content

Commit 00b672a

Browse files
committed
[BlinkGenPropertyTrees] Fix crash when handling fullscreen pepper layer
RenderWidgetFullscreenPepper manages a cc layer without letting blink know it, so the layer's paint property states are not set, causing crashes. This CL is temporary, by forcing cc::PropertyTreeBuilder to initialize the paint properties of the pepper fullscreen root layer that is not managed by blink. TODO(crbug.com/925855): We should let blink manage the paint properties for all content cc::Layers. Test: https://chromium-review.googlesource.com/c/chromium/src/+/1437182 passes all CQ bots. [email protected] (cherry picked from commit 41d634c) Bug: 913464,925855 Change-Id: I3438562f4671dce5224f8180616e2fe2af2f6cae Reviewed-on: https://chromium-review.googlesource.com/c/1436493 Commit-Queue: Xianzhu Wang <[email protected]> Reviewed-by: vmpstr <[email protected]> Reviewed-by: Antoine Labour <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#626745} Reviewed-on: https://chromium-review.googlesource.com/c/1444392 Reviewed-by: Xianzhu Wang <[email protected]> Cr-Commit-Position: refs/branch-heads/3683@{#52} Cr-Branched-From: e510299-refs/heads/master@{#625896}
1 parent 3a68da0 commit 00b672a

File tree

5 files changed

+45
-7
lines changed

5 files changed

+45
-7
lines changed

cc/trees/layer_tree_host.cc

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ void LayerTreeHost::UpdateDeferMainFrameUpdateInternal() {
439439
}
440440

441441
bool LayerTreeHost::IsUsingLayerLists() const {
442-
return settings_.use_layer_lists;
442+
return settings_.use_layer_lists && !force_use_property_tree_builder_;
443443
}
444444

445445
void LayerTreeHost::CommitComplete() {
@@ -662,10 +662,11 @@ bool LayerTreeHost::UpdateLayers() {
662662
property_trees_.clear();
663663
return false;
664664
}
665+
665666
DCHECK(!root_layer()->parent());
666667
base::ElapsedTimer timer;
667668

668-
bool result = DoUpdateLayers(root_layer());
669+
bool result = DoUpdateLayers();
669670
micro_benchmark_controller_.DidUpdateLayers();
670671

671672
if (const char* client_name = GetClientNameForMetrics()) {
@@ -736,7 +737,7 @@ void LayerTreeHost::RecordGpuRasterizationHistogram(
736737
gpu_rasterization_histogram_recorded_ = true;
737738
}
738739

739-
bool LayerTreeHost::DoUpdateLayers(Layer* root_layer) {
740+
bool LayerTreeHost::DoUpdateLayers() {
740741
TRACE_EVENT1("cc,benchmark", "LayerTreeHost::DoUpdateLayers",
741742
"source_frame_number", SourceFrameNumber());
742743

@@ -750,13 +751,13 @@ bool LayerTreeHost::DoUpdateLayers(Layer* root_layer) {
750751
if (!IsUsingLayerLists()) {
751752
TRACE_EVENT0("cc", "LayerTreeHost::UpdateLayers::BuildPropertyTrees");
752753
Layer* root_scroll =
753-
PropertyTreeBuilder::FindFirstScrollableLayer(root_layer);
754+
PropertyTreeBuilder::FindFirstScrollableLayer(root_layer_.get());
754755
Layer* page_scale_layer = viewport_layers_.page_scale.get();
755756
if (!page_scale_layer && root_scroll)
756757
page_scale_layer = root_scroll->parent();
757758
gfx::Transform identity_transform;
758759
PropertyTreeBuilder::BuildPropertyTrees(
759-
root_layer, page_scale_layer, inner_viewport_scroll_layer(),
760+
root_layer_.get(), page_scale_layer, inner_viewport_scroll_layer(),
760761
outer_viewport_scroll_layer(), overscroll_elasticity_element_id(),
761762
elastic_overscroll_, page_scale_factor_, device_scale_factor_,
762763
gfx::Rect(device_viewport_size_), identity_transform, &property_trees_);
@@ -792,6 +793,11 @@ bool LayerTreeHost::DoUpdateLayers(Layer* root_layer) {
792793
DCHECK(property_trees_.clip_tree.Node(layer->clip_tree_index()));
793794
DCHECK(property_trees_.scroll_tree.Node(layer->scroll_tree_index()));
794795
}
796+
#else
797+
// This is a quick sanity check for readiness of paint properties.
798+
// TODO(crbug.com/913464): This is to help analysis of crashes of the bug.
799+
// Remove this CHECK when we close the bug.
800+
CHECK(property_trees_.effect_tree.Node(root_layer_->effect_tree_index()));
795801
#endif
796802

797803
draw_property_utils::UpdatePropertyTrees(this, &property_trees_);
@@ -1047,9 +1053,20 @@ void LayerTreeHost::SetRootLayer(scoped_refptr<Layer> root_layer) {
10471053
content_has_non_aa_paint_ = false;
10481054
gpu_rasterization_histogram_recorded_ = false;
10491055

1056+
force_use_property_tree_builder_ = false;
1057+
10501058
SetNeedsFullTreeSync();
10511059
}
10521060

1061+
void LayerTreeHost::SetNonBlinkManagedRootLayer(
1062+
scoped_refptr<Layer> root_layer) {
1063+
SetRootLayer(std::move(root_layer));
1064+
1065+
DCHECK(root_layer_->children().empty());
1066+
if (IsUsingLayerLists() && root_layer_)
1067+
force_use_property_tree_builder_ = true;
1068+
}
1069+
10531070
LayerTreeHost::ViewportLayers::ViewportLayers() = default;
10541071

10551072
LayerTreeHost::ViewportLayers::~ViewportLayers() = default;

cc/trees/layer_tree_host.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,14 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
309309
Layer* root_layer() { return root_layer_.get(); }
310310
const Layer* root_layer() const { return root_layer_.get(); }
311311

312+
// Sets the root layer which is not managed by blink, and we will initialize
313+
// its paint properties using PropertyTreeBuilder. For ui::Compositor, because
314+
// for now we always use PropertyTreeBulder, this function is equivalent to
315+
// SetRootLayer().
316+
// TODO(crbug.com/925855): This is temporary. Eventually we should let the
317+
// caller inform blink about the layer and remove the function.
318+
void SetNonBlinkManagedRootLayer(scoped_refptr<Layer> root_layer);
319+
312320
// Viewport Layers are used to identify key layers to the compositor thread,
313321
// so that it can perform viewport-based scrolling independently, such as
314322
// for pinch-zoom or overscroll elasticity.
@@ -708,7 +716,7 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
708716
void ApplyPageScaleDeltaFromImplSide(float page_scale_delta);
709717
void InitializeProxy(std::unique_ptr<Proxy> proxy);
710718

711-
bool DoUpdateLayers(Layer* root_layer);
719+
bool DoUpdateLayers();
712720

713721
void UpdateDeferMainFrameUpdateInternal();
714722

@@ -843,6 +851,13 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
843851
// for every layer during property tree building.
844852
bool has_copy_request_ = false;
845853

854+
// When settings_.use_layer_lists is true, paint properties are generated by
855+
// blink and we don't use PropertyTreeBuilder, except that the root layer
856+
// is set by SetNonBlinkManagedRootLayer().
857+
// TODO(crbug.com/925855): Remove this field when removing
858+
// SetNonBlinkManagedRootLayer().
859+
bool force_use_property_tree_builder_ = false;
860+
846861
MutatorHost* mutator_host_;
847862

848863
std::vector<std::pair<PaintImage, base::OnceCallback<void(bool)>>>

content/renderer/compositor/layer_tree_view.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,11 @@ void LayerTreeView::ClearRootLayer() {
319319
layer_tree_host_->SetRootLayer(nullptr);
320320
}
321321

322+
void LayerTreeView::SetNonBlinkManagedRootLayer(
323+
scoped_refptr<cc::Layer> layer) {
324+
layer_tree_host_->SetNonBlinkManagedRootLayer(std::move(layer));
325+
}
326+
322327
cc::AnimationHost* LayerTreeView::CompositorAnimationHost() {
323328
return animation_host_.get();
324329
}

content/renderer/compositor/layer_tree_view.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ class LayerTreeView : public blink::WebLayerTreeView,
132132
// blink::WebLayerTreeView implementation.
133133
viz::FrameSinkId GetFrameSinkId() override;
134134
void SetRootLayer(scoped_refptr<cc::Layer> layer) override;
135+
void SetNonBlinkManagedRootLayer(scoped_refptr<cc::Layer> layer);
135136
void ClearRootLayer() override;
136137
cc::AnimationHost* CompositorAnimationHost() override;
137138
gfx::Size GetViewportSize() const override;

content/renderer/render_widget_fullscreen_pepper.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ void RenderWidgetFullscreenPepper::SetLayer(cc::Layer* layer) {
355355
}
356356
UpdateLayerBounds();
357357
layer_->SetIsDrawable(true);
358-
layer_tree_view()->SetRootLayer(layer_);
358+
layer_tree_view()->SetNonBlinkManagedRootLayer(layer_);
359359
}
360360

361361
bool RenderWidgetFullscreenPepper::OnMessageReceived(const IPC::Message& msg) {

0 commit comments

Comments
 (0)