Fix evicted app windows showing as empty in overview

The Files app, as well as other apps, if they're evicted due to
being occluded by other windows, will show as empty for a second or
more in overview until reloaded.

This CL makes them behave the same way as browser windows, that is
they show stale contents until reloaded.

BUG=986085, 866622
TEST=Manually, follow steps on bug, and note the issue no longer shows.

Change-Id: If9a2b26b914fe55672d15d4b560cbe6d7e996591
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1711843Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682390}

content/browser/renderer_host/delegated_frame_host.cc

@@ -81,13 +81,21 @@ DelegatedFrameHost::~DelegatedFrameHost() {
   host_frame_sink_manager_->InvalidateFrameSinkId(frame_sink_id_);
 }
 
+void DelegatedFrameHost::AddObserverForTesting(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void DelegatedFrameHost::RemoveObserverForTesting(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void DelegatedFrameHost::WasShown(
     const viz::LocalSurfaceId& new_local_surface_id,
     const gfx::Size& new_dip_size,
     const base::Optional<RecordTabSwitchTimeRequest>&
         record_tab_switch_time_request) {
   // Cancel any pending frame eviction and unpause it if paused.
-  frame_eviction_state_ = FrameEvictionState::kNotStarted;
+  SetFrameEvictionStateAndNotifyObservers(FrameEvictionState::kNotStarted);
 
   frame_evictor_->SetVisible(true);
   if (record_tab_switch_time_request && compositor_) {
@@ -181,6 +189,16 @@ void DelegatedFrameHost::CopyFromCompositingSurfaceInternal(
       viz::SurfaceId(frame_sink_id_, local_surface_id_), std::move(request));
 }
 
+void DelegatedFrameHost::SetFrameEvictionStateAndNotifyObservers(
+    FrameEvictionState frame_eviction_state) {
+  if (frame_eviction_state_ == frame_eviction_state)
+    return;
+
+  frame_eviction_state_ = frame_eviction_state;
+  for (auto& obs : observers_)
+    obs.OnFrameEvictionStateChanged(frame_eviction_state_);
+}
+
 bool DelegatedFrameHost::CanCopyFromCompositingSurface() const {
   return local_surface_id_.is_valid();
 }
@@ -373,7 +391,8 @@ void DelegatedFrameHost::EvictDelegatedFrame() {
   // CrOS overview mode.
   if (client_->ShouldShowStaleContentOnEviction() &&
       !stale_content_layer_->has_external_content()) {
-    frame_eviction_state_ = FrameEvictionState::kPendingEvictionRequests;
+    SetFrameEvictionStateAndNotifyObservers(
+        FrameEvictionState::kPendingEvictionRequests);
     auto callback =
         base::BindOnce(&DelegatedFrameHost::DidCopyStaleContent, GetWeakPtr());
 
@@ -399,7 +418,7 @@ void DelegatedFrameHost::DidCopyStaleContent(
   DCHECK_EQ(result->format(), viz::CopyOutputResult::Format::RGBA_TEXTURE);
 
   DCHECK_NE(frame_eviction_state_, FrameEvictionState::kNotStarted);
-  frame_eviction_state_ = FrameEvictionState::kNotStarted;
+  SetFrameEvictionStateAndNotifyObservers(FrameEvictionState::kNotStarted);
   ContinueDelegatedFrameEviction();
 
   auto transfer_resource = viz::TransferableResource::MakeGL(

content/browser/renderer_host/delegated_frame_host.h

@@ -69,6 +69,19 @@ class CONTENT_EXPORT DelegatedFrameHost
       public viz::mojom::CompositorFrameSinkClient,
       public viz::HostFrameSinkClient {
  public:
+  enum class FrameEvictionState {
+    kNotStarted = 0,          // Frame eviction is ready.
+    kPendingEvictionRequests  // Frame eviction is paused with pending requests.
+  };
+
+  class Observer {
+   public:
+    virtual void OnFrameEvictionStateChanged(FrameEvictionState new_state) = 0;
+
+   protected:
+    virtual ~Observer() = default;
+  };
+
   // |should_register_frame_sink_id| flag indicates whether DelegatedFrameHost
   // is responsible for registering the associated FrameSinkId with the
   // compositor or not. This is set only on non-aura platforms, since aura is
@@ -78,6 +91,9 @@ class CONTENT_EXPORT DelegatedFrameHost
                      bool should_register_frame_sink_id);
   ~DelegatedFrameHost() override;
 
+  void AddObserverForTesting(Observer* observer);
+  void RemoveObserverForTesting(Observer* observer);
+
   // ui::CompositorObserver implementation.
   void OnCompositingShuttingDown(ui::Compositor* compositor) override;
 
@@ -173,6 +189,14 @@ class CONTENT_EXPORT DelegatedFrameHost
     return weak_factory_.GetWeakPtr();
   }
 
+  const ui::Layer* stale_content_layer() const {
+    return stale_content_layer_.get();
+  }
+
+  FrameEvictionState frame_eviction_state() const {
+    return frame_eviction_state_;
+  }
+
  private:
   friend class DelegatedFrameHostClient;
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraBrowserTest,
@@ -200,6 +224,9 @@ class CONTENT_EXPORT DelegatedFrameHost
       viz::CopyOutputRequest::ResultFormat format,
       viz::CopyOutputRequest::CopyOutputRequestCallback callback);
 
+  void SetFrameEvictionStateAndNotifyObservers(
+      FrameEvictionState frame_eviction_state);
+
   const viz::FrameSinkId frame_sink_id_;
   DelegatedFrameHostClient* const client_;
   const bool enable_viz_;
@@ -231,11 +258,6 @@ class CONTENT_EXPORT DelegatedFrameHost
 
   viz::LocalSurfaceId first_local_surface_id_after_navigation_;
 
-  enum class FrameEvictionState {
-    kNotStarted = 0,          // Frame eviction is ready.
-    kPendingEvictionRequests  // Frame eviction is paused with pending requests.
-  };
-
   FrameEvictionState frame_eviction_state_ = FrameEvictionState::kNotStarted;
 
   // Layer responsible for displaying the stale content for the DFHC when the
@@ -245,6 +267,8 @@ class CONTENT_EXPORT DelegatedFrameHost
 
   TabSwitchTimeRecorder tab_switch_time_recorder_;
 
+  base::ObserverList<Observer>::Unchecked observers_;
+
   base::WeakPtrFactory<DelegatedFrameHost> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(DelegatedFrameHost);

content/browser/renderer_host/render_widget_host_view_aura.h

@@ -459,6 +459,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
 
   class WindowAncestorObserver;
   friend class WindowAncestorObserver;
+  friend void VerifyStaleContentOnFrameEviction(
+      RenderWidgetHostView* render_widget_host_view);
 
   // Allocate a new FrameSinkId if this object is the platform view of a
   // RenderWidgetHostViewGuest. This FrameSinkId will not be actually used in

content/public/test/browser_test_utils.cc

@@ -137,6 +137,7 @@
 #endif
 
 #if defined(USE_AURA)
+#include "content/browser/renderer_host/delegated_frame_host.h"
 #include "content/browser/renderer_host/overscroll_controller.h"
 #include "content/browser/renderer_host/render_widget_host_view_aura.h"
 #include "ui/aura/test/window_event_dispatcher_test_api.h"
@@ -3092,6 +3093,78 @@ MockOverscrollController* MockOverscrollController::Create(
   return raw_mock;
 }
 
+namespace {
+
+// This class interacts with the internals of the DelegatedFrameHost without
+// exposing them in the header.
+class EvictionStateWaiter : public DelegatedFrameHost::Observer {
+ public:
+  EvictionStateWaiter(DelegatedFrameHost* delegated_frame_host)
+      : delegated_frame_host_(delegated_frame_host) {
+    delegated_frame_host_->AddObserverForTesting(this);
+  }
+
+  ~EvictionStateWaiter() override {
+    delegated_frame_host_->RemoveObserverForTesting(this);
+  }
+
+  void WaitForEvictionState(DelegatedFrameHost::FrameEvictionState state) {
+    if (delegated_frame_host_->frame_eviction_state() == state)
+      return;
+
+    waited_eviction_state_ = state;
+    base::RunLoop run_loop;
+    quit_closure_ = run_loop.QuitClosure();
+    run_loop.Run();
+  }
+
+  // DelegatedFrameHost::Observer:
+  void OnFrameEvictionStateChanged(
+      DelegatedFrameHost::FrameEvictionState new_state) override {
+    if (!quit_closure_.is_null() && (new_state == waited_eviction_state_))
+      std::move(quit_closure_).Run();
+  }
+
+ private:
+  DelegatedFrameHost* delegated_frame_host_;
+  DelegatedFrameHost::FrameEvictionState waited_eviction_state_;
+  base::OnceClosure quit_closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(EvictionStateWaiter);
+};
+
+}  // namespace
+
+void VerifyStaleContentOnFrameEviction(
+    RenderWidgetHostView* render_widget_host_view) {
+  auto* render_widget_host_view_aura =
+      static_cast<RenderWidgetHostViewAura*>(render_widget_host_view);
+  DelegatedFrameHost* delegated_frame_host =
+      render_widget_host_view_aura->GetDelegatedFrameHost();
+
+  // Initially there should be no stale content set.
+  EXPECT_FALSE(
+      delegated_frame_host->stale_content_layer()->has_external_content());
+  EXPECT_EQ(delegated_frame_host->frame_eviction_state(),
+            DelegatedFrameHost::FrameEvictionState::kNotStarted);
+
+  // Hide the view and evict the frame, and expect that stale content will be
+  // set.
+  EvictionStateWaiter waiter{delegated_frame_host};
+  render_widget_host_view_aura->WasOccluded();
+  static_cast<viz::FrameEvictorClient*>(delegated_frame_host)
+      ->EvictDelegatedFrame();
+  EXPECT_EQ(delegated_frame_host->frame_eviction_state(),
+            DelegatedFrameHost::FrameEvictionState::kPendingEvictionRequests);
+  // Wait until the stale frame content is copied and set onto the layer, i.e.
+  // the eviction state changes from kPendingEvictionRequests back to
+  // kNotStarted.
+  waiter.WaitForEvictionState(
+      DelegatedFrameHost::FrameEvictionState::kNotStarted);
+  EXPECT_TRUE(
+      delegated_frame_host->stale_content_layer()->has_external_content());
+}
+
 #endif  // defined(USE_AURA)
 
 ContextMenuFilter::ContextMenuFilter()

content/public/test/browser_test_utils.h

@@ -1589,6 +1589,12 @@ class MockOverscrollController {
   // Waits until the mock receives a consumed GestureScrollUpdate.
   virtual void WaitForConsumedScroll() = 0;
 };
+
+// Tests that a |render_widget_host_view| stores a stale content when its frame
+// gets evicted. |render_widget_host_view| has to be a RenderWidgetHostViewAura.
+void VerifyStaleContentOnFrameEviction(
+    RenderWidgetHostView* render_widget_host_view);
+
 #endif  // defined(USE_AURA)
 
 // This class filters for FrameHostMsg_ContextMenu messages coming in

extensions/browser/app_window/app_window.cc

@@ -454,6 +454,10 @@ void AppWindow::ExitPictureInPicture() {
   app_delegate_->ExitPictureInPicture();
 }
 
+bool AppWindow::ShouldShowStaleContentOnEviction(content::WebContents* source) {
+  return true;
+}
+
 bool AppWindow::OnMessageReceived(const IPC::Message& message,
                                   content::RenderFrameHost* render_frame_host) {
   bool handled = true;

extensions/browser/app_window/app_window.h

@@ -451,6 +451,7 @@ class AppWindow : public content::WebContentsDelegate,
       const viz::SurfaceId& surface_id,
       const gfx::Size& natural_size) override;
   void ExitPictureInPicture() override;
+  bool ShouldShowStaleContentOnEviction(content::WebContents* source) override;
 
   // content::WebContentsObserver implementation.
   bool OnMessageReceived(const IPC::Message& message,

extensions/browser/app_window/app_window_browsertest.cc

@@ -66,6 +66,18 @@ IN_PROC_BROWSER_TEST_F(AppWindowBrowserTest, FrameInsetsForNoFrame) {
   CloseAppWindow(app_window);
 }
 
+#if defined(OS_CHROMEOS)
+
+IN_PROC_BROWSER_TEST_F(AppWindowBrowserTest, ShouldShowStaleContentOnEviction) {
+  AppWindow* app_window = CreateTestAppWindow("{}");
+  // Helper function as this test requires inspecting a number of content::
+  // internal objects.
+  content::VerifyStaleContentOnFrameEviction(
+      app_window->web_contents()->GetRenderWidgetHostView());
+}
+
+#endif  // defined(OS_CHROMEOS)
+
 }  // namespace
 
 }  // namespace extensions