BrowserCompositorMac: Remove detached compositor state

With MacViews, it is now very rare to actually need a
RenderWidgetHostViewMac to have its own compositor. The only
circumstances where this happens are
- content shell
- popup windows (the date/time picker)
- tab capture of backgrounded tabs

Remove the concept of having a "detached" compositor, and simplify
the transitions between states. This change will introduce flashes to
paths that rely on the RenderWidgetHostViewMac having its own
compositor (which are now vanishingly rare).

Change-Id: I0feeec67a42f05de05aa4d72fe9af93f033d9d64
Reviewed-on: https://chromium-review.googlesource.com/c/1292377
Commit-Queue: ccameron <ccameron@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601409}

content/browser/renderer_host/browser_compositor_view_mac.h

@@ -159,28 +159,14 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
   cc::DeadlinePolicy GetDeadlinePolicy(bool is_resize) const;
 
   // The state of |delegated_frame_host_| and |recyclable_compositor_| to
-  // manage being visible, occluded, hidden, or drawn via a ui::Layer. Note that
-  // TransitionToState will transition through each intermediate state according
-  // to enum values (e.g, going from HasAttachedCompositor to HasNoCompositor
-  // will temporarily go through HasDetachedCompositor).
+  // manage being visible, hidden, or drawn via a ui::Layer.
   enum State {
     // Effects:
     // - |recyclable_compositor_| exists and is attached to
     //   |delegated_frame_host_|.
     // Happens when:
     // - |render_widet_host_| is in the visible state.
-    HasAttachedCompositor = 0,
-    // Effects:
-    // - |recyclable_compositor_| exists, but |delegated_frame_host_| is
-    //   hidden and detached from it.
-    // Happens when:
-    // - The |render_widget_host_| is hidden, but |cocoa_view_| is still in the
-    //   NSWindow hierarchy (e.g, when the window is occluded or offscreen).
-    // - Note: In this state, |recyclable_compositor_| and its CALayers are kept
-    //   around so that we will have content to show when we are un-occluded. If
-    //   we had a way to keep the CALayers attached to the NSView while
-    //   detaching the ui::Compositor, then there would be no need for this
-    HasDetachedCompositor = 1,
+    HasAttachedCompositor,
     // Effects:
     // - |recyclable_compositor_| has been recycled and |delegated_frame_host_|
     //   is hidden and detached from it.
@@ -188,13 +174,13 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
     // - The |render_widget_host_| hidden or gone, and |cocoa_view_| is not
     //   attached to an NSWindow.
     // - This happens for backgrounded tabs.
-    HasNoCompositor = 2,
+    HasNoCompositor,
     // Effects:
     // - |recyclable_compositor_| does not exist. |delegated_frame_host_| is
     //   attached to |parent_ui_layer_|'s compositor.
     // Happens when:
     // - |parent_ui_layer_| is non-nullptr.
-    UseParentLayerCompositor = 3,
+    UseParentLayerCompositor,
   };
   State state_ = HasNoCompositor;
   void UpdateState();

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -90,8 +90,6 @@ void BrowserCompositorMac::ClearCompositorFrame() {
   // compositor. This ensures that we are able to swap in a new blank frame to
   // replace any old content.
   // https://crbug.com/739621
-  if (recyclable_compositor_)
-    recyclable_compositor_->Unsuspend();
   if (delegated_frame_host_)
     delegated_frame_host_->ClearDelegatedFrame();
 }
