RenderWidgetHostViewMac: Fix bug where casted content disappears

Simplify BrowserCompositorMac::TransitionToState, now that we no longer
have a "detached" state. The new version has 3 steps
- detect no-op changes and early-out
- detach from the current compositor, if there is one
- attach to the new compositor, if there is one
This allows the code the common code between the "parent ui layer"
compositor and the "has attached compositor" state.

Update the comments about the states to reflect the simpler reality.

Fix a bug wherein transitioning between HasOwnCompositor and
UseParentLayerCompositor would briefly hide the DelegatedFrameHost
and allow the frame to be inappropriately evicted.

Bug: 897156
Change-Id: If6d772a8d605568c3cdab9be0843919ded5ac5e3
Reviewed-on: https://chromium-review.googlesource.com/c/1317147
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605935}

content/browser/renderer_host/browser_compositor_view_mac.h

@@ -149,26 +149,18 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
 
   // The state of |delegated_frame_host_| and |recyclable_compositor_| to
   // manage being visible, hidden, or drawn via a ui::Layer.
+  // The state of |recyclable_compositor_| and |parent_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,
-    // Effects:
-    // - |recyclable_compositor_| has been recycled and |delegated_frame_host_|
-    //   is hidden and detached from it.
-    // Happens when:
-    // - The |render_widget_host_| hidden or gone, and |cocoa_view_| is not
-    //   attached to an NSWindow.
-    // - This happens for backgrounded tabs.
+    // We are drawing using |recyclable_compositor_|. This happens when the
+    // renderer, but no parent ui::Layer has been specified. This is used by
+    // content shell, popup windows (time/date picker), and when tab capturing
+    // a backgrounded tab.
+    HasOwnCompositor,
+    // There is no compositor. This is true when the renderer is not visible
+    // and no parent ui::Layer is specified.
     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.
+    // We are drawing using |parent_ui_layer_|'s compositor. This happens
+    // whenever |parent_ui_layer_| is non-nullptr.
     UseParentLayerCompositor,
   };
   State state_ = HasNoCompositor;

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -216,7 +216,7 @@ void BrowserCompositorMac::UpdateState() {
 
   // If the host is visible and a compositor is required then create one.
   if (!render_widget_host_is_hidden_) {
-    TransitionToState(HasAttachedCompositor);
+    TransitionToState(HasOwnCompositor);
     return;
   }
 
@@ -225,26 +225,51 @@ void BrowserCompositorMac::UpdateState() {
 }
 
 void BrowserCompositorMac::TransitionToState(State new_state) {
-  // Note that the state enum values represent the other through which
-  // transitions must be done (see comments in State definition).
-
-  // Transition UseParentLayerCompositor -> HasNoCompositor. Note that this
-  // transition will be made if we are already in UseParentLayerCompositor, but
-  // with a different parent layer.
-  if (state_ == UseParentLayerCompositor &&
-      (new_state != UseParentLayerCompositor ||
-       parent_ui_layer_ != root_layer_->parent())) {
+  // Skip if there is no change to make.
+  bool is_no_op = false;
+  if (state_ == new_state) {
+    if (state_ == UseParentLayerCompositor)
+      is_no_op = parent_ui_layer_ == root_layer_->parent();
+    else
+      is_no_op = true;
+  }
+  if (is_no_op)
+    return;
+
+  // First, detach from the current compositor, if there is one.
+  delegated_frame_host_->DetachFromCompositor();
+  if (state_ == UseParentLayerCompositor) {
     DCHECK(root_layer_->parent());
     state_ = HasNoCompositor;
     root_layer_->parent()->RemoveObserver(this);
     root_layer_->parent()->Remove(root_layer_.get());
+  }
+  if (state_ == HasOwnCompositor) {
+    recyclable_compositor_->widget()->ResetNSView();
+    recyclable_compositor_->compositor()->SetRootLayer(nullptr);
+    recyclable_compositor_->InvalidateSurface();
+    ui::RecyclableCompositorMacFactory::Get()->RecycleCompositor(
+        std::move(recyclable_compositor_));
+  }
+
+  // The compositor is now detached. If this is the target state, we're done.
+  state_ = HasNoCompositor;
+  if (new_state == HasNoCompositor) {
+    // Don't transiently hide the DelegatedFrameHost because that can cause the
+    // current frame to be inappropriately evicted.
+    // https://crbug.com/897156
     delegated_frame_host_->WasHidden();
-    delegated_frame_host_->DetachFromCompositor();
+    return;
   }
 
-  // Transition HasNoCompositor -> HasAttachedCompositor.
-  if (state_ == HasNoCompositor && new_state == HasAttachedCompositor) {
-    state_ = HasAttachedCompositor;
+  // Attach to the new compositor.
+  if (new_state == UseParentLayerCompositor) {
+    DCHECK(parent_ui_layer_);
+    parent_ui_layer_->Add(root_layer_.get());
+    parent_ui_layer_->AddObserver(this);
+    state_ = UseParentLayerCompositor;
+  }
+  if (new_state == HasOwnCompositor) {
     recyclable_compositor_ =
         ui::RecyclableCompositorMacFactory::Get()->CreateCompositor(
             content::GetContextFactory(), content::GetContextFactoryPrivate());
@@ -256,40 +281,13 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
         dfh_display_.color_space());
     recyclable_compositor_->widget()->SetNSView(
         accelerated_widget_mac_ns_view_);
-    delegated_frame_host_->AttachToCompositor(
-        recyclable_compositor_->compositor());
-    delegated_frame_host_->WasShown(GetRendererLocalSurfaceId(), dfh_size_dip_,
-                                    false /* record_presentation_time */);
     recyclable_compositor_->Unsuspend();
+    state_ = HasOwnCompositor;
   }
-
-  // Transition HasAttachedCompositor -> HasNoCompositor.
-  if (state_ == HasAttachedCompositor && new_state != HasAttachedCompositor) {
-    state_ = HasNoCompositor;
-    // 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();
-    recyclable_compositor_->widget()->ResetNSView();
-    recyclable_compositor_->compositor()->SetRootLayer(nullptr);
-    recyclable_compositor_->InvalidateSurface();
-    ui::RecyclableCompositorMacFactory::Get()->RecycleCompositor(
-        std::move(recyclable_compositor_));
-  }
-
-  // Transition HasNoCompositor -> UseParentLayerCompositor.
-  if (state_ == HasNoCompositor && new_state == UseParentLayerCompositor) {
-    DCHECK(parent_ui_layer_);
-    DCHECK(parent_ui_layer_->GetCompositor());
-    DCHECK(!root_layer_->parent());
-    state_ = UseParentLayerCompositor;
-    delegated_frame_host_->AttachToCompositor(
-        parent_ui_layer_->GetCompositor());
-    delegated_frame_host_->WasShown(GetRendererLocalSurfaceId(), dfh_size_dip_,
-                                    false /* record_presentation_time */);
-    parent_ui_layer_->Add(root_layer_.get());
-    parent_ui_layer_->AddObserver(this);
-  }
+  DCHECK_EQ(state_, new_state);
+  delegated_frame_host_->AttachToCompositor(GetCompositor());
+  delegated_frame_host_->WasShown(GetRendererLocalSurfaceId(), dfh_size_dip_,
+                                  false /* record_presentation_time */);
 }
 
 // static