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

This reverts commit 1c6f831f.

Reason for revert: Revert due to FindIt analysis.

Original change's description:
> 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/1290140
> Reviewed-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}

TBR=danakj@chromium.org,dcheng@chromium.org,piman@chromium.org

Change-Id: I2ec1cc5c7bfa723f4fafea7efe01d38ae5019532
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 894899, 419087
Reviewed-on: https://chromium-review.googlesource.com/c/1298330Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602428}

content/renderer/gpu/layer_tree_view.cc

@@ -753,8 +753,4 @@ 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,12 +116,6 @@ 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,12 +1092,6 @@ 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;
 }
@@ -1544,12 +1538,6 @@ 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,17 +1548,10 @@ LayerTreeView* RenderWidget::InitializeLayerTreeView() {
   // LayerTreeFrameSink creation.
   bool should_generate_frame_sink =
       !compositor_never_visible_ && RenderThreadImpl::current();
-  if (!should_generate_frame_sink) {
-    // Prevents SetVisible() from blink from doing anything.
+  if (!should_generate_frame_sink)
     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_));
@@ -2890,19 +2883,10 @@ 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,20 +343,25 @@ 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);
 
-  // Initiates the compositor to set up IPC channels and begin its scheduler.
+  // Begins the compositor's scheduler to start producing frames.
   void StartCompositor();
-  // Pauses the compositor's scheduler and tears down its IPC channels.
-  void StopCompositor();
+
+  // Stop compositing.
+  void WillCloseLayerTreeView();
 
   LayerTreeView* layer_tree_view() const { return layer_tree_view_.get(); }
   WidgetInputHandlerManager* widget_input_handler_manager() {
@@ -566,9 +571,6 @@ 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,7 +332,8 @@ WebViewImpl::WebViewImpl(WebViewClient* client,
       should_dispatch_first_layout_after_finished_loading_(false),
       display_mode_(kWebDisplayModeBrowser),
       elastic_overscroll_(FloatSize()),
-      mutator_dispatcher_(nullptr) {
+      mutator_dispatcher_(nullptr),
+      override_compositor_visibility_(false) {
   DCHECK_EQ(!!client_, !!widget_client_);
   Page::PageClients page_clients;
   page_clients.chrome_client = chrome_client_.Get();
@@ -3362,29 +3363,23 @@ 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);
 
-  // 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);
-    }
-  }
-
+  bool visible = visibility_state == mojom::PageVisibilityState::kVisible;
+  if (layer_tree_view_ && !override_compositor_visibility_)
+    layer_tree_view_->SetVisible(visible);
   GetPage()->GetPageScheduler()->SetPageVisible(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);
+}
+
 void WebViewImpl::ForceNextWebGLContextCreationToFail() {
   CoreInitializer::GetInstance().ForceNextWebGLContextCreationToFail();
 }

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

@@ -467,6 +467,10 @@ 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();
 
@@ -663,6 +667,12 @@ 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,7 +19,13 @@ 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
@@ -175,7 +181,9 @@ bool WebViewFrameWidget::ScrollFocusedEditableElementIntoView() {
   return web_view_->ScrollFocusedEditableElementIntoView();
 }
 
-void WebViewFrameWidget::Initialize() {}
+void WebViewFrameWidget::Initialize() {
+  web_view_->SetCompositorVisibility(true);
+}
 
 void WebViewFrameWidget::SetLayerTreeView(WebLayerTreeView*) {
   // The WebViewImpl already has its LayerTreeView, the WebWidgetClient