@@ -155,8 +153,6 @@ bool BrowserCompositorMac::UpdateNSViewAndDisplay(
   root_layer_->SetBounds(gfx::Rect(dfh_size_dip_));
 
   if (needs_new_surface_id) {
-    if (recyclable_compositor_)
-      recyclable_compositor_->Suspend();
     GetDelegatedFrameHost()->EmbedSurface(
         dfh_local_surface_id_allocator_.GenerateId(), dfh_size_dip_,
         GetDeadlinePolicy(is_resize));
@@ -239,13 +235,6 @@ void BrowserCompositorMac::UpdateState() {
     return;
   }
 
-  // If the host is not visible but we are attached to a window then keep around
-  // a compositor only if it already exists.
-  if (ns_view_attached_to_window_ && state_ == HasAttachedCompositor) {
-    TransitionToState(HasDetachedCompositor);
-    return;
-  }
-
   // Otherwise put the compositor up for recycling.
   TransitionToState(HasNoCompositor);
 }
@@ -258,7 +247,7 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
   // transition will be made if we are already in UseParentLayerCompositor, but
   // with a different parent layer.
   if (state_ == UseParentLayerCompositor &&
-      (new_state < UseParentLayerCompositor ||
+      (new_state != UseParentLayerCompositor ||
        parent_ui_layer_ != root_layer_->parent())) {
     DCHECK(root_layer_->parent());
     root_layer_->parent()->RemoveObserver(this);
@@ -268,8 +257,8 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
     state_ = HasNoCompositor;
   }
 
-  // Transition HasNoCompositor -> HasDetachedCompositor.
-  if (state_ == HasNoCompositor && new_state < HasNoCompositor) {
+  // Transition HasNoCompositor -> HasAttachedCompositor.
+  if (state_ == HasNoCompositor && new_state == HasAttachedCompositor) {
     recyclable_compositor_ =
         ui::RecyclableCompositorMacFactory::Get()->CreateCompositor(
             content::GetContextFactory(), content::GetContextFactoryPrivate());
@@ -281,38 +270,20 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
         dfh_display_.color_space());
     recyclable_compositor_->widget()->SetNSView(
         accelerated_widget_mac_ns_view_);
-    state_ = HasDetachedCompositor;
-  }
-
-  // Transition HasDetachedCompositor -> HasAttachedCompositor.
-  if (state_ == HasDetachedCompositor && new_state < HasDetachedCompositor) {
     delegated_frame_host_->AttachToCompositor(
         recyclable_compositor_->compositor());
     delegated_frame_host_->WasShown(GetRendererLocalSurfaceId(), dfh_size_dip_,
                                     false /* record_presentation_time */);
-    // If there exists a saved frame ready to display, unsuspend the compositor
-    // now (if one is not ready, the compositor will unsuspend on first surface
-    // activation).
-    if (delegated_frame_host_->HasActiveSurface())
-      recyclable_compositor_->Unsuspend();
-
+    recyclable_compositor_->Unsuspend();
     state_ = HasAttachedCompositor;
   }
 
-  // Transition HasAttachedCompositor -> HasDetachedCompositor.
-  if (state_ == HasAttachedCompositor && new_state > HasAttachedCompositor) {
-    // Ensure that any changes made to the ui::Compositor do not result in new
-    // frames being produced.
-    recyclable_compositor_->Suspend();
+  // Transition HasAttachedCompositor -> HasNoCompositor.
+  if (state_ == HasAttachedCompositor && new_state != HasAttachedCompositor) {
     // Marking the DelegatedFrameHost as removed from the window hierarchy is
     // necessary to remove all connections to its old ui::Compositor.
     delegated_frame_host_->WasHidden();
     delegated_frame_host_->DetachFromCompositor();
-    state_ = HasDetachedCompositor;
-  }
-
-  // Transition HasDetachedCompositor -> HasNoCompositor.
-  if (state_ == HasDetachedCompositor && new_state > HasDetachedCompositor) {
     recyclable_compositor_->widget()->ResetNSView();
     recyclable_compositor_->compositor()->SetRootLayer(nullptr);
     recyclable_compositor_->InvalidateSurface();
@@ -322,7 +293,7 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
   }
 
   // Transition HasNoCompositor -> UseParentLayerCompositor.
-  if (state_ == HasNoCompositor && new_state > HasNoCompositor) {
+  if (state_ == HasNoCompositor && new_state == UseParentLayerCompositor) {
     DCHECK(parent_ui_layer_);
     DCHECK(parent_ui_layer_->GetCompositor());
     DCHECK(!root_layer_->parent());
@@ -362,12 +333,6 @@ void BrowserCompositorMac::TakeFallbackContentFrom(
     BrowserCompositorMac* other) {
   delegated_frame_host_->TakeFallbackContentFrom(
       other->delegated_frame_host_.get());
-
-  // We will have a flash if we can't recycle the compositor from |other|.
-  if (other->state_ == HasDetachedCompositor && state_ == HasNoCompositor) {
-    other->TransitionToState(HasNoCompositor);
-    TransitionToState(HasAttachedCompositor);
-  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -387,10 +352,6 @@ SkColor BrowserCompositorMac::DelegatedFrameHostGetGutterColor() const {
 
 void BrowserCompositorMac::OnFirstSurfaceActivation(
     const viz::SurfaceInfo& surface_info) {
-  if (!recyclable_compositor_)
-    return;
-
-  recyclable_compositor_->Unsuspend();
 }
 
 void BrowserCompositorMac::OnBeginFrame(base::TimeTicks frame_time) {

content/browser/renderer_host/render_widget_host_view_mac_unittest.mm

@@ -2014,7 +2014,7 @@ TEST_F(RenderWidgetHostViewMacTest, ClearCompositorFrame) {
   BrowserCompositorMac* browser_compositor = rwhv_mac_->BrowserCompositor();
   ui::Compositor* ui_compositor = browser_compositor->GetCompositor();
   EXPECT_NE(ui_compositor, nullptr);
-  EXPECT_TRUE(ui_compositor->IsLocked());
+  EXPECT_FALSE(ui_compositor->IsLocked());
   rwhv_mac_->ClearCompositorFrame();
   EXPECT_EQ(browser_compositor->GetCompositor(), ui_compositor);
   EXPECT_FALSE(ui_compositor->IsLocked());