[Reland] 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
TEST=Manually: follow steps on bug, and note the issue no longer shows.
TEST=Added new: browser_tests --gtest_filter=AppWindowBrowserTest.ShouldShowStaleContentOnEviction

Change-Id: Ie5ad2f07be3eae8627f16d9ddd3977d22f9085a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894095Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712172}

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_) {
@@ -187,6 +195,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();
 }
@@ -379,7 +397,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());
 
@@ -405,7 +424,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;
 
@@ -180,6 +196,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,
@@ -207,6 +231,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_;
@@ -238,11 +265,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
@@ -252,6 +274,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

@@ -462,6 +462,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

@@ -140,6 +140,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"
@@ -3147,6 +3148,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

@@ -1613,6 +1613,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,31 @@ 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("{}");
+  // Make sure that a surface gets embedded in the frame evictor of the
+  // DelegateFrameHost.
+  app_window->Show(AppWindow::SHOW_ACTIVE);
+  ASSERT_TRUE(app_window->web_contents());
+  content::WaitForResizeComplete(app_window->web_contents());
+
+  // Make sure the renderer submits at least one frame before we test frame
+  // eviction.
+  content::RenderFrameSubmissionObserver submission_observer(
+      app_window->web_contents());
+  if (!submission_observer.render_frame_count())
+    submission_observer.WaitForAnyFrameSubmission();
+
+  // 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