Don't SetVisible on swapped out RenderWidgets, and drop gpu resources.

When a RenderWidget is created for a RenderViewImpl, it may be
considered "swapped out". This means the main frame for this
RenderView's frame tree is not local, the RenderView exists to
hold some other local subframe, which will be composited and have
a RenderWidget. Thus the RenderWidget attached to the RenderViewImpl
is not actually used, there is no output from the remote main frame
in this tree. I like to refer to these swapped out RenderWidgets as
zombies.

During navigations, a RenderViewImpl's main frame may change to or
from being local, in which case the RenderWidget attached to it changes
to or from being a zombie.

When becoming a zombie, the WebFrameWidget attached to the
RenderViewImpl (wrapping the WebViewImpl) is destroyed, and this
marks the compositor as SetVisible(false) on the RenderWidget
(via the WebViewImpl).

When becoming alive again, a new WebFrameWidget is attached to the
RenderViewImpl, and the WebFrameWidget marks the compositor as
SetVisible(true) on the RenderWidget (via the WebViewImpl).

While a zombie, the WebViewImpl is also told not to allow any
visibility changes to be passed along to the RenderWidget. This is
because as the local subframe becomes visible with the Page, we'd
tell the WebView about this, to tell the Page. But since the
WebView now has a zombie RenderWidget, it shouldn't also tell that.

Note when the RenderViewImpl is created and initially swapped out
it creates a RenderWidget but nothing tells its compositor about the
zombie state of affairs, so it becomes visible, which makes it
acquire a Gpu channel and start its scheduler etc. Woops.

This CL rearranges things a bit.
- A new RenderWidget does not SetVisible(true) if swapped out.
- RenderViewImpl directly marks it SetVisible(false) when removing the
WebFrameWidget.
- RenderViewImpl *also* removes the LayerTreeFrameSink to drop the
Gpu channel since we don't need to be fast at becoming visible (the
RenderWidget is a zombie!)
- RenderViewImpl directly marks it as SetVisible(true) when getting
a new WebFrameWidget as the RenderWidget stops being a zombie.
- WebViewImpl stops calling SetVisible on the RenderWidget when its
main frame is not visible. In the future the RenderWidget should be
a member of the main LocalFrame, and thus won't even exist when the
main frame is not local.

R=dcheng@chromium.org, piman@chromium.org

Change-Id: I87a5d0dfeaaf08cb91c9348b26b4205db55b3a81
Bug: 894899, 419087
Reviewed-on: https://chromium-review.googlesource.com/c/1290140Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602389}

content/renderer/gpu/layer_tree_view.cc

@@ -753,4 +753,8 @@ void LayerTreeView::SetURLForUkm(const GURL& url) {
   layer_tree_host_->SetURLForUkm(url);
 }
 
+void LayerTreeView::ReleaseLayerTreeFrameSink() {
+  layer_tree_host_->ReleaseLayerTreeFrameSink();
+}
+
 }  // namespace content

content/renderer/gpu/layer_tree_view.h

@@ -116,6 +116,12 @@ class CONTENT_EXPORT LayerTreeView
   void RequestNewLocalSurfaceId();
   void SetViewportVisibleRect(const gfx::Rect& visible_rect);
   void SetURLForUkm(const GURL& url);
+  // Call this if the compositor is becoming non-visible in a way that it won't
+  // be used any longer. In this case, becoming visible is longer but this
+  // releases more resources (such as its use of the GpuChannel).
+  // TODO(crbug.com/419087): This is to support a swapped out RenderWidget which
+  // should just be destroyed instead.
+  void ReleaseLayerTreeFrameSink();
 
   // blink::WebLayerTreeView implementation.
   viz::FrameSinkId GetFrameSinkId() override;

content/renderer/render_view_impl.cc

