MacViews: Fix RenderWidgetHostViewMac ui::Layer visibility

Two bugs here.

First bug: Interactions with calling RWHVMac::Show/Hide.

With the non-unified compositor, calling RWHVMac::Show/Hide will cause
the Cocoa NSView to have its show or hide method called.

With the unified compositor, calls to RWHVMac::Show/Hide are largely
ignored. The ui::Layer will always be visible if it has a parent
ui::Layer. This is a bug -- the ui::Layer should be made visible or not
based on the visibility specified by the calls to RWHVMac::Show/Hide.

Second bug: Child NSViews not matching child ui::Layers

It is possible for a WebContentsViewCocoa NSView to have multiple
RenderWidgetHostViewCocoa NSViews. Which of these views is visible
is controlled by called to RWHVMac::Show/Hide.

Track the parent RenderWidgetHostViewBases that added NSViews, and
iterate through all of them when updating the parent ui::Layer.

Unrelated cleanup: Remove parameter from BrowserCompositorViewMac that
is always false (and add a DCHECK that it is false, just for good
measure).

Bug: 859834, 865227
Change-Id: I8767692443d5eb6381a8ea4b087c5c312fef305d
Reviewed-on: https://chromium-review.googlesource.com/1141148Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577334}

content/browser/renderer_host/browser_compositor_view_mac.h

@@ -52,7 +52,6 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
       ui::AcceleratedWidgetMacNSView* accelerated_widget_mac_ns_view,
       BrowserCompositorMacClient* client,
       bool render_widget_host_is_hidden,
-      bool ns_view_attached_to_window,
       const display::Display& initial_display,
       const viz::FrameSinkId& frame_sink_id);
   ~BrowserCompositorMac() override;
@@ -106,6 +105,9 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
   // hidden (e.g, because it is occluded by another window).
   void SetNSViewAttachedToWindow(bool attached);
 
+  // Specify if the ui::Layer should be visible or not.
+  void SetViewVisible(bool visible);
+
   // Sets or clears the parent ui::Layer and updates state to reflect that
   // we are now using the ui::Compositor from |parent_ui_layer| (if non-nullptr)
   // or one from |recyclable_compositor_| (if a compositor is needed).

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -47,7 +47,6 @@ BrowserCompositorMac::BrowserCompositorMac(
     ui::AcceleratedWidgetMacNSView* accelerated_widget_mac_ns_view,
     BrowserCompositorMacClient* client,
     bool render_widget_host_is_hidden,
-    bool ns_view_attached_to_window,
     const display::Display& initial_display,
     const viz::FrameSinkId& frame_sink_id)
     : client_(client),
@@ -66,7 +65,7 @@ BrowserCompositorMac::BrowserCompositorMac(
       true /* should_register_frame_sink_id */));
 
   SetRenderWidgetHostIsHidden(render_widget_host_is_hidden);
