Move purging from OnMemoryState to OnPurgeMemory() in cc/

cc/tiles/gpu_image_decode_cache.cc

 // Copyright 2016 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 "cc/tiles/gpu_image_decode_cache.h"
 
 #include <inttypes.h>
 
+#include "base/auto_reset.h"
 #include "base/debug/alias.h"
 #include "base/memory/discardable_memory_allocator.h"
 #include "base/memory/memory_coordinator_client_registry.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_math.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "cc/debug/devtools_instrumentation.h"
@@ skipping 1242 matching lines @@
   return image_data->decode.is_locked();
 }
 
 bool GpuImageDecodeCache::IsInInUseCacheForTesting(
     const DrawImage& image) const {
   auto found = in_use_cache_.find(GenerateInUseCacheKey(image));
   return found != in_use_cache_.end();
 }
 
 void GpuImageDecodeCache::OnMemoryStateChange(base::MemoryState state) {
-  switch (state) {
-    case base::MemoryState::NORMAL:
-      memory_state_ = state;
-      break;
-    case base::MemoryState::THROTTLED:
-    case base::MemoryState::SUSPENDED: {
-      memory_state_ = state;
+  memory_state_ = state;
+}
 
-      // We've just changed our memory state to a (potentially) more
-      // restrictive one. Re-enforce cache limits.
-      base::AutoLock lock(lock_);
-      EnsureCapacity(0);
-      break;
-    }
-    case base::MemoryState::UNKNOWN:
-      // NOT_REACHED.
-      break;
-  }
+void GpuImageDecodeCache::OnPurgeMemory() {
+  base::AutoLock lock(lock_);
+  // Temporary changes |memory_state_| to free up cache as much as possible.
+  base::AutoReset<base::MemoryState> reset(&memory_state_,
+                                           base::MemoryState::SUSPENDED);
+  EnsureCapacity(0);

[bashi, 2017/02/07 00:58:59]

It seems EnsureCapacity() changes its behavior based on |memory_state_|. Should
we change EnsureCapacity() instead of temporary changing |memory_state_|? Or
should we add a method that clears |perspstent_cache_|?

[ericrk, 2017/02/07 19:20:14]

Does the coordinator call OnMemoryStateChange(SUSPENDED) before or after calling
Purge? If before, it seems like this is unnecessary. If after, it seems possible
to race where we Purge, the cache allocates new memory, then we set to suspended
and that memory is never freed?

[bashi, 2017/02/09 00:46:18]

Thanks. I wasn't aware of the race. Current order is OnPurgeMemory() ->
OnMemoryStateChange(). I'll change the order.

That said our current plan is that OnPurgeMemory() should free up memory
regardless of memory state. We would call OnPurgeMemory() even in NORMAL state
to avoid the OS starting page swapping. Maybe I should modify EnsureCapacity()
to make it state independent but I'm not sure it's a right way. Probably revisit
later?

[ericrk, 2017/02/09 18:36:30]

I'm good re-visitng later - once we get to enabling THROTTLED, we'll need some
tweaks anyway.

 }
 
 }  // namespace cc