@@ -1092,6 +1092,12 @@ void RenderViewImpl::DidHandleGestureEventForWidget(
 
 void RenderViewImpl::OverrideCloseForWidget() {
   DCHECK(frame_widget_);
+  // The RenderWidget isn't actually closed here because we might need to use it
+  // again since it can't be recreated as it is part of |this|. So instead just
+  // stop the compositor.
+  // TODO(crbug.com/419087): The RenderWidget should be destroyed here along
+  // with the WebFrameWidget, then we won't have to do this.
+  GetWidget()->StopCompositor();
   frame_widget_->Close();
   frame_widget_ = nullptr;
 }
@@ -1538,6 +1544,12 @@ void RenderViewImpl::AttachWebFrameWidget(blink::WebFrameWidget* frame_widget) {
   // The previous WebFrameWidget must already be detached by CloseForFrame().
   DCHECK(!frame_widget_);
   frame_widget_ = frame_widget;
+  // In CloseForFrame() the RenderWidget's compositor was stopped instead of
+  // deleting the RenderWidget. So here we can start it again. For the first
+  // main frame, it would already be started by default so this does nothing.
+  // TODO(crbug.com/419087): The RenderWidget should be newly created here along
+  // with the WebFrameWidget, then we won't have to do this.
+  GetWidget()->StartCompositor();
 }
 
 void RenderViewImpl::SetZoomLevel(double zoom_level) {

content/renderer/render_widget.cc

@@ -1548,10 +1548,17 @@ LayerTreeView* RenderWidget::InitializeLayerTreeView() {
   // LayerTreeFrameSink creation.
   bool should_generate_frame_sink =
       !compositor_never_visible_ && RenderThreadImpl::current();
-  if (!should_generate_frame_sink)
+  if (!should_generate_frame_sink) {
+    // Prevents SetVisible() from blink from doing anything.
     layer_tree_view_->SetNeverVisible();
+  } else if (!is_swapped_out_) {
+    // Begins the compositor's scheduler to start producing frames.
+    // Don't do this if the RenderWidget is attached to a RenderViewImpl for a
+    // remote main frame, as this RenderWidget is a zombie then, which won't be
+    // used for compositing until a WebFrameWidget is attached.
+    StartCompositor();
+  }
 
-  StartCompositor();
   DCHECK_NE(MSG_ROUTING_NONE, routing_id_);
   layer_tree_view_->SetFrameSinkId(
       viz::FrameSinkId(RenderThread::Get()->GetClientId(), routing_id_));
@@ -2883,10 +2890,19 @@ cc::ManagedMemoryPolicy RenderWidget::GetGpuMemoryPolicy(
 }
 
 void RenderWidget::StartCompositor() {
-  if (!is_hidden())
+  if (!is_hidden_)
     layer_tree_view_->SetVisible(true);
 }
 
+void RenderWidget::StopCompositor() {
+  layer_tree_view_->SetVisible(false);
+  // Drop all gpu resources, this makes SetVisible(true) more expensive/slower
+  // but we don't expect to use this RenderWidget again until some possible
+  // future navigation. This brings us a bit closer to emulating deleting the
+  // RenderWidget instead of just stopping the compositor.
+  layer_tree_view_->ReleaseLayerTreeFrameSink();
+}
+
 void RenderWidget::HasPointerRawMoveEventHandlers(bool has_handlers) {
   if (input_event_queue_)
     input_event_queue_->HasPointerRawMoveEventHandlers(has_handlers);

content/renderer/render_widget.h

@@ -343,25 +343,20 @@ class CONTENT_EXPORT RenderWidget
   // position.
   ui::TextInputType GetTextInputType();
 
-  // Internal helper that generates the LayerTreeSettings to be given to the
-  // compositor in StartCompositor().
   static cc::LayerTreeSettings GenerateLayerTreeSettings(
       CompositorDependencies* compositor_deps,
       bool is_for_subframe,
       const gfx::Size& initial_screen_size,
       float initial_device_scale_factor);
-  // Internal helper that generates the ManagedMemoryPolicy to be given to the
-  // compositor in StartCompositor().
   static cc::ManagedMemoryPolicy GetGpuMemoryPolicy(
       const cc::ManagedMemoryPolicy& policy,
       const gfx::Size& initial_screen_size,
       float initial_device_scale_factor);
 
-  // Begins the compositor's scheduler to start producing frames.
+  // Initiates the compositor to set up IPC channels and begin its scheduler.
   void StartCompositor();
-
-  // Stop compositing.
-  void WillCloseLayerTreeView();
+  // Pauses the compositor's scheduler and tears down its IPC channels.
+  void StopCompositor();
 
   LayerTreeView* layer_tree_view() const { return layer_tree_view_.get(); }
   WidgetInputHandlerManager* widget_input_handler_manager() {
@@ -571,6 +566,9 @@ class CONTENT_EXPORT RenderWidget
   // It is safe to call this multiple times, which happens in the case of
   // frame widgets beings closed, since subsequent calls are ignored.
   void CloseWebWidget();
+  // Informs the WebWidget that compositor is being destroyed, so it can remove
+  // references to it first.
+  void WillCloseLayerTreeView();
 
 #if BUILDFLAG(USE_EXTERNAL_POPUP_MENU)
   void SetExternalPopupOriginAdjustmentsForEmulation(

third_party/blink/renderer/core/exported/web_view_impl.cc

@@ -332,8 +332,7 @@ WebViewImpl::WebViewImpl(WebViewClient* client,
       should_dispatch_first_layout_after_finished_loading_(false),
       display_mode_(kWebDisplayModeBrowser),
       elastic_overscroll_(FloatSize()),
-      mutator_dispatcher_(nullptr),
-      override_compositor_visibility_(false) {
+      mutator_dispatcher_(nullptr) {
   DCHECK_EQ(!!client_, !!widget_client_);
   Page::PageClients page_clients;
   page_clients.chrome_client = chrome_client_.Get();
@@ -3363,21 +3362,27 @@ void WebViewImpl::SetVisibilityState(
     mojom::PageVisibilityState visibility_state,
     bool is_initial_state) {
   DCHECK(GetPage());
+  const bool visible = visibility_state == mojom::PageVisibilityState::kVisible;
+
   GetPage()->SetVisibilityState(visibility_state, is_initial_state);
 
-  bool visible = visibility_state == mojom::PageVisibilityState::kVisible;
-  if (layer_tree_view_ && !override_compositor_visibility_)
-    layer_tree_view_->SetVisible(visible);
-  GetPage()->GetPageScheduler()->SetPageVisible(visible);
-}
+  // There is no frame yet during creation, but we set visibility on the page.
+  // The creator of the LayerTreeView is responsible for setting up its
+  // visibility.
+  if (GetPage()->MainFrame()) {
+    // The compositor for the main frame should be marked as visible or not only
+    // when the main frame is local. A remote main frame is not composited from
+    // this WebView, it would never be visible even if the Page is.
+    if (GetPage()->MainFrame()->IsLocalFrame()) {
+      // TODO(danakj): We shouldn't be changing visibility after closing, so why
+      // do we need to null check here - only for the DoDeferredClose case which
+      // does close out of order, starting with blink before IPCs are closed.
+      if (layer_tree_view_)
+        layer_tree_view_->SetVisible(visible);
+    }
+  }
 
-void WebViewImpl::SetCompositorVisibility(bool is_visible) {
-  if (!is_visible)
-    override_compositor_visibility_ = true;
-  else
-    override_compositor_visibility_ = false;
-  if (layer_tree_view_)
-    layer_tree_view_->SetVisible(is_visible);
+  GetPage()->GetPageScheduler()->SetPageVisible(visible);
 }
 
 void WebViewImpl::ForceNextWebGLContextCreationToFail() {

third_party/blink/renderer/core/exported/web_view_impl.h

@@ -467,10 +467,6 @@ class CORE_EXPORT WebViewImpl final : public WebView,
                                float bottom_controls_height,
                                bool browser_controls_shrink_layout);
 
-  // Overrides the compositor visibility. See the description of
-  // m_overrideCompositorVisibility for more details.
-  void SetCompositorVisibility(bool);
-
   // TODO(lfg): Remove once WebViewFrameWidget is deleted.
   void ScheduleAnimationForWidget();
 
@@ -667,12 +663,6 @@ class CORE_EXPORT WebViewImpl final : public WebView,
 
   WebPageImportanceSignals page_importance_signals_;
 
-  // TODO(lfg): This is used in order to disable compositor visibility while
-  // the page is still visible. This is needed until the WebView and WebWidget
-  // split is complete, since in out-of-process iframes the page can be
-  // visible, but the WebView should not be used as a widget.
-  bool override_compositor_visibility_;
-
   // We defer commits when transitioning to a new page. ChromeClientImpl calls
   // StopDeferringCommits() to release this when a new page is loaded.
   std::unique_ptr<cc::ScopedDeferCommits> scoped_defer_commits_;

third_party/blink/renderer/core/frame/web_view_frame_widget.cc

@@ -19,13 +19,7 @@ WebViewFrameWidget::WebViewFrameWidget(WebWidgetClient& client,
 WebViewFrameWidget::~WebViewFrameWidget() = default;
 
 void WebViewFrameWidget::Close() {
-  // Note: it's important to use the captured main frame pointer here. During
-  // a frame swap, the swapped frame is detached *after* the frame tree is
-  // updated. If the main frame is being swapped, then
-  // m_webView()->mainFrameImpl() will no longer point to the original frame.
-  web_view_->SetCompositorVisibility(false);
   web_view_ = nullptr;
-
   WebFrameWidgetBase::Close();
 
   // Note: this intentionally does not forward to WebView::close(), to make it
@@ -181,9 +175,7 @@ bool WebViewFrameWidget::ScrollFocusedEditableElementIntoView() {
   return web_view_->ScrollFocusedEditableElementIntoView();
 }
 
-void WebViewFrameWidget::Initialize() {
-  web_view_->SetCompositorVisibility(true);
-}
+void WebViewFrameWidget::Initialize() {}
 
 void WebViewFrameWidget::SetLayerTreeView(WebLayerTreeView*) {
   // The WebViewImpl already has its LayerTreeView, the WebWidgetClient