-  SetNSViewAttachedToWindow(ns_view_attached_to_window);
+  SetNSViewAttachedToWindow(false);
 }
 
 BrowserCompositorMac::~BrowserCompositorMac() {
@@ -215,6 +214,10 @@ void BrowserCompositorMac::SetNSViewAttachedToWindow(bool attached) {
   UpdateState();
 }
 
+void BrowserCompositorMac::SetViewVisible(bool visible) {
+  root_layer_->SetVisible(visible);
+}
+
 void BrowserCompositorMac::UpdateState() {
   // Always use the parent ui::Layer's ui::Compositor if available.
   if (parent_ui_layer_) {

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -171,9 +171,9 @@ RenderWidgetHostViewMac::RenderWidgetHostViewMac(RenderWidgetHost* widget,
                                        ? AllocateFrameSinkIdForGuestViewHack()
                                        : host()->GetFrameSinkId();
 
-  browser_compositor_.reset(
-      new BrowserCompositorMac(this, this, host()->is_hidden(),
-                               [cocoa_view() window], display_, frame_sink_id));
+  browser_compositor_.reset(new BrowserCompositorMac(
+      this, this, host()->is_hidden(), display_, frame_sink_id));
+  DCHECK(![cocoa_view() window]);
 
   if (!is_guest_view_hack_)
     host()->SetView(this);
@@ -220,7 +220,7 @@ RenderWidgetHostViewMac::~RenderWidgetHostViewMac() {
 }
 
 void RenderWidgetHostViewMac::SetParentUiLayer(ui::Layer* parent_ui_layer) {
-  if (!display_only_using_parent_ui_layer_) {
+  if (parent_ui_layer && !display_only_using_parent_ui_layer_) {
     // The first time that we display using a parent ui::Layer, permanently
     // switch from drawing using Cocoa to only drawing using ui::Views. Erase
     // the existing content being drawn by Cocoa (which may have been set due
@@ -380,6 +380,7 @@ void RenderWidgetHostViewMac::GetScreenInfo(ScreenInfo* screen_info) const {
 void RenderWidgetHostViewMac::Show() {
   is_visible_ = true;
   ns_view_bridge_->SetVisible(is_visible_);
+  browser_compositor_->SetViewVisible(is_visible_);
   browser_compositor_->SetRenderWidgetHostIsHidden(false);
 
   WasUnOccluded();
@@ -388,6 +389,7 @@ void RenderWidgetHostViewMac::Show() {
 void RenderWidgetHostViewMac::Hide() {
   is_visible_ = false;
   ns_view_bridge_->SetVisible(is_visible_);
+  browser_compositor_->SetViewVisible(is_visible_);
   host()->WasHidden();
   browser_compositor_->SetRenderWidgetHostIsHidden(true);
 }

content/browser/web_contents/web_contents_view_mac.h

@@ -7,6 +7,7 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include <list>
 #include <memory>
 #include <string>
 #include <vector>
@@ -167,6 +168,13 @@ class WebContentsViewMac : public WebContentsView,
   // Whether to allow other views.
   bool allow_other_views_;
 
+  // This contains all RenderWidgetHostViewMacs that have been added as child
+  // NSViews to this NSView. Note that this list may contain RWHVMacs besides
+  // just |web_contents_->GetRenderWidgetHostView()|. The only time that the
+  // RWHVMac's NSView is removed from the WCVMac's NSView is when it is
+  // destroyed.
+  std::list<base::WeakPtr<RenderWidgetHostViewBase>> child_views_;
+
   ui::Layer* parent_ui_layer_ = nullptr;
 
   std::unique_ptr<PopupMenuHelper> popup_menu_helper_;

content/browser/web_contents/web_contents_view_mac.mm

@@ -370,8 +370,10 @@ RenderWidgetHostViewBase* WebContentsViewMac::CreateViewForWidget(
     view->SetDelegate(rw_delegate.get());
   }
   view->SetAllowPauseForResizeOrRepaint(!allow_other_views_);
-  if (parent_ui_layer_)
-    view->SetParentUiLayer(parent_ui_layer_);
+
+  // Add the RenderWidgetHostView to the ui::Layer heirarchy.
+  child_views_.push_back(view->GetWeakPtr());
+  SetParentUiLayer(parent_ui_layer_);
 
   // Fancy layout comes later; for now just make it our size and resize it
   // with us. In case there are other siblings of the content area, we want
@@ -448,11 +450,16 @@ void WebContentsViewMac::CloseTab() {
 }
 
 void WebContentsViewMac::SetParentUiLayer(ui::Layer* parent_ui_layer) {
-  parent_ui_layer_ = parent_ui_layer;
-  RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
-      web_contents_->GetRenderWidgetHostView());
-  if (view)
-    view->SetParentUiLayer(parent_ui_layer);
+  // Remove any child NSViews that have been destroyed.
+  for (auto iter = child_views_.begin(); iter != child_views_.end();) {
+    auto iter_next = iter;
+    iter_next++;
+    if (*iter)
+      (*iter)->SetParentUiLayer(parent_ui_layer);
+    else
+      child_views_.erase(iter);
+    iter = iter_next;
+  }
 }
 
 }  // namespace content