[discardable] Rename methods in ClientDiscardableSharedMemoryManager.

Some naming in ClientDiscardableSharedMemoryManager is inconsistent.
This CL addresses this issue.

Bug: 1157471
Change-Id: I5e76e44979706c6d4def9864686084f75dba4f49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587788Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Thiabaud Engelbrecht <thiabaud@google.com>
Cr-Commit-Position: refs/heads/master@{#837723}

components/discardable_memory/client/client_discardable_shared_memory_manager.cc

@@ -107,7 +107,7 @@ ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
 
 ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
     ~DiscardableMemoryImpl() {
-  base::AutoLock lock(manager_->GetLock());
+  base::AutoLock lock(manager_->lock_);
   if (!span_) {
     DCHECK(!is_locked());
     return;
@@ -119,7 +119,7 @@ ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
 }
 
 bool ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::Lock() {
-  base::AutoLock lock(manager_->GetLock());
+  base::AutoLock lock(manager_->lock_);
   DCHECK(!is_locked());
 
   if (span_ && manager_->LockSpan(span_.get()))
@@ -132,7 +132,7 @@ bool ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::Lock() {
 }
 
 void ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::Unlock() {
-  base::AutoLock lock(manager_->GetLock());
+  base::AutoLock lock(manager_->lock_);
   DCHECK(is_locked());
   DCHECK(span_);
 
@@ -158,7 +158,7 @@ void* ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::data()
     const {
 #if DCHECK_IS_ON()
   {
-    base::AutoLock lock(manager_->GetLock());
+    base::AutoLock lock(manager_->lock_);
     DCHECK(is_locked());
   }
 #endif
@@ -174,7 +174,7 @@ void ClientDiscardableSharedMemoryManager::DiscardableMemoryImpl::
     DiscardForTesting() {
 #if DCHECK_IS_ON()
   {
-    base::AutoLock lock(manager_->GetLock());
+    base::AutoLock lock(manager_->lock_);
     DCHECK(!is_locked());
   }
 #endif
@@ -222,7 +222,7 @@ ClientDiscardableSharedMemoryManager::~ClientDiscardableSharedMemoryManager() {
   // touched after it is destroyed, or it will cause a use-after-free. This
   // check ensures that we stop before that can happen, instead of continuing
   // with dangling pointers.
-  CHECK_EQ(heap_->GetSize(), heap_->GetSizeOfFreeLists());
+  CHECK_EQ(heap_->GetSize(), heap_->GetFreelistSize());
   if (heap_->GetSize())
     MemoryUsageChanged(0, 0);
 
@@ -321,7 +321,7 @@ ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
 
     // Memory usage is guaranteed to have changed after having removed
     // at least one span from the free lists.
-    MemoryUsageChanged(heap_->GetSize(), heap_->GetSizeOfFreeLists());
+    MemoryUsageChanged(heap_->GetSize(), heap_->GetFreelistSize());
 
     auto discardable_memory =
         std::make_unique<DiscardableMemoryImpl>(this, std::move(free_span));
@@ -335,7 +335,7 @@ ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
 
   // Make sure crash keys are up to date in case allocation fails.
   if (heap_->GetSize() != heap_size_prior_to_releasing_purged_memory)
-    MemoryUsageChanged(heap_->GetSize(), heap_->GetSizeOfFreeLists());
+    MemoryUsageChanged(heap_->GetSize(), heap_->GetFreelistSize());
 
   size_t pages_to_allocate =
       std::max(allocation_size / base::GetPageSize(), pages);
@@ -383,7 +383,7 @@ ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
                           false);
   }
 
-  MemoryUsageChanged(heap_->GetSize(), heap_->GetSizeOfFreeLists());
+  MemoryUsageChanged(heap_->GetSize(), heap_->GetFreelistSize());
 
   auto discardable_memory =
       std::make_unique<DiscardableMemoryImpl>(this, std::move(new_span));
@@ -398,7 +398,7 @@ bool ClientDiscardableSharedMemoryManager::OnMemoryDump(
   base::AutoLock lock(lock_);
   if (foregrounded_) {
     const size_t total_size = heap_->GetSize() / 1024;                // in KiB
-    const size_t freelist_size = heap_->GetSizeOfFreeLists() / 1024;  // in KiB
+    const size_t freelist_size = heap_->GetFreelistSize() / 1024;     // in KiB
 
     base::UmaHistogramCounts1M("Memory.Discardable.FreelistSize.Foreground",
                                freelist_size);
@@ -416,7 +416,7 @@ size_t ClientDiscardableSharedMemoryManager::GetBytesAllocated() const {
 }
 
 size_t ClientDiscardableSharedMemoryManager::GetBytesAllocatedLocked() const {
-  return heap_->GetSize() - heap_->GetSizeOfFreeLists();
+  return heap_->GetSize() - heap_->GetFreelistSize();
 }
 
 void ClientDiscardableSharedMemoryManager::BackgroundPurge() {
@@ -464,10 +464,10 @@ void ClientDiscardableSharedMemoryManager::PurgeUnlockedMemory(
       DiscardableMemoryImpl* mem = *prev;
 
       // This assert is only required because the static checker can't figure
-      // out that |mem->manager_->GetLock()| is the same as |this->lock_|, as
+      // out that |mem->manager_->lock_| is the same as |this->lock_|, as
       // verified by the DCHECK.
-      DCHECK_EQ(&lock_, &mem->manager_->GetLock());
-      mem->manager_->GetLock().AssertAcquired();
+      DCHECK_EQ(&lock_, &mem->manager_->lock_);
+      mem->manager_->lock_.AssertAcquired();
 
       auto span = mem->Purge(now - min_age);
       if (span) {
@@ -498,7 +498,7 @@ void ClientDiscardableSharedMemoryManager::ReleaseFreeMemoryImpl() {
   heap_->ReleaseFreeMemory();
 
   if (heap_->GetSize() != heap_size_prior_to_releasing_memory)
-    MemoryUsageChanged(heap_->GetSize(), heap_->GetSizeOfFreeLists());
+    MemoryUsageChanged(heap_->GetSize(), heap_->GetFreelistSize());
 }
 
 bool ClientDiscardableSharedMemoryManager::LockSpan(
@@ -557,7 +557,7 @@ void ClientDiscardableSharedMemoryManager::ReleaseSpan(
   heap_->MergeIntoFreeLists(std::move(span));
 
   // Bytes of free memory changed.
-  MemoryUsageChanged(heap_->GetSize(), heap_->GetSizeOfFreeLists());
+  MemoryUsageChanged(heap_->GetSize(), heap_->GetFreelistSize());
 }
 
 base::trace_event::MemoryAllocatorDump*

components/discardable_memory/client/client_discardable_shared_memory_manager.h

@@ -67,9 +67,9 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
   void ReleaseFreeMemory() override;
 
   bool LockSpan(DiscardableSharedMemoryHeap::Span* span)
-      EXCLUSIVE_LOCKS_REQUIRED(GetLock());
+      EXCLUSIVE_LOCKS_REQUIRED(lock_);
   void UnlockSpan(DiscardableSharedMemoryHeap::Span* span)
-      EXCLUSIVE_LOCKS_REQUIRED(GetLock());
+      EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
   base::trace_event::MemoryAllocatorDump* CreateMemoryAllocatorDump(
       DiscardableSharedMemoryHeap::Span* span,
@@ -136,11 +136,10 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
     // Returns |span_| if it has been unlocked since at least |min_ticks|,
     // otherwise nullptr.
     std::unique_ptr<DiscardableSharedMemoryHeap::Span> Purge(
-        base::TimeTicks min_ticks)
-        EXCLUSIVE_LOCKS_REQUIRED(manager_->GetLock());
+        base::TimeTicks min_ticks) EXCLUSIVE_LOCKS_REQUIRED(manager_->lock_);
 
    private:
-    bool is_locked() const EXCLUSIVE_LOCKS_REQUIRED(manager_->GetLock());
+    bool is_locked() const EXCLUSIVE_LOCKS_REQUIRED(manager_->lock_);
 
     friend class ClientDiscardableSharedMemoryManager;
     // We need to ensure that |manager_| outlives |this|, to avoid a
@@ -149,7 +148,7 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
     std::unique_ptr<DiscardableSharedMemoryHeap::Span> span_;
     // Set to an invalid base::TimeTicks when |this| is Lock()-ed, and to
     // |TimeTicks::Now()| each time |this| is Unlock()-ed.
-    base::TimeTicks last_locked_ GUARDED_BY(manager_->GetLock());
+    base::TimeTicks last_locked_ GUARDED_BY(manager_->lock_);
   };
 
   // Purge any unlocked memory from foreground that hasn't been touched in a
@@ -180,7 +179,6 @@ class DISCARDABLE_MEMORY_EXPORT ClientDiscardableSharedMemoryManager
       EXCLUSIVE_LOCKS_REQUIRED(lock_);
   void ReleaseSpan(std::unique_ptr<DiscardableSharedMemoryHeap::Span> span)
       EXCLUSIVE_LOCKS_REQUIRED(lock_);
-  base::Lock& GetLock() { return lock_; }
   size_t GetBytesAllocatedLocked() const EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;

components/discardable_memory/client/client_discardable_shared_memory_manager_unittest.cc

@@ -60,9 +60,9 @@ class TestClientDiscardableSharedMemoryManager
     return heap_->GetSize();
   }
 
-  size_t GetSizeOfFreeLists() const {
+  size_t GetFreelistSize() const {
     base::AutoLock lock(lock_);
-    return heap_->GetSizeOfFreeLists();
+    return heap_->GetFreelistSize();
   }
 
   bool IsPurgeScheduled() const {
@@ -91,7 +91,7 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest, Simple) {
       base::MakeRefCounted<TestClientDiscardableSharedMemoryManager>();
   // Initially, we should have no memory allocated
   ASSERT_EQ(client->GetBytesAllocated(), 0u);
-  ASSERT_EQ(client->GetSizeOfFreeLists(), 0u);
+  ASSERT_EQ(client->GetFreelistSize(), 0u);
 
   auto mem = client->AllocateLockedDiscardableMemory(page_size_);
 
@@ -123,7 +123,7 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest, MultipleOneByOne) {
   auto client =
       base::MakeRefCounted<TestClientDiscardableSharedMemoryManager>();
   ASSERT_EQ(client->GetBytesAllocated(), 0u);
-  ASSERT_EQ(client->GetSizeOfFreeLists(), 0u);
+  ASSERT_EQ(client->GetFreelistSize(), 0u);
 
   auto mem1 = client->AllocateLockedDiscardableMemory(page_size_ * 2.2);
   auto mem2 = client->AllocateLockedDiscardableMemory(page_size_ * 1.1);
@@ -175,7 +175,7 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest, MultipleAtOnce) {
   auto client =
       base::MakeRefCounted<TestClientDiscardableSharedMemoryManager>();
   ASSERT_EQ(client->GetBytesAllocated(), 0u);
-  ASSERT_EQ(client->GetSizeOfFreeLists(), 0u);
+  ASSERT_EQ(client->GetFreelistSize(), 0u);
 
   auto mem1 = client->AllocateLockedDiscardableMemory(page_size_ * 2.2);
   auto mem2 = client->AllocateLockedDiscardableMemory(page_size_ * 1.1);
@@ -204,38 +204,38 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest, Release) {
   auto client =
       base::MakeRefCounted<TestClientDiscardableSharedMemoryManager>();
   ASSERT_EQ(client->GetBytesAllocated(), 0u);
-  ASSERT_EQ(client->GetSizeOfFreeLists(), 0u);
+  ASSERT_EQ(client->GetFreelistSize(), 0u);
 
   auto mem1 = client->AllocateLockedDiscardableMemory(page_size_ * 3);
   auto mem2 = client->AllocateLockedDiscardableMemory(page_size_ * 2);
 
-  size_t freelist_size = client->GetSizeOfFreeLists();
+  size_t freelist_size = client->GetFreelistSize();
   EXPECT_EQ(client->GetBytesAllocated(), 5 * page_size_);
 
   mem1 = nullptr;
 
   // Less memory is now allocated, but freelists are grown.
   EXPECT_EQ(client->GetBytesAllocated(), page_size_ * 2);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), freelist_size + page_size_ * 3);
+  EXPECT_EQ(client->GetFreelistSize(), freelist_size + page_size_ * 3);
 
   client->BackgroundPurge();
 
   // Purging doesn't remove any memory since none is unlocked, also doesn't
   // remove freelists since we still have some.
   EXPECT_EQ(client->GetBytesAllocated(), page_size_ * 2);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), freelist_size + page_size_ * 3);
+  EXPECT_EQ(client->GetFreelistSize(), freelist_size + page_size_ * 3);
 
   mem2 = nullptr;
 
   // No memory is allocated, but freelists are grown.
   EXPECT_EQ(client->GetBytesAllocated(), 0u);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), freelist_size + page_size_ * 5);
+  EXPECT_EQ(client->GetFreelistSize(), freelist_size + page_size_ * 5);
 
   client->BackgroundPurge();
 
   // Purging now shrinks freelists as well.
   EXPECT_EQ(client->GetBytesAllocated(), 0u);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), 0u);
+  EXPECT_EQ(client->GetFreelistSize(), 0u);
 }
 
 // Similar to previous test, but makes sure that freelist still shrinks when
@@ -244,38 +244,38 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest, ReleaseUnlocked) {
   auto client =
       base::MakeRefCounted<TestClientDiscardableSharedMemoryManager>();
   ASSERT_EQ(client->GetBytesAllocated(), 0u);
-  ASSERT_EQ(client->GetSizeOfFreeLists(), 0u);
+  ASSERT_EQ(client->GetFreelistSize(), 0u);
 
   auto mem1 = client->AllocateLockedDiscardableMemory(page_size_ * 3);
   auto mem2 = client->AllocateLockedDiscardableMemory(page_size_ * 2);
 
-  size_t freelist_size = client->GetSizeOfFreeLists();
+  size_t freelist_size = client->GetFreelistSize();
   EXPECT_EQ(client->GetBytesAllocated(), 5 * page_size_);
 
   mem1 = nullptr;
 
   // Less memory is now allocated, but freelists are grown.
   EXPECT_EQ(client->GetBytesAllocated(), page_size_ * 2);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), freelist_size + page_size_ * 3);
