Fix Blank Screens When Restarting on Android

Recently on Android, we moved to invalidating the LocalSurfaceId upon frame
eviction. This way one can be allocated closer to usage.

This has a side effect when Viz Display Compositor is not active. When not
active, the browser itself has different logic for connecting to its
CompositorFrameSink. Where upon any connection, current frames are evicted. If
this occurs while a RenderWidgetHostViewAndroid is showing, then there is no
subsequent actions which update the LocalSurfaceId.

This leads to us evicting the active frame, without generating a new one.

This change updateds RenderWidgetHostViewAndroid::WasEvicted to only invalidate
when not showing. Which addresses the frame eviction of background tabs. While
returning to generating new ids when showing. Which supports the restarting
case.

Bug: 909903
Change-Id: I5d5da55a8072f71773e2ceb4331fed4fd3402bf0
Reviewed-on: https://chromium-review.googlesource.com/c/1355845
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612894}

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -2409,7 +2409,22 @@ RenderWidgetHostViewAndroid::DidUpdateVisualProperties(
 }
 
 void RenderWidgetHostViewAndroid::WasEvicted() {
-  local_surface_id_allocator_.Invalidate();
+  // Eviction can occur when the CompositorFrameSink has changed. This can
+  // occur either from a lost connection, as well as from the initial conneciton
+  // upon creating RenderWidgetHostViewAndroid. When this occurs while visible
+  // a new LocalSurfaceId should be generated. If eviction occurs while not
+  // visible, then the new LocalSurfaceId can be allocated upon the next Show.
+  if (is_showing_) {
+    local_surface_id_allocator_.GenerateId();
+    // Guarantee that the new LocalSurfaceId is propagated. Rather than relying
+    // upon calls to Show() and OnDidUpdateVisualPropertiesComplete(). As there
+    // is no guarantee that they will occur after the eviction.
+    SynchronizeVisualProperties(
+        cc::DeadlinePolicy::UseExistingDeadline(),
+        local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation());
+  } else {
+    local_surface_id_allocator_.Invalidate();
+  }
 }
 
 }  // namespace content

content/browser/renderer_host/render_widget_host_view_browsertest.cc

@@ -52,9 +52,9 @@ namespace {
     return;  \
   }
 
-// Common base class for browser tests.  This is subclassed twice: Once to test
-// the browser in forced-compositing mode, and once to test with compositing
-// mode disabled.
+// Common base class for browser tests.  This is subclassed three times: Once to
+// test the browser in forced-compositing mode; once to test with compositing
+// mode disabled; once with no surface creation for non-visual tests.
 class RenderWidgetHostViewBrowserTest : public ContentBrowserTest {
  public:
   RenderWidgetHostViewBrowserTest()
@@ -178,6 +178,52 @@ class RenderWidgetHostViewBrowserTestBase : public ContentBrowserTest {
   }
 };
 
+// Base class for testing a RenderWidgetHostViewBase where visual output is not
+// relevant. This class does not setup surfaces for compositing.
+class NoCompositingRenderWidgetHostViewBrowserTest
+    : public RenderWidgetHostViewBrowserTest {
+ public:
+  NoCompositingRenderWidgetHostViewBrowserTest() {}
+  ~NoCompositingRenderWidgetHostViewBrowserTest() override {}
+
+  bool SetUpSourceSurface(const char* wait_message) override {
+    NOTIMPLEMENTED();
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NoCompositingRenderWidgetHostViewBrowserTest);
+};
+
+// When creating the first RenderWidgetHostViewBase, the CompositorFrameSink can
+// change. When this occurs we need to evict the current frame, and recreate
+// surfaces. This tests that when frame eviction occurs while the
+// RenderWidgetHostViewBase is visible, that we generate a new LocalSurfaceId.
+// Simply invalidating can lead to displaying blank screens.
+// (https://crbug.com/909903)
+IN_PROC_BROWSER_TEST_F(NoCompositingRenderWidgetHostViewBrowserTest,
+                       ValidLocalSurfaceIdAllocationAfterInitialNavigation) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  // Creates the initial RenderWidgetHostViewBase, and connects to a
+  // CompositorFrameSink. This will trigger frame eviction.
+  NavigateToURL(shell(),
+                embedded_test_server()->GetURL("/page_with_animation.html"));
+  RenderWidgetHostViewBase* rwhvb = GetRenderWidgetHostView();
+  // Eviction normally invalidates the LocalSurfaceId, however if the
+  // RenderWidgetHostViewBase is visible, a new id must be allocated. Otherwise
+  // blank content is shown.
+  EXPECT_TRUE(rwhvb);
+  // Mac does not initialize RenderWidgetHostViewBase as visible.
+#if !defined(OS_MACOSX)
+  EXPECT_TRUE(rwhvb->IsShowing());
+#endif
+  EXPECT_TRUE(rwhvb->GetLocalSurfaceIdAllocation().IsValid());
+  // TODO(jonross): Unify FrameEvictor into RenderWidgetHostViewBase so that we
+  // can generically test all eviction paths. However this should only be for
+  // top level renderers. Currently the FrameEvict implementations are platform
+  // dependent so we can't have a single generic test.
+}
+
 // Flaky on Android (especially on Marshmallow). crbug.com/896466
 #if defined(OS_ANDROID)
 #define MAYBE_CompositorWorksWhenReusingRenderer \