cros: No frame eviction while applying occlusion state changes

Hold off frame eviction while occlusion state changes are being
applied so that we don't evict a frame and have to load it again
soon because its window is made visible.

Bug: 911213
Change-Id: Ie5787c56d535e54c4c9488299a0095c06716df15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1654150Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668585}

components/viz/client/BUILD.gn

@@ -40,6 +40,7 @@ viz_source_set("unit_tests") {
   testonly = true
   sources = [
     "client_resource_provider_unittest.cc",
+    "frame_eviction_manager_unittest.cc",
     "hit_test_data_provider_draw_quad_unittest.cc",
   ]
 

components/viz/client/frame_eviction_manager.cc

@@ -23,11 +23,19 @@ const int kCriticalPressurePercentage = 10;
 
 }  // namespace
 
+FrameEvictionManager::ScopedPause::ScopedPause() {
+  FrameEvictionManager::GetInstance()->Pause();
+}
+
+FrameEvictionManager::ScopedPause::~ScopedPause() {
+  FrameEvictionManager::GetInstance()->Unpause();
+}
+
 FrameEvictionManager* FrameEvictionManager::GetInstance() {
   return base::Singleton<FrameEvictionManager>::get();
 }
 
-FrameEvictionManager::~FrameEvictionManager() {}
+FrameEvictionManager::~FrameEvictionManager() = default;
 
 void FrameEvictionManager::AddFrame(FrameEvictionManagerClient* frame,
                                     bool locked) {
@@ -109,6 +117,11 @@ FrameEvictionManager::FrameEvictionManager()
 }
 
 void FrameEvictionManager::CullUnlockedFrames(size_t saved_frame_limit) {
+  if (pause_count_) {
+    pending_unlocked_frame_limit_ = saved_frame_limit;
+    return;
+  }
+
   while (!unlocked_frames_.empty() &&
          unlocked_frames_.size() + locked_frames_.size() > saved_frame_limit) {
     size_t old_size = unlocked_frames_.size();
@@ -144,4 +157,18 @@ void FrameEvictionManager::PurgeAllUnlockedFrames() {
   CullUnlockedFrames(0);
 }
 
+void FrameEvictionManager::Pause() {
+  ++pause_count_;
+}
+
+void FrameEvictionManager::Unpause() {
+  --pause_count_;
+  DCHECK_GE(pause_count_, 0);
+
+  if (pause_count_ == 0 && pending_unlocked_frame_limit_) {
+    CullUnlockedFrames(pending_unlocked_frame_limit_.value());
+    pending_unlocked_frame_limit_.reset();
+  }
+}
+
 }  // namespace viz

components/viz/client/frame_eviction_manager.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/memory/memory_pressure_listener.h"
 #include "base/memory/singleton.h"
+#include "base/optional.h"
 #include "components/viz/client/viz_client_export.h"
 
 namespace viz {
@@ -31,6 +32,16 @@ class FrameEvictionManagerClient {
 // tab is visible, or while capturing a screenshot.
 class VIZ_CLIENT_EXPORT FrameEvictionManager {
  public:
+  // Pauses frame eviction within its scope.
+  class VIZ_CLIENT_EXPORT ScopedPause {
+   public:
+    ScopedPause();
+    ~ScopedPause();
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(ScopedPause);
+  };
+
   static FrameEvictionManager* GetInstance();
 
   void AddFrame(FrameEvictionManagerClient*, bool locked);
@@ -54,6 +65,8 @@ class VIZ_CLIENT_EXPORT FrameEvictionManager {
   void PurgeAllUnlockedFrames();
 
  private:
+  friend struct base::DefaultSingletonTraits<FrameEvictionManager>;
+
   FrameEvictionManager();
   ~FrameEvictionManager();
 
@@ -61,7 +74,9 @@ class VIZ_CLIENT_EXPORT FrameEvictionManager {
 
   void PurgeMemory(int percentage);
 
-  friend struct base::DefaultSingletonTraits<FrameEvictionManager>;
+  // Pauses/unpauses frame eviction.
+  void Pause();
+  void Unpause();
 
   // Listens for system under pressure notifications and adjusts number of
   // cached frames accordingly.
@@ -71,6 +86,12 @@ class VIZ_CLIENT_EXPORT FrameEvictionManager {
   std::list<FrameEvictionManagerClient*> unlocked_frames_;
   size_t max_number_of_saved_frames_;
 
+  // Counter of the outstanding pauses.
+  int pause_count_ = 0;
+
+  // Argument of the last CullUnlockedFrames call while paused.
+  base::Optional<size_t> pending_unlocked_frame_limit_;
+
   DISALLOW_COPY_AND_ASSIGN(FrameEvictionManager);
 };
 

components/viz/client/frame_eviction_manager_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/viz/client/frame_eviction_manager.h"
+
+#include <algorithm>
+#include <vector>
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace viz {
+
+namespace {
+
+class TestFrameEvictionManagerClient : public FrameEvictionManagerClient {
+ public:
+  TestFrameEvictionManagerClient() = default;
+  ~TestFrameEvictionManagerClient() override = default;
+
+  // FrameEvictionManagerClient:
+  void EvictCurrentFrame() override {
+    FrameEvictionManager::GetInstance()->RemoveFrame(this);
+    has_frame_ = false;
+  }
+
+  bool has_frame() const { return has_frame_; }
+
+ private:
+  bool has_frame_ = true;
+
+  DISALLOW_COPY_AND_ASSIGN(TestFrameEvictionManagerClient);
+};
+
+}  // namespace
+
+using FrameEvictionManagerTest = testing::Test;
+
+TEST_F(FrameEvictionManagerTest, ScopedPause) {
+  constexpr int kMaxSavedFrames = 1;
+  constexpr int kFrames = 2;
+
+  FrameEvictionManager* manager = FrameEvictionManager::GetInstance();
+  manager->set_max_number_of_saved_frames(kMaxSavedFrames);
+
+  std::vector<TestFrameEvictionManagerClient> frames(kFrames);
+  {
+    FrameEvictionManager::ScopedPause scoped_pause;
+
+    for (auto& frame : frames)
+      manager->AddFrame(&frame, /*locked=*/false);
+
+    // All frames stays because |scoped_pause| holds off frame eviction.
+    EXPECT_EQ(kFrames,
+              std::count_if(frames.begin(), frames.end(),
+                            [](const TestFrameEvictionManagerClient& frame) {
+                              return frame.has_frame();
+                            }));
+  }
+
+  // Frame eviction happens when |scoped_pause| goes out of scope.
+  EXPECT_EQ(kMaxSavedFrames,
+            std::count_if(frames.begin(), frames.end(),
+                          [](const TestFrameEvictionManagerClient& frame) {
+                            return frame.has_frame();
+                          }));
+}
+
+}  // namespace viz

ui/aura/window_occlusion_change_builder.cc

@@ -7,6 +7,7 @@
 #include "base/containers/flat_map.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "components/viz/client/frame_eviction_manager.h"
 #include "third_party/skia/include/core/SkRegion.h"
 #include "ui/aura/window_tracker.h"
 
@@ -18,6 +19,9 @@ class DefaultWindowOcclusionChangeBuilder
  public:
   DefaultWindowOcclusionChangeBuilder() = default;
   ~DefaultWindowOcclusionChangeBuilder() override {
+    // No frame eviction until all occlusion state changes are applied.
+    viz::FrameEvictionManager::ScopedPause scoped_frame_eviction_pause;
+
     while (!windows_.windows().empty()) {
       Window* window = windows_.Pop();
       auto it = changes_.find(window);