Avoid using TaskObserver at MemoryCache

We want to avoid TaskObserver since this binds the current message
loop's default task runner and a class tightly.

This CL makes MemoryCache not be a TaskObserver and actual pruning is
deferred by posting a task. This changes the behavior that the pruning
task is enqueued to the end of the task runner. This should not affect
actual memory usages so much.

This CL also replaces the default task runner usages in the tests with
the test task runner since we also want to avoid the default task runner
in the test as much as possible.

Bug: 870606
Change-Id: I6865ffe2a02800b5106b6da8605e7624498d6316
Reviewed-on: https://chromium-review.googlesource.com/c/1258805Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596476}

third_party/blink/renderer/core/imagebitmap/image_bitmap_test.cc

@@ -35,6 +35,7 @@
 #include "build/build_config.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/core/html/canvas/html_canvas_element.h"
@@ -74,7 +75,8 @@ class ImageBitmapTest : public testing::Test {
     image2_ = surface2->makeImageSnapshot();
 
     // Save the global memory cache to restore it upon teardown.
-    global_memory_cache_ = ReplaceMemoryCacheForTesting(MemoryCache::Create());
+    global_memory_cache_ = ReplaceMemoryCacheForTesting(MemoryCache::Create(
+        blink::scheduler::GetSingleThreadTaskRunnerForTesting()));
 
     auto factory = [](FakeGLES2Interface* gl, bool* gpu_compositing_disabled)
         -> std::unique_ptr<WebGraphicsContext3DProvider> {

third_party/blink/renderer/core/loader/resource/css_style_sheet_resource_test.cc

@@ -8,6 +8,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/platform/platform.h"
+#include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h"
 #include "third_party/blink/public/platform/web_url_response.h"
 #include "third_party/blink/renderer/core/css/css_crossfade_value.h"
 #include "third_party/blink/renderer/core/css/css_image_value.h"
@@ -45,8 +46,8 @@ namespace {
 class CSSStyleSheetResourceTest : public PageTestBase {
  protected:
   CSSStyleSheetResourceTest() {
-    original_memory_cache_ =
-        ReplaceMemoryCacheForTesting(MemoryCache::Create());
+    original_memory_cache_ = ReplaceMemoryCacheForTesting(MemoryCache::Create(
+        blink::scheduler::GetSingleThreadTaskRunnerForTesting()));
   }
 
   ~CSSStyleSheetResourceTest() override {

third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d_test.cc

@@ -231,7 +231,8 @@ void CanvasRenderingContext2DTest::SetUp() {
   StringOrCanvasGradientOrCanvasPattern wrapped_alpha_gradient;
   this->AlphaGradient().SetCanvasGradient(alpha_gradient);
 
-  global_memory_cache_ = ReplaceMemoryCacheForTesting(MemoryCache::Create());
+  global_memory_cache_ = ReplaceMemoryCacheForTesting(MemoryCache::Create(
+      blink::scheduler::GetSingleThreadTaskRunnerForTesting()));
 }
 
 void CanvasRenderingContext2DTest::TearDown() {

third_party/blink/renderer/platform/loader/fetch/memory_cache.cc

@@ -46,8 +46,10 @@ static const float kCTargetPrunePercentage = .95f;
 
 MemoryCache* GetMemoryCache() {
   DCHECK(WTF::IsMainThread());
-  if (!g_memory_cache)
-    g_memory_cache = new Persistent<MemoryCache>(MemoryCache::Create());
+  if (!g_memory_cache) {
+    g_memory_cache = new Persistent<MemoryCache>(MemoryCache::Create(
+        Platform::Current()->MainThread()->GetTaskRunner()));
+  }
   return g_memory_cache->Get();
 }
 
@@ -72,7 +74,8 @@ void MemoryCacheEntry::ClearResourceWeak(Visitor* visitor) {
   resource_.Clear();
 }
 
-inline MemoryCache::MemoryCache()
+inline MemoryCache::MemoryCache(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
     : in_prune_resources_(false),
       prune_pending_(false),
       max_prune_deferral_delay_(kCMaxPruneDeferralDelay),
@@ -81,20 +84,19 @@ inline MemoryCache::MemoryCache()
       last_frame_paint_time_stamp_(0.0),
       capacity_(kCDefaultCacheCapacity),
       delay_before_live_decoded_prune_(kCMinDelayBeforeLiveDecodedPrune),
-      size_(0) {
+      size_(0),
+      task_runner_(std::move(task_runner)) {
   MemoryCacheDumpProvider::Instance()->SetMemoryCache(this);
   if (MemoryCoordinator::IsLowEndDevice())
     MemoryCoordinator::Instance().RegisterClient(this);
 }
 
-MemoryCache* MemoryCache::Create() {
-  return new MemoryCache;
+MemoryCache* MemoryCache::Create(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+  return new MemoryCache(std::move(task_runner));
 }
 
-MemoryCache::~MemoryCache() {
-  if (prune_pending_)
-    Platform::Current()->CurrentThread()->RemoveTaskObserver(this);
-}
+MemoryCache::~MemoryCache() = default;
 
 void MemoryCache::Trace(blink::Visitor* visitor) {
   visitor->Trace(resource_maps_);
@@ -367,43 +369,33 @@ void MemoryCache::Prune() {
   double current_time = WTF::CurrentTime();
   if (prune_pending_) {
     if (current_time - prune_time_stamp_ >= max_prune_deferral_delay_) {
-      PruneNow(current_time, kAutomaticPrune);
+      PruneNow(kAutomaticPrune);
     }
   } else {
     if (current_time - prune_time_stamp_ >= max_prune_deferral_delay_) {
-      PruneNow(current_time, kAutomaticPrune);  // Delay exceeded, prune now.
+      PruneNow(kAutomaticPrune);  // Delay exceeded, prune now.
     } else {
       // Defer.
-      Platform::Current()->CurrentThread()->AddTaskObserver(this);
+      task_runner_->PostTask(
+          FROM_HERE, base::BindOnce(&MemoryCache::PruneNow,
+                                    base::Unretained(this), kAutomaticPrune));
       prune_pending_ = true;
     }
   }
 }
 
-void MemoryCache::WillProcessTask() {}
-
-void MemoryCache::DidProcessTask() {
-  // Perform deferred pruning
-  DCHECK(prune_pending_);
-  PruneNow(WTF::CurrentTime(), kAutomaticPrune);
-}
-
 void MemoryCache::PruneAll() {
-  double current_time = WTF::CurrentTime();
-  PruneNow(current_time, kMaximalPrune);
+  PruneNow(kMaximalPrune);
 }
 
-void MemoryCache::PruneNow(double current_time, PruneStrategy strategy) {
-  if (prune_pending_) {
-    prune_pending_ = false;
-    Platform::Current()->CurrentThread()->RemoveTaskObserver(this);
-  }
+void MemoryCache::PruneNow(PruneStrategy strategy) {
+  prune_pending_ = false;
 
   base::AutoReset<bool> reentrancy_protector(&in_prune_resources_, true);
 
   PruneResources(strategy);
   prune_frame_time_stamp_ = last_frame_paint_time_stamp_;
-  prune_time_stamp_ = current_time;
+  prune_time_stamp_ = WTF::CurrentTime();
 }
 
 void MemoryCache::UpdateFramePaintTimestamp() {

third_party/blink/renderer/platform/loader/fetch/memory_cache.h

@@ -26,7 +26,6 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_LOADER_FETCH_MEMORY_CACHE_H_
 #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_LOADER_FETCH_MEMORY_CACHE_H_
 
-#include "third_party/blink/public/platform/web_thread.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/memory_cache_dump_provider.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource.h"
 #include "third_party/blink/renderer/platform/memory_coordinator.h"
@@ -72,14 +71,14 @@ WILL_NOT_BE_EAGERLY_TRACED_CLASS(MemoryCacheEntry);
 // stylesheets, etc.
 class PLATFORM_EXPORT MemoryCache final
     : public GarbageCollectedFinalized<MemoryCache>,
-      public WebThread::TaskObserver,
       public MemoryCacheDumpClient,
       public MemoryCoordinatorClient {
   USING_GARBAGE_COLLECTED_MIXIN(MemoryCache);
   WTF_MAKE_NONCOPYABLE(MemoryCache);
 
  public:
-  static MemoryCache* Create();
+  static MemoryCache* Create(
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~MemoryCache() override;
   void Trace(blink::Visitor*) override;
 
@@ -156,10 +155,6 @@ class PLATFORM_EXPORT MemoryCache final
   size_t Capacity() const { return capacity_; }
   size_t size() const { return size_; }
 
-  // TaskObserver implementation
-  void WillProcessTask() override;
-  void DidProcessTask() override;
-
   void PruneAll();
 
   void UpdateFramePaintTimestamp();
@@ -186,13 +181,13 @@ class PLATFORM_EXPORT MemoryCache final
   ResourceMap* EnsureResourceMap(const String& cache_identifier);
   ResourceMapIndex resource_maps_;
 
-  MemoryCache();
+  explicit MemoryCache(scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
   void AddInternal(ResourceMap*, MemoryCacheEntry*);
   void RemoveInternal(ResourceMap*, const ResourceMap::iterator&);
 
   void PruneResources(PruneStrategy);
-  void PruneNow(double current_time, PruneStrategy);
+  void PruneNow(PruneStrategy);
 
   bool in_prune_resources_;
   bool prune_pending_;
@@ -208,6 +203,8 @@ class PLATFORM_EXPORT MemoryCache final
   // The number of bytes currently consumed by resources in the cache.
   size_t size_;
 
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
   friend class MemoryCacheTest;
 };
 

third_party/blink/renderer/platform/loader/fetch/memory_cache_correctness_test.cc

@@ -111,7 +111,8 @@ class MemoryCacheCorrectnessTest : public testing::Test {
   // Overrides testing::Test.
   void SetUp() override {
     // Save the global memory cache to restore it upon teardown.
-    global_memory_cache_ = ReplaceMemoryCacheForTesting(MemoryCache::Create());
+    global_memory_cache_ = ReplaceMemoryCacheForTesting(
+        MemoryCache::Create(platform_->test_task_runner()));
 
     MockFetchContext* context =
         MockFetchContext::Create(MockFetchContext::kShouldNotLoadNewResource);

third_party/blink/renderer/platform/loader/fetch/memory_cache_test.cc

@@ -106,7 +106,8 @@ class MemoryCacheTest : public testing::Test {
  protected:
   void SetUp() override {
     // Save the global memory cache to restore it upon teardown.
-    global_memory_cache_ = ReplaceMemoryCacheForTesting(MemoryCache::Create());
+    global_memory_cache_ = ReplaceMemoryCacheForTesting(
+        MemoryCache::Create(platform_->test_task_runner()));
     fetcher_ = ResourceFetcher::Create(
         MockFetchContext::Create(MockFetchContext::kShouldLoadNewResource));
   }
@@ -157,7 +158,7 @@ TEST_F(MemoryCacheTest, MAYBE_VeryLargeResourceAccounting) {
   EXPECT_EQ(cached_resource->size(), GetMemoryCache()->size());
 }
 
-static void RunTask1(Resource* resource1, Resource* resource2) {
+static void RunTask(Resource* resource1, Resource* resource2) {
   // The resource size has to be nonzero for this test to be meaningful, but
   // we do not rely on it having any particular value.
   EXPECT_GT(resource1->size(), 0u);
@@ -173,23 +174,20 @@ static void RunTask1(Resource* resource1, Resource* resource2) {
   EXPECT_GT(resource1->DecodedSize(), 0u);
   EXPECT_GT(resource2->DecodedSize(), 0u);
 
-  // We expect actual pruning doesn't occur here synchronously but deferred
-  // to the end of this task, due to the previous pruning invoked in
-  // testResourcePruningAtEndOfTask().
+  // We expect actual pruning doesn't occur here synchronously but deferred,
+  // due to the previous pruning invoked in TestResourcePruningLater().
   GetMemoryCache()->Prune();
   EXPECT_EQ(total_size, GetMemoryCache()->size());
   EXPECT_GT(resource1->DecodedSize(), 0u);
   EXPECT_GT(resource2->DecodedSize(), 0u);
 }
 
-static void RunTask2(unsigned size_without_decode) {
-  // Next task: now, the resources was pruned.
-  EXPECT_EQ(size_without_decode, GetMemoryCache()->size());
-}
+static void TestResourcePruningLater(ResourceFetcher* fetcher,
+                                     const String& identifier1,
+                                     const String& identifier2) {
+  auto* platform = static_cast<TestingPlatformSupportWithMockScheduler*>(
+      Platform::Current());
 
-static void TestResourcePruningAtEndOfTask(ResourceFetcher* fetcher,
-                                           const String& identifier1,
-                                           const String& identifier2) {
   GetMemoryCache()->SetDelayBeforeLiveDecodedPrune(0);
 
   // Enforce pruning by adding |dummyResource| and then call prune().
@@ -221,46 +219,44 @@ static void TestResourcePruningAtEndOfTask(ResourceFetcher* fetcher,
   resource2->AppendData(kData, 4u);
   resource2->FinishForTest();
 
-  Platform::Current()->CurrentThread()->GetTaskRunner()->PostTask(
-      FROM_HERE, WTF::Bind(&RunTask1, WrapPersistent(resource1),
+  platform->test_task_runner()->PostTask(
+      FROM_HERE, WTF::Bind(&RunTask, WrapPersistent(resource1),
                            WrapPersistent(resource2)));
-  Platform::Current()->CurrentThread()->GetTaskRunner()->PostTask(
-      FROM_HERE,
-      WTF::Bind(&RunTask2,
-                resource1->EncodedSize() + resource1->OverheadSize() +
-                    resource2->EncodedSize() + resource2->OverheadSize()));
-  static_cast<TestingPlatformSupportWithMockScheduler*>(Platform::Current())
-      ->RunUntilIdle();
+  platform->RunUntilIdle();
+
+  // Now, the resources was pruned.
+  unsigned size_without_decode =
+      resource1->EncodedSize() + resource1->OverheadSize() +
+      resource2->EncodedSize() + resource2->OverheadSize();
+  EXPECT_EQ(size_without_decode, GetMemoryCache()->size());
 }
 
-// Verified that when ordering a prune in a runLoop task, the prune
-// is deferred to the end of the task.
+// Verified that when ordering a prune in a runLoop task, the prune is deferred.
 // TODO(crbug.com/850788): Reenable this.
 #if defined(OS_ANDROID)
-#define MAYBE_ResourcePruningAtEndOfTask_Basic \
-  DISABLED_ResourcePruningAtEndOfTask_Basic
+#define MAYBE_ResourcePruningLater_Basic DISABLED_ResourcePruningLater_Basic
 #else
-#define MAYBE_ResourcePruningAtEndOfTask_Basic ResourcePruningAtEndOfTask_Basic
+#define MAYBE_ResourcePruningLater_Basic ResourcePruningLater_Basic
 #endif
-TEST_F(MemoryCacheTest, MAYBE_ResourcePruningAtEndOfTask_Basic) {
-  TestResourcePruningAtEndOfTask(fetcher_, "", "");
+TEST_F(MemoryCacheTest, MAYBE_ResourcePruningLater_Basic) {
+  TestResourcePruningLater(fetcher_, "", "");
 }
 
 // TODO(crbug.com/850788): Reenable this.
 #if defined(OS_ANDROID)
-#define MAYBE_ResourcePruningAtEndOfTask_MultipleResourceMaps \
-  DISABLED_ResourcePruningAtEndOfTask_MultipleResourceMaps
+#define MAYBE_ResourcePruningLater_MultipleResourceMaps \
+  DISABLED_ResourcePruningLater_MultipleResourceMaps
 #else
-#define MAYBE_ResourcePruningAtEndOfTask_MultipleResourceMaps \
-  ResourcePruningAtEndOfTask_MultipleResourceMaps
+#define MAYBE_ResourcePruningLater_MultipleResourceMaps \
+  ResourcePruningLater_MultipleResourceMaps
 #endif
-TEST_F(MemoryCacheTest, MAYBE_ResourcePruningAtEndOfTask_MultipleResourceMaps) {
+TEST_F(MemoryCacheTest, MAYBE_ResourcePruningLater_MultipleResourceMaps) {
   {
-    TestResourcePruningAtEndOfTask(fetcher_, "foo", "");
+    TestResourcePruningLater(fetcher_, "foo", "");
     GetMemoryCache()->EvictResources();
   }
   {
-    TestResourcePruningAtEndOfTask(fetcher_, "foo", "bar");
+    TestResourcePruningLater(fetcher_, "foo", "bar");
     GetMemoryCache()->EvictResources();
   }
 }