base: Improve DiscardableSharedMemory support for ashmem.

Locking can now succeed even when the range of pages has been
purged. This avoids unnecessary IPC as it reduces the need to
create new DiscardableSharedMemory instances when backed by ashmem.

BUG=429416
TEST=base_unittests --gtest_filter=DiscardableSharedMemoryTest*

Review URL: https://codereview.chromium.org/871043003

Cr-Commit-Position: refs/heads/master@{#312992}

base/memory/discardable_memory_shmem_allocator.cc

@@ -18,7 +18,11 @@ class DiscardableMemoryShmemChunkImpl : public DiscardableMemoryShmemChunk {
       : shared_memory_(shared_memory.Pass()) {}
 
   // Overridden from DiscardableMemoryShmemChunk:
-  bool Lock() override { return shared_memory_->Lock(0, 0); }
+  bool Lock() override {
+    auto result = shared_memory_->Lock(0, 0);
+    DCHECK_NE(result, DiscardableSharedMemory::PURGED);
+    return result == DiscardableSharedMemory::SUCCESS;
+  }
   void Unlock() override { shared_memory_->Unlock(0, 0); }
   void* Memory() const override { return shared_memory_->memory(); }
   bool IsMemoryResident() const override {

base/memory/discardable_shared_memory.cc

@@ -157,7 +157,8 @@ bool DiscardableSharedMemory::Map(size_t size) {
   return true;
 }
 
-bool DiscardableSharedMemory::Lock(size_t offset, size_t length) {
+DiscardableSharedMemory::LockResult DiscardableSharedMemory::Lock(
+    size_t offset, size_t length) {
   DCHECK_EQ(AlignToPageSize(offset), offset);
   DCHECK_EQ(AlignToPageSize(length), length);
 
@@ -167,7 +168,7 @@ bool DiscardableSharedMemory::Lock(size_t offset, size_t length) {
   // Return false when instance has been purged or not initialized properly by
   // checking if |last_known_usage_| is NULL.
   if (last_known_usage_.is_null())
-    return false;
+    return FAILED;
 
   DCHECK(shared_memory_.memory());
 
@@ -184,7 +185,7 @@ bool DiscardableSharedMemory::Lock(size_t offset, size_t length) {
       // Update |last_known_usage_| in case the above CAS failed because of
       // an incorrect timestamp.
       last_known_usage_ = result.GetTimestamp();
-      return false;
+      return FAILED;
     }
   }
 
@@ -214,11 +215,11 @@ bool DiscardableSharedMemory::Lock(size_t offset, size_t length) {
   DCHECK(SharedMemory::IsHandleValid(handle));
   if (ashmem_pin_region(
           handle.fd, AlignToPageSize(sizeof(SharedState)) + offset, length)) {
-    return false;
+    return PURGED;
   }
 #endif
 
-  return true;
+  return SUCCESS;
 }
 
 void DiscardableSharedMemory::Unlock(size_t offset, size_t length) {

base/memory/discardable_shared_memory.h

@@ -23,6 +23,8 @@ namespace base {
 // access to an instance of this class.
 class BASE_EXPORT DiscardableSharedMemory {
  public:
+  enum LockResult { SUCCESS, PURGED, FAILED };
+
   DiscardableSharedMemory();
 
   // Create a new DiscardableSharedMemory object from an existing, open shared
@@ -47,16 +49,18 @@ class BASE_EXPORT DiscardableSharedMemory {
   SharedMemoryHandle handle() const { return shared_memory_.handle(); }
 
   // Locks a range of memory so that it will not be purged by the system.
-  // Returns true if successful and the memory is still resident. Locking can
-  // fail for three reasons; object might have been purged, our last known usage
-  // timestamp might be out of date or memory might already be locked. Last
-  // know usage time is updated to the actual last usage timestamp if memory
-  // is still resident or 0 if not. The range of memory must be unlocked. The
-  // result of trying to lock an already locked range is undefined.
-  // |offset| and |length| must both be a multiple of the page size as returned
-  // by GetPageSize().
+  // The range of memory must be unlocked. The result of trying to lock an
+  // already locked range is undefined. |offset| and |length| must both be
+  // a multiple of the page size as returned by GetPageSize().
   // Passing 0 for |length| means "everything onward".
-  bool Lock(size_t offset, size_t length);
+  // Returns SUCCESS if range was successfully locked and the memory is still
+  // resident, PURGED if range was successfully locked but has been purged
+  // since last time it was locked and FAILED if range could not be locked.
+  // Locking can fail for two reasons; object might have been purged, our
+  // last known usage timestamp might be out of date. Last known usage time
+  // is updated to the actual last usage timestamp if memory is still resident
+  // or 0 if not.
+  LockResult Lock(size_t offset, size_t length);
 
   // Unlock a previously successfully locked range of memory. The range of
   // memory must be locked. The result of trying to unlock a not

base/memory/discardable_shared_memory_unittest.cc

@@ -64,14 +64,14 @@ TEST(DiscardableSharedMemoryTest, LockAndUnlock) {
   memory1.Unlock(0, 0);
 
   // Lock and unlock memory.
-  rv = memory1.Lock(0, 0);
-  EXPECT_TRUE(rv);
+  auto lock_rv = memory1.Lock(0, 0);
+  EXPECT_EQ(DiscardableSharedMemory::SUCCESS, lock_rv);
   memory1.SetNow(Time::FromDoubleT(2));
   memory1.Unlock(0, 0);
 
   // Lock again before duplicating and passing ownership to new instance.
-  rv = memory1.Lock(0, 0);
-  EXPECT_TRUE(rv);
+  lock_rv = memory1.Lock(0, 0);
+  EXPECT_EQ(DiscardableSharedMemory::SUCCESS, lock_rv);
 
   SharedMemoryHandle shared_handle;
   ASSERT_TRUE(
@@ -87,8 +87,8 @@ TEST(DiscardableSharedMemoryTest, LockAndUnlock) {
   memory2.Unlock(0, 0);
 
   // Lock second instance before passing ownership back to first instance.
-  rv = memory2.Lock(0, 0);
-  EXPECT_TRUE(rv);
+  lock_rv = memory2.Lock(0, 0);
+  EXPECT_EQ(DiscardableSharedMemory::SUCCESS, lock_rv);
 
   // Memory should still be resident.
   rv = memory1.IsMemoryResident();
@@ -135,8 +135,8 @@ TEST(DiscardableSharedMemoryTest, Purge) {
   EXPECT_TRUE(rv);
 
   // Lock should fail as memory has been purged.
-  rv = memory2.Lock(0, 0);
-  EXPECT_FALSE(rv);
+  auto lock_rv = memory2.Lock(0, 0);
+  EXPECT_EQ(DiscardableSharedMemory::FAILED, lock_rv);
 
   ASSERT_FALSE(memory2.IsMemoryResident());
 }
@@ -162,8 +162,8 @@ TEST(DiscardableSharedMemoryTest, LastUsed) {
 
   EXPECT_EQ(memory2.last_known_usage(), Time::FromDoubleT(1));
 
-  rv = memory2.Lock(0, 0);
-  EXPECT_TRUE(rv);
+  auto lock_rv = memory2.Lock(0, 0);
+  EXPECT_EQ(DiscardableSharedMemory::SUCCESS, lock_rv);
 
   // This should fail as memory is locked.
   rv = memory1.Purge(Time::FromDoubleT(2));
@@ -232,10 +232,10 @@ TEST(DiscardableSharedMemoryTest, LockShouldAlwaysFailAfterSuccessfulPurge) {
   EXPECT_TRUE(rv);
 
   // Lock should fail as memory has been purged.
-  rv = memory2.Lock(0, 0);
-  EXPECT_FALSE(rv);
-  rv = memory1.Lock(0, 0);
-  EXPECT_FALSE(rv);
+  auto lock_rv = memory2.Lock(0, 0);
+  EXPECT_EQ(DiscardableSharedMemory::FAILED, lock_rv);
+  lock_rv = memory1.Lock(0, 0);
+  EXPECT_EQ(DiscardableSharedMemory::FAILED, lock_rv);
 }
 
 TEST(DiscardableSharedMemoryTest, LockAndUnlockRange) {

content/child/child_discardable_shared_memory_manager.cc

@@ -72,10 +72,11 @@ ChildDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
 
     // Attempt to lock |free_span|. Delete span and search free list again
     // if locking failed.
-    if (!free_span->shared_memory()->Lock(
+    if (free_span->shared_memory()->Lock(
             free_span->start() * base::GetPageSize() -
                 reinterpret_cast<size_t>(free_span->shared_memory()->memory()),
-            free_span->length() * base::GetPageSize())) {
+            free_span->length() * base::GetPageSize()) !=
+        base::DiscardableSharedMemory::SUCCESS) {
       heap_.DeleteSpan(free_span.Pass());
       continue;
     }
@@ -114,19 +115,34 @@ ChildDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
 bool ChildDiscardableSharedMemoryManager::LockSpan(
     DiscardableSharedMemoryHeap::Span* span) {
   base::AutoLock lock(lock_);
-  return span->shared_memory()->Lock(
-      span->start() * base::GetPageSize() -
-          reinterpret_cast<size_t>(span->shared_memory()->memory()),
-      span->length() * base::GetPageSize());
+
+  size_t offset = span->start() * base::GetPageSize() -
+      reinterpret_cast<size_t>(span->shared_memory()->memory());
+  size_t length = span->length() * base::GetPageSize();
+
+  switch (span->shared_memory()->Lock(offset, length)) {
+    case base::DiscardableSharedMemory::SUCCESS:
+      return true;
+    case base::DiscardableSharedMemory::PURGED:
+      span->shared_memory()->Unlock(offset, length);
+      return false;
+    case base::DiscardableSharedMemory::FAILED:
+      return false;
+  }
+
+  NOTREACHED();
+  return false;
 }
 
 void ChildDiscardableSharedMemoryManager::UnlockSpan(
     DiscardableSharedMemoryHeap::Span* span) {
   base::AutoLock lock(lock_);
-  return span->shared_memory()->Unlock(
-      span->start() * base::GetPageSize() -
-          reinterpret_cast<size_t>(span->shared_memory()->memory()),
-      span->length() * base::GetPageSize());
+
+  size_t offset = span->start() * base::GetPageSize() -
+      reinterpret_cast<size_t>(span->shared_memory()->memory());
+  size_t length = span->length() * base::GetPageSize();
+
+  return span->shared_memory()->Unlock(offset, length);
 }
 
 bool ChildDiscardableSharedMemoryManager::IsSpanResident(

content/common/host_discardable_shared_memory_manager_unittest.cc

@@ -78,7 +78,7 @@ TEST_F(HostDiscardableSharedMemoryManagerTest, AllocateForChild) {
   memory.SetNow(base::Time::FromDoubleT(1));
   memory.Unlock(0, 0);
 
-  ASSERT_TRUE(memory.Lock(0, 0));
+  ASSERT_EQ(base::DiscardableSharedMemory::SUCCESS, memory.Lock(0, 0));
   EXPECT_EQ(memcmp(data, memory.memory(), kDataSize), 0);
   memory.Unlock(0, 0);
 }
@@ -122,10 +122,10 @@ TEST_F(HostDiscardableSharedMemoryManagerTest, Purge) {
   EXPECT_TRUE(memory1.IsMemoryResident());
   EXPECT_TRUE(memory2.IsMemoryResident());
 
-  rv = memory1.Lock(0, 0);
-  EXPECT_TRUE(rv);
-  rv = memory2.Lock(0, 0);
-  EXPECT_TRUE(rv);
+  auto lock_rv = memory1.Lock(0, 0);
+  EXPECT_EQ(base::DiscardableSharedMemory::SUCCESS, lock_rv);
+  lock_rv = memory2.Lock(0, 0);
+  EXPECT_EQ(base::DiscardableSharedMemory::SUCCESS, lock_rv);
 
   memory1.SetNow(base::Time::FromDoubleT(4));
   memory1.Unlock(0, 0);
@@ -141,10 +141,10 @@ TEST_F(HostDiscardableSharedMemoryManagerTest, Purge) {
   EXPECT_FALSE(memory1.IsMemoryResident());
   EXPECT_TRUE(memory2.IsMemoryResident());
 
-  rv = memory1.Lock(0, 0);
-  EXPECT_FALSE(rv);
-  rv = memory2.Lock(0, 0);
-  EXPECT_TRUE(rv);
+  lock_rv = memory1.Lock(0, 0);
+  EXPECT_EQ(base::DiscardableSharedMemory::FAILED, lock_rv);
+  lock_rv = memory2.Lock(0, 0);
+  EXPECT_EQ(base::DiscardableSharedMemory::SUCCESS, lock_rv);
 }
 
 TEST_F(HostDiscardableSharedMemoryManagerTest, EnforceMemoryPolicy) {
@@ -181,7 +181,7 @@ TEST_F(HostDiscardableSharedMemoryManagerTest, EnforceMemoryPolicy) {
   // Memory policy should have successfully been enforced.
   EXPECT_FALSE(manager_->enforce_memory_policy_pending());
 
-  EXPECT_FALSE(memory.Lock(0, 0));
+  EXPECT_EQ(base::DiscardableSharedMemory::FAILED, memory.Lock(0, 0));
 }
 
 }  // namespace