Commit bb5335de authored by danakj's avatar danakj Committed by Commit Bot

Remove SetVisible calls when setting the root layer in WebViewImpl.

The call in SetRootGraphicsLayer() comes from this CL:
https://codereview.chromium.org/1078473002/diff/100001/Source/web/WebViewImpl.cpp#newcode4168

In that CL it mentions that visibility is not always propagated to the
WebViewImpl, but in a racey way. At that time WebViewImpl would
initialize its compositor on line 452 after SetVisibilityState on line
449.

This means at startup the compositor's visibility would not be set. It
would only be toggled if the browser send a shown IPC to the renderer,
which it appears did not always end up arriving.

Things have changed significantly since then. We now SetVisible on the
compositor in order to start its scheduler. At the time we had a
separate start signal via SetLayerTreeHostClientReady(). So now
RenderWidget would set the compositor as visible if its marked as
shown when it is created.

The call in SetRootLayer was moved in 7e410b31 from
WebViewImpl::attachPaintArtifactCompositor() which included
-    // TODO(jbroman): This is cargo-culted from setRootGraphicsLayer. Is it
-    // necessary?
-    m_layerTreeView->setVisible(page()->isPageVisible());

So I would say no it is not.

R=enne@chromium.org

Change-Id: Icaea3fb4b164b9b4de348dd2e216c4a6a3344b60
Bug: 896836, 894899
Reviewed-on: https://chromium-review.googlesource.com/c/1298341Reviewed-by: default avatarenne <enne@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603265}
parent 91b547aa
......@@ -3172,12 +3172,6 @@ void WebViewImpl::SetRootGraphicsLayer(GraphicsLayer* graphics_layer) {
// We register viewport layers here since there may not be a layer
// tree view prior to this point.
RegisterViewportLayersWithCompositor();
// TODO(enne): Work around page visibility changes not being
// propagated to the WebView in some circumstances. This needs to
// be refreshed here when setting a new root layer to avoid being
// stuck in a presumed incorrectly invisible state.
layer_tree_view_->SetVisible(GetPage()->IsPageVisible());
} else {
root_graphics_layer_ = nullptr;
visual_viewport_container_layer_ = nullptr;
......@@ -3198,7 +3192,6 @@ void WebViewImpl::SetRootLayer(scoped_refptr<cc::Layer> layer) {
if (layer) {
root_layer_ = layer;
layer_tree_view_->SetRootLayer(root_layer_);
layer_tree_view_->SetVisible(GetPage()->IsPageVisible());
} else {
root_layer_ = nullptr;
// This means that we're transitioning to a new page. Suppress
......
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