Fix blank page after switching out of simplified view

Sometimes EmbedSurface is called right before WasShown when the tab
is hidden. When frame evictor is notified of this new surface, it
might immediately evict it because it doesn't know the tab will be
visible soon. This will result in an evicted surface getting embedded
which will show blank. Do two things:
- Only call SwappedFrame when the tab is visible so don't have the
problem mention above. Generally there is no point in calling
SwappedFrame when it's invisible because even though a new id
is allocated, there will not be any CompositorFrames submitted to this
surface until the tab is shown, so there is no point in notifying the
frame evictor.
- Notify RHWVAndroid via WasEvicted when the surface is evicted so that
it can allocate a new id for the next time it's shown.

Bug: 896633,893731
Change-Id: I6fbc339f606d70e65ecc79b0f74bcd44133bd4fc
Reviewed-on: https://chromium-review.googlesource.com/c/1289953
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601408}

content/browser/renderer_host/delegated_frame_host.cc

@@ -210,16 +210,9 @@ void DelegatedFrameHost::EmbedSurface(
   const viz::SurfaceId* primary_surface_id =
       client_->DelegatedFrameHostGetLayer()->GetPrimarySurfaceId();
 
-  bool id_changed = local_surface_id_ != new_local_surface_id;
-
   local_surface_id_ = new_local_surface_id;
   surface_dip_size_ = new_dip_size;
 
-  if (id_changed) {
-    frame_evictor_->SwappedFrame(client_->DelegatedFrameHostIsVisible());
-    // Note: the frame may have been evicted immediately.
-  }
-
   viz::SurfaceId new_primary_surface_id(frame_sink_id_, local_surface_id_);
 
   if (!client_->DelegatedFrameHostIsVisible()) {
@@ -237,6 +230,10 @@ void DelegatedFrameHost::EmbedSurface(
     return;
   }
 
+  // Notify |frame_evictor_| that a new surface was embedded.
+  // TODO(samans): Rename this and remove visibility as parameter.
+  frame_evictor_->SwappedFrame(true /* visibility */);
+
   if (!primary_surface_id ||
       primary_surface_id->local_surface_id() != local_surface_id_) {
 #if defined(OS_WIN) || defined(USE_X11)

content/browser/renderer_host/delegated_frame_host_client_android.cc

@@ -42,4 +42,8 @@ void DelegatedFrameHostClientAndroid::OnFrameTokenChanged(
   render_widget_host_view_->OnFrameTokenChangedForView(frame_token);
 }
 
+void DelegatedFrameHostClientAndroid::WasEvicted() {
+  render_widget_host_view_->WasEvicted();
+}
+
 }  // namespace content

content/browser/renderer_host/delegated_frame_host_client_android.h

@@ -31,6 +31,7 @@ class CONTENT_EXPORT DelegatedFrameHostClientAndroid
   void ReclaimResources(
       const std::vector<viz::ReturnedResource>& resources) override;
   void OnFrameTokenChanged(uint32_t frame_token) override;
+  void WasEvicted() override;
 
   RenderWidgetHostViewAndroid* render_widget_host_view_;
 

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -2390,4 +2390,8 @@ RenderWidgetHostViewAndroid::DidUpdateVisualProperties(
   return viz::ScopedSurfaceIdAllocator(std::move(allocation_task));
 }
 
+void RenderWidgetHostViewAndroid::WasEvicted() {
+  local_surface_id_allocator_.GenerateId();
+}
+
 }  // namespace content

content/browser/renderer_host/render_widget_host_view_android.h

@@ -332,6 +332,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
   void OnRenderFrameMetadataChangedBeforeActivation(
       const cc::RenderFrameMetadata& metadata) override;
 
+  void WasEvicted();
+
  protected:
   // RenderWidgetHostViewBase:
   void UpdateBackgroundColor() override;

ui/android/delegated_frame_host_android.cc

@@ -206,6 +206,7 @@ void DelegatedFrameHostAndroid::EvictDelegatedFrame() {
       viz::SurfaceId(frame_sink_id_, local_surface_id_)};
   host_frame_sink_manager_->EvictSurfaces(surface_ids);
   frame_evictor_->DiscardedFrame();
+  client_->WasEvicted();
 }
 
 void DelegatedFrameHostAndroid::ResetFallbackToFirstNavigationSurface() {
@@ -305,19 +306,9 @@ void DelegatedFrameHostAndroid::EmbedSurface(
   if (!enable_surface_synchronization_)
     return;
 
-  bool id_changed = local_surface_id_ != new_local_surface_id;
-
   local_surface_id_ = new_local_surface_id;
   surface_size_in_pixels_ = new_size_in_pixels;
 
-  if (id_changed) {
-    // TODO(fsamuel): "SwappedFrame" is a bad name. Also, this method doesn't
-    // really need to take in visibility. FrameEvictor already has the latest
-    // visibility state.
-    frame_evictor_->SwappedFrame(frame_evictor_->visible());
-    // Note: the frame may have been evicted immediately.
-  }
-
   viz::SurfaceId current_primary_surface_id =
       content_layer_->primary_surface_id();
   viz::SurfaceId new_primary_surface_id(frame_sink_id_, local_surface_id_);
@@ -337,6 +328,12 @@ void DelegatedFrameHostAndroid::EmbedSurface(
     // is visible, EmbedSurface will be called again. See WasShown.
     return;
   }
+
+  // TODO(fsamuel): "SwappedFrame" is a bad name. Also, this method doesn't
+  // really need to take in visibility. FrameEvictor already has the latest
+  // visibility state.
+  frame_evictor_->SwappedFrame(true /* visibility */);
+
   if (!current_primary_surface_id.is_valid() ||
       current_primary_surface_id.local_surface_id() != local_surface_id_) {
     if (base::android::BuildInfo::GetInstance()->sdk_int() <

ui/android/delegated_frame_host_android.h

@@ -52,6 +52,7 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
     virtual void ReclaimResources(
         const std::vector<viz::ReturnedResource>& resources) = 0;
     virtual void OnFrameTokenChanged(uint32_t frame_token) = 0;
+    virtual void WasEvicted() = 0;
   };
 
   DelegatedFrameHostAndroid(ViewAndroid* view,

ui/android/delegated_frame_host_android_unittest.cc

@@ -40,6 +40,7 @@ class MockDelegatedFrameHostAndroidClient
   MOCK_METHOD2(DidPresentCompositorFrame,
                void(uint32_t, const gfx::PresentationFeedback&));
   MOCK_METHOD1(OnFrameTokenChanged, void(uint32_t));
+  MOCK_METHOD0(WasEvicted, void());
 };
 
 class MockWindowAndroidCompositor : public WindowAndroidCompositor {
@@ -303,5 +304,22 @@ TEST_F(DelegatedFrameHostAndroidTest, TestBothCompositorLocks) {
   EXPECT_FALSE(IsLocked());
 }
 
+// Make sure frame evictor is notified of the newly embedded surface after
+// WasShown.
+TEST_F(DelegatedFrameHostAndroidSurfaceSynchronizationTest, EmbedWhileHidden) {
+  {
+    EXPECT_CALL(client_, WasEvicted());
+    frame_host_->EvictDelegatedFrame();
+  }
+  EXPECT_FALSE(frame_host_->HasSavedFrame());
+  viz::LocalSurfaceId id = allocator_.GenerateId();
+  gfx::Size size(100, 100);
+  frame_host_->WasHidden();
+  frame_host_->EmbedSurface(id, size, cc::DeadlinePolicy::UseDefaultDeadline());
+  EXPECT_FALSE(frame_host_->HasSavedFrame());
+  frame_host_->WasShown(id, size);
+  EXPECT_TRUE(frame_host_->HasSavedFrame());
+}
+
 }  // namespace
 }  // namespace ui