content: fixes edge case resulting in layer staying hidden

The following sequence leaves the layer associated with
RenderWidgetHostViewAndroid hidden:
1. window hides
2. view is removed from window.
3. view added back to window
4. window is shown.

The bug occurs because StopObservingRootWindow() assumes the
window is shown, so that in step 4 when OnRootWindowVisibilityChanged()
is called the code early outs, leaving the layer hidden.

BUG=1075744
TEST=RenderWidgetHostViewAndroidTest.HideWindowRemoveViewAddViewShowWindow

Change-Id: I7ba61189b0be4882335721cf6fdc5ae2253c5827
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437718Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812870}

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -1477,7 +1477,7 @@ void RenderWidgetHostViewAndroid::HideInternal() {
       << "Hide called when the widget should be shown.";
 
   // Only preserve the frontbuffer if the activity was stopped while the
-  // window is still visible. This avoids visual artificts when transitioning
+  // window is still visible. This avoids visual artifacts when transitioning
   // between activities.
   bool hide_frontbuffer = is_window_activity_started_ || !is_window_visible_;
 
@@ -2095,7 +2095,11 @@ void RenderWidgetHostViewAndroid::OnRootWindowVisibilityChanged(bool visible) {
                "RenderWidgetHostViewAndroid::OnRootWindowVisibilityChanged",
                "visible", visible);
   DCHECK(observing_root_window_);
-  if (is_window_visible_ == visible)
+
+  // Don't early out if visibility hasn't changed and visible. This is necessary
+  // as OnDetachedFromWindow() sets |is_window_visible_| to true, so that this
+  // may be called when ShowInternal() needs to be called.
+  if (is_window_visible_ == visible && !visible)
     return;
 
   is_window_visible_ = visible;

content/browser/renderer_host/render_widget_host_view_android_unittest.cc

@@ -18,6 +18,7 @@
 #include "content/test/test_view_android_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/android/view_android.h"
+#include "ui/android/window_android.h"
 
 namespace content {
 
@@ -42,6 +43,8 @@ class RenderWidgetHostViewAndroidTest : public testing::Test {
   void SetUp() override;
   void TearDown() override;
 
+  ui::ViewAndroid* parent_view() { return &parent_view_; }
+
   std::unique_ptr<TestViewAndroidDelegate> test_view_android_delegate_;
 
  private:
@@ -162,4 +165,35 @@ TEST_F(RenderWidgetHostViewAndroidTest, InsetVisualViewport) {
   EXPECT_TRUE(rwhva->GetLocalSurfaceId().IsNewerThan(inset_surface));
 }
 
+TEST_F(RenderWidgetHostViewAndroidTest, HideWindowRemoveViewAddViewShowWindow) {
+  std::unique_ptr<ui::WindowAndroid> window(
+      ui::WindowAndroid::CreateForTesting());
+  window->AddChild(parent_view());
+  EXPECT_TRUE(render_widget_host_view_android()->IsShowing());
+  // The layer should be visible once attached to a window.
+  EXPECT_FALSE(render_widget_host_view_android()
+                   ->GetNativeView()
+                   ->GetLayer()
+                   ->hide_layer_and_subtree());
+
+  // Hiding the window should and removing the view should hide the layer.
+  window->OnVisibilityChanged(nullptr, nullptr, false);
+  parent_view()->RemoveFromParent();
+  EXPECT_TRUE(render_widget_host_view_android()->IsShowing());
+  EXPECT_TRUE(render_widget_host_view_android()
+                  ->GetNativeView()
+                  ->GetLayer()
+                  ->hide_layer_and_subtree());
+
+  // Adding the view back to a window and notifying the window is visible should
+  // make the layer visible again.
+  window->AddChild(parent_view());
+  window->OnVisibilityChanged(nullptr, nullptr, true);
+  EXPECT_TRUE(render_widget_host_view_android()->IsShowing());
+  EXPECT_FALSE(render_widget_host_view_android()
+                   ->GetNativeView()
+                   ->GetLayer()
+                   ->hide_layer_and_subtree());
+}
+
 }  // namespace content