+  EXPECT_EQ(client->GetFreelistSize(), freelist_size + page_size_ * 3);
 
   client->BackgroundPurge();
 
   // Purging doesn't remove any memory since none is unlocked, also doesn't
   // remove freelists since we still have some.
   EXPECT_EQ(client->GetBytesAllocated(), page_size_ * 2);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), freelist_size + page_size_ * 3);
+  EXPECT_EQ(client->GetFreelistSize(), freelist_size + page_size_ * 3);
 
   mem2->Unlock();
 
   // No change in memory usage, since memory was only unlocked not released.
   EXPECT_EQ(client->GetBytesAllocated(), page_size_ * 2);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), freelist_size + page_size_ * 3);
+  EXPECT_EQ(client->GetFreelistSize(), freelist_size + page_size_ * 3);
 
   client->BackgroundPurge();
 
   // Purging now shrinks freelists as well.
   EXPECT_EQ(client->GetBytesAllocated(), 0u);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), 0u);
+  EXPECT_EQ(client->GetFreelistSize(), 0u);
 }
 
 // This tests that memory is actually removed by the periodic purging. We mock a
@@ -287,7 +287,7 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest, ScheduledReleaseUnlocked) {
   auto client =
       base::MakeRefCounted<TestClientDiscardableSharedMemoryManager>();
   ASSERT_EQ(client->GetBytesAllocated(), 0u);
-  ASSERT_EQ(client->GetSizeOfFreeLists(), 0u);
+  ASSERT_EQ(client->GetFreelistSize(), 0u);
 
   auto mem1 = client->AllocateLockedDiscardableMemory(page_size_ * 3);
 
@@ -313,7 +313,7 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest,
   auto client =
       base::MakeRefCounted<TestClientDiscardableSharedMemoryManager>();
   ASSERT_EQ(client->GetBytesAllocated(), 0u);
-  ASSERT_EQ(client->GetSizeOfFreeLists(), 0u);
+  ASSERT_EQ(client->GetFreelistSize(), 0u);
 
   auto mem1 = client->AllocateLockedDiscardableMemory(page_size_ * 3);
   auto mem2 = client->AllocateLockedDiscardableMemory(page_size_ * 2);
@@ -349,7 +349,7 @@ TEST_F(ClientDiscardableSharedMemoryManagerTest,
       ClientDiscardableSharedMemoryManager::kMinAgeForScheduledPurge * 2);
 
   EXPECT_EQ(client->GetBytesAllocated(), 0u);
-  EXPECT_EQ(client->GetSizeOfFreeLists(), 0u);
+  EXPECT_EQ(client->GetFreelistSize(), 0u);
 }
 
 // Tests that the UMA for Lock()-ing successes

