Do not hide the aura window of RWHVA

Windows placed in inactive virtual desks will be considered
hidden by the occlusion tracker. This currently end up in
a call to RenderWidgetHostViewAura::Hide() which unnecessarily
hides the aura::Window. This causes issues when reflecting that
window in the desks mini_views.

This CL keeps the aura::Window visible, while updating only
the occlusion state.

BUG=866622
TEST=Added a new test.

Change-Id: I7f71cfa072240264fea2e4934f2c7350f66a74c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1711113Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681459}

content/browser/renderer_host/render_widget_host_view_aura.cc

@@ -334,6 +334,7 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(
       window_(nullptr),
       in_shutdown_(false),
       in_bounds_changed_(false),
+      is_occluded_(true),
       popup_parent_host_view_(nullptr),
       popup_child_host_view_(nullptr),
       is_loading_(false),
@@ -478,12 +479,14 @@ void RenderWidgetHostViewAura::Show() {
                                 window_->GetLocalSurfaceIdAllocation());
   }
 
-  window_->Show();
+  // See documentation of |is_occluded_|.
+  DCHECK(window_->TargetVisibility());
   WasUnOccluded();
 }
 
 void RenderWidgetHostViewAura::Hide() {
-  window_->Hide();
+  // See documentation of |is_occluded_|.
+  DCHECK(window_->TargetVisibility());
   WasOccluded();
 }
 
@@ -627,10 +630,13 @@ void RenderWidgetHostViewAura::EnsureSurfaceSynchronizedForWebTest() {
 }
 
 bool RenderWidgetHostViewAura::IsShowing() {
-  return window_->IsVisible();
+  // See documentation of |is_occluded_|.
+  DCHECK(window_->TargetVisibility());
+  return !is_occluded_;
 }
 
 void RenderWidgetHostViewAura::WasUnOccluded() {
+  is_occluded_ = false;
   if (!host_->is_hidden())
     return;
 
@@ -663,6 +669,7 @@ void RenderWidgetHostViewAura::WasUnOccluded() {
 }
 
 void RenderWidgetHostViewAura::WasOccluded() {
+  is_occluded_ = true;
   if (!host()->is_hidden()) {
     host()->WasHidden();
     if (delegated_frame_host_)
@@ -1692,6 +1699,8 @@ void RenderWidgetHostViewAura::OnWindowDestroyed(aura::Window* window) {
 }
 
 void RenderWidgetHostViewAura::OnWindowTargetVisibilityChanged(bool visible) {
+  // See documentation of |is_occluded_|.
+  DCHECK(visible);
 }
 
 bool RenderWidgetHostViewAura::HasHitTestMask() const {
@@ -2024,6 +2033,10 @@ void RenderWidgetHostViewAura::CreateAuraWindow(aura::client::WindowType type) {
   // Init(), because it needs to have the layer.
   if (frame_sink_id_.is_valid())
     window_->SetEmbedFrameSinkId(frame_sink_id_);
+
+  // The |window_|'s visiblility remains visible the entire time.
+  // See documentation of |is_occluded_|.
+  window_->Show();
 }
 
 void RenderWidgetHostViewAura::CreateDelegatedFrameHostClient() {

content/browser/renderer_host/render_widget_host_view_aura.h

@@ -591,6 +591,14 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
   // True if in the process of handling a window bounds changed notification.
   bool in_bounds_changed_;
 
+  // True if the content of this view is occluded (i.e. not visible).
+  // Note that we keep |window_|'s target visibility true all the time. Its
+  // actual visibility (i.e. Window::IsVisible()) will depend on its location in
+  // the window tree hierarchy and whether its layer is drawn or not (due to
+  // e.g. occlusion). Hiding |window_| is unnecessary and will cause issues when
+  // the browser or app is mirrored for alt-tab, or virtual desks mini_views.
+  bool is_occluded_;
+
   // Our parent host view, if this is a popup.  NULL otherwise.
   RenderWidgetHostViewAura* popup_parent_host_view_;
 

content/browser/renderer_host/render_widget_host_view_aura_browsertest.cc

@@ -72,6 +72,10 @@ class RenderWidgetHostViewAuraBrowserTest : public ContentBrowserTest {
         GetRenderViewHost()->GetWidget()->GetView());
   }
 
+  aura::Window* GetWindow() const {
+    return GetRenderWidgetHostView()->window();
+  }
+
   DelegatedFrameHost* GetDelegatedFrameHost() const {
     return GetRenderWidgetHostView()->delegated_frame_host_.get();
   }
@@ -100,6 +104,7 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
   // Hide the view and evict the frame. This should trigger a copy of the stale
   // frame content.
   GetRenderWidgetHostView()->Hide();
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
   static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost())
       ->EvictDelegatedFrame();
   EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_,
@@ -115,6 +120,7 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
   // Unhidding the view should reset the stale content layer to show the new
   // frame content.
   GetRenderWidgetHostView()->Show();
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
   EXPECT_FALSE(
       GetDelegatedFrameHost()->stale_content_layer_->has_external_content());
 }
@@ -140,12 +146,14 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
   // Hide the view and evict the frame. This should trigger a copy of the stale
   // frame content.
   GetRenderWidgetHostView()->Hide();
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
   static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost())
       ->EvictDelegatedFrame();
   EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_,
             DelegatedFrameHost::FrameEvictionState::kPendingEvictionRequests);
 
   GetRenderWidgetHostView()->Show();
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
   EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_,
             DelegatedFrameHost::FrameEvictionState::kNotStarted);
 
@@ -180,6 +188,7 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
   // Hide the view and evict the frame. This should not trigger a copy of the
   // stale frame content as the WebContentDelegate returns false.
   GetRenderWidgetHostView()->Hide();
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
   static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost())
       ->EvictDelegatedFrame();
 
@@ -192,6 +201,25 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
   EXPECT_FALSE(
       GetDelegatedFrameHost()->stale_content_layer_->has_external_content());
 }
+
+IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
+                       OcclusionDoesntAffectWindowTargetVisibility) {
+  NavigateToURL(shell(), GURL(kMinimalPageDataURL));
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
+  EXPECT_TRUE(GetRenderWidgetHostView()->IsShowing());
+
+  // Hide the window's parent, the content should be occluded but the window's
+  // target visibility should remain visible.
+  GetWindow()->parent()->Hide();
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
+  EXPECT_FALSE(GetRenderWidgetHostView()->IsShowing());
+
+  // Show the parent again, and expect that the contents are unoccluded again.
+  GetWindow()->parent()->Show();
+  EXPECT_TRUE(GetWindow()->TargetVisibility());
+  EXPECT_TRUE(GetRenderWidgetHostView()->IsShowing());
+}
+
 #endif  // #if defined(OS_CHROMEOS)
 
 }  // namespace content