components/discardable_memory/common/discardable_shared_memory_heap.cc

@@ -246,7 +246,7 @@ size_t DiscardableSharedMemoryHeap::GetSize() const {
   return num_blocks_ * block_size_;
 }
 
-size_t DiscardableSharedMemoryHeap::GetSizeOfFreeLists() const {
+size_t DiscardableSharedMemoryHeap::GetFreelistSize() const {
   return num_free_blocks_ * block_size_;
 }
 
@@ -258,7 +258,7 @@ bool DiscardableSharedMemoryHeap::OnMemoryDump(
   // for each segment below.
   auto* total_dump = pmd->CreateAllocatorDump(base::StringPrintf(
       "discardable/child_0x%" PRIXPTR, reinterpret_cast<uintptr_t>(this)));
-  const size_t freelist_size = GetSizeOfFreeLists();
+  const size_t freelist_size = GetFreelistSize();
   total_dump->AddScalar("freelist_size",
                         base::trace_event::MemoryAllocatorDump::kUnitsBytes,
                         freelist_size);

components/discardable_memory/common/discardable_shared_memory_heap.h

@@ -91,7 +91,7 @@ class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryHeap {
   size_t GetSize() const;
 
   // Returns bytes of memory currently in the free lists.
-  size_t GetSizeOfFreeLists() const;
+  size_t GetFreelistSize() const;
 
   // Dumps memory statistics for chrome://tracing.
   bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,

components/discardable_memory/common/discardable_shared_memory_heap_unittest.cc

@@ -29,7 +29,7 @@ TEST(DiscardableSharedMemoryHeapTest, Basic) {
   EXPECT_EQ(0u, heap.GetSize());
 
   // Initial size of free lists should be 0.
-  EXPECT_EQ(0u, heap.GetSizeOfFreeLists());
+  EXPECT_EQ(0u, heap.GetFreelistSize());
 
   // Free lists are initially empty.
   EXPECT_FALSE(heap.SearchFreeLists(1, 0));
@@ -51,7 +51,7 @@ TEST(DiscardableSharedMemoryHeapTest, Basic) {
   EXPECT_EQ(memory_size, heap.GetSize());
 
   // Size of free lists should still be 0.
-  EXPECT_EQ(0u, heap.GetSizeOfFreeLists());
+  EXPECT_EQ(0u, heap.GetFreelistSize());
 
   // Free list should still be empty as |new_span| is currently in use.
   EXPECT_FALSE(heap.SearchFreeLists(1, 0));
@@ -60,7 +60,7 @@ TEST(DiscardableSharedMemoryHeapTest, Basic) {
   heap.MergeIntoFreeLists(std::move(new_span));
 
   // Size of free lists should now match |memory_size|.
-  EXPECT_EQ(memory_size, heap.GetSizeOfFreeLists());
+  EXPECT_EQ(memory_size, heap.GetFreelistSize());
 
   // Free lists should not contain a span that is larger than kBlocks.
   EXPECT_FALSE(heap.SearchFreeLists(kBlocks + 1, 0));
@@ -214,7 +214,7 @@ TEST(DiscardableSharedMemoryHeapTest, ReleaseFreeMemory) {
                 next_discardable_shared_memory_id++, base::BindOnce(NullTask));
 
   // Free lists should be empty.
-  EXPECT_EQ(0u, heap.GetSizeOfFreeLists());
+  EXPECT_EQ(0u, heap.GetFreelistSize());
 
   heap.ReleaseFreeMemory();
 
@@ -226,7 +226,7 @@ TEST(DiscardableSharedMemoryHeapTest, ReleaseFreeMemory) {
 
   // Memory should have been released.
   EXPECT_EQ(0u, heap.GetSize());
-  EXPECT_EQ(0u, heap.GetSizeOfFreeLists());
+  EXPECT_EQ(0u, heap.GetFreelistSize());
 }
 
 TEST(DiscardableSharedMemoryHeapTest, ReleasePurgedMemory) {