Refactor DiscardableMemory for the upcoming DiscardableMemoryAllocator.

base/memory/discardable_memory_android.cc

Index: base/memory/discardable_memory_android.cc
diff --git a/base/memory/discardable_memory_android.cc b/base/memory/discardable_memory_android.cc
index 73a25ae419e897b7bf2f3f903edd931a1599be27..b88a13611b9c7a0488a4fd6a76751201e4b1bdf5 100644
--- a/base/memory/discardable_memory_android.cc
+++ b/base/memory/discardable_memory_android.cc
@@ -7,14 +7,19 @@
 #include <sys/mman.h>
 #include <unistd.h>
 
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/synchronization/lock.h"
 #include "third_party/ashmem/ashmem.h"
 
+namespace base {
 namespace {
 
+// Protects |g_num_discardable_memory| below.
 base::LazyInstance<base::Lock>::Leaky g_discardable_memory_lock =
     LAZY_INSTANCE_INITIALIZER;
 
@@ -25,130 +30,129 @@ int g_num_discardable_memory = 0;
 // descriptor limit.
 const int kDiscardableMemoryNumLimit = 128;
 
-}
-
-namespace base {
-
-// static
-bool DiscardableMemory::Supported() {
-  return true;
-}
-
-DiscardableMemory::~DiscardableMemory() {
-  if (is_locked_)
-    Unlock();
-  // If fd_ is smaller than 0, initialization must have failed and
-  // g_num_discardable_memory is not incremented by the caller.
-  if (fd_ < 0)
-    return;
-  HANDLE_EINTR(close(fd_));
-  fd_ = -1;
-  ReleaseFileDescriptor();
-}
-
-bool DiscardableMemory::ReserveFileDescriptor() {
+bool CreateAshmemRegion(const char* name,
+                        size_t size,
+                        int* out_fd,
+                        void** out_address) {
   base::AutoLock lock(g_discardable_memory_lock.Get());
-  if (g_num_discardable_memory < kDiscardableMemoryNumLimit) {
-    ++g_num_discardable_memory;
-    return true;
-  }
-  return false;
-}
-
-void DiscardableMemory::ReleaseFileDescriptor() {
-  base::AutoLock lock(g_discardable_memory_lock.Get());
-  --g_num_discardable_memory;
-  DCHECK_LE(0, g_num_discardable_memory);
-}
-
-bool DiscardableMemory::InitializeAndLock(size_t size) {
-  // When this function returns true, fd_ should be larger or equal than 0
-  // and g_num_discardable_memory is incremented by 1. Otherwise, fd_
-  // is less than 0 and g_num_discardable_memory is not incremented by
-  // the caller.
-  DCHECK_EQ(fd_, -1);
-  DCHECK(!memory_);
-  if (!ReserveFileDescriptor())
+  if (g_num_discardable_memory + 1 > kDiscardableMemoryNumLimit)
     return false;
-
-  size_ = size;
-  fd_ = ashmem_create_region("", size);
-
-  if (fd_ < 0) {
+  int fd = ashmem_create_region(name, size);
+  if (fd < 0) {
     DLOG(ERROR) << "ashmem_create_region() failed";
-    ReleaseFileDescriptor();
     return false;
   }
+  file_util::ScopedFD fd_closer(&fd);
 
-  int err = ashmem_set_prot_region(fd_, PROT_READ | PROT_WRITE);
+  const int err = ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE);
   if (err < 0) {
     DLOG(ERROR) << "Error " << err << " when setting protection of ashmem";
-    HANDLE_EINTR(close(fd_));
-    fd_ = -1;
-    ReleaseFileDescriptor();
     return false;
   }
 
-  if (!Map()) {
-    // Close the file descriptor in case of any initialization errors.
-    HANDLE_EINTR(close(fd_));
-    fd_ = -1;
-    ReleaseFileDescriptor();
+  // There is a problem using MAP_PRIVATE here. As we are constantly calling
+  // Lock() and Unlock(), data could get lost if they are not written to the
+  // underlying file when Unlock() gets called.
+  void* const address = mmap(
+      NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+  if (address == MAP_FAILED) {
+    DPLOG(ERROR) << "Failed to map memory.";
     return false;
   }
 
-  is_locked_ = true;
+  ignore_result(fd_closer.release());
+  ++g_num_discardable_memory;
+  *out_fd = fd;
+  *out_address = address;
   return true;
 }
 
-LockDiscardableMemoryStatus DiscardableMemory::Lock() {
-  DCHECK_NE(fd_, -1);
-  DCHECK(!is_locked_);
-
-  bool purged = false;
-  if (ashmem_pin_region(fd_, 0, 0) == ASHMEM_WAS_PURGED)
-    purged = true;
-
-  if (!Map())
-    return DISCARDABLE_MEMORY_FAILED;
-
-  is_locked_ = true;
-  return purged ? DISCARDABLE_MEMORY_PURGED : DISCARDABLE_MEMORY_SUCCESS;
+bool DeleteAshmemRegion(int fd, size_t size, void* address) {
+  base::AutoLock lock(g_discardable_memory_lock.Get());
+  --g_num_discardable_memory;
+  if (munmap(address, size) == -1) {
+    DPLOG(ERROR) << "Failed to unmap memory.";
+    close(fd);
+    return false;
+  }
+  return close(fd) == 0;
 }
 
-void DiscardableMemory::Unlock() {
-  DCHECK_GE(fd_, 0);
-  DCHECK(is_locked_);
+LockDiscardableMemoryStatus LockAshmemRegion(int fd,
+                                             size_t off,
+                                             size_t size,
+                                             const void* address) {
+  const int result = ashmem_pin_region(fd, off, size);
+  DCHECK_EQ(0, mprotect(address, size, PROT_READ | PROT_WRITE));
+  return result == ASHMEM_WAS_PURGED ?
+      DISCARDABLE_MEMORY_PURGED : DISCARDABLE_MEMORY_SUCCESS;
+}
 
-  Unmap();
-  if (ashmem_unpin_region(fd_, 0, 0))
+bool UnlockAshmemRegion(int fd, size_t off, size_t size, const void* address) {
+  const int failed = ashmem_unpin_region(fd, off, size);
+  if (failed)
     DLOG(ERROR) << "Failed to unpin memory.";
-  is_locked_ = false;
+  // This allows us to catch accesses to unlocked memory.
+  DCHECK_EQ(0, mprotect(address, size, PROT_NONE));
+  return !failed;
 }
 
-bool DiscardableMemory::Map() {
-  DCHECK(!memory_);
-  // There is a problem using MAP_PRIVATE here. As we are constantly calling
-  // Lock() and Unlock(), data could get lost if they are not written to the
-  // underlying file when Unlock() gets called.
-  memory_ = mmap(NULL, size_, PROT_READ | PROT_WRITE, MAP_SHARED, fd_, 0);
-  if (memory_ == (void*)-1) {
-    DPLOG(ERROR) << "Failed to map memory.";
-    memory_ = NULL;
-    if (ashmem_unpin_region(fd_, 0, 0))
-      DLOG(ERROR) << "Failed to unpin memory.";
-    return false;
+class DiscardableMemoryAndroid : public DiscardableMemory {
+ public:
+  DiscardableMemoryAndroid(int fd, void* address, size_t size)
+      : fd_(fd),
+        memory_(address),
+        size_(size) {
+    DCHECK_GE(fd_, 0);
+    DCHECK(memory_);
   }
-  return true;
-}
 
-void DiscardableMemory::Unmap() {
-  DCHECK(memory_);
+  virtual ~DiscardableMemoryAndroid() {
+    DeleteAshmemRegion(fd_, size_, memory_);
+  }
 
-  if (-1 == munmap(memory_, size_))
-    DPLOG(ERROR) << "Failed to unmap memory.";
+  // DiscardableMemory:
+  virtual LockDiscardableMemoryStatus Lock() OVERRIDE {
+    return LockAshmemRegion(fd_, 0, size_, memory_);
+  }
+
+  virtual void Unlock() OVERRIDE {
+    UnlockAshmemRegion(fd_, 0, size_, memory_);
+  }
+
+  virtual void* Memory() const OVERRIDE {
+    return memory_;
+  }
+
+ private:
+  const int fd_;
+  void* const memory_;
+  const size_t size_;
+
+  DISALLOW_COPY_AND_ASSIGN(DiscardableMemoryAndroid);
+};
 
-  memory_ = NULL;
+}  // namespace
+
+// static
+bool DiscardableMemory::Supported() {
+  return true;
+}
+
+// static
+scoped_ptr<DiscardableMemory> DiscardableMemory::CreateLockedMemory(
+    size_t size) {
+  // Pinning & unpinning works with page granularity therefore align the size
+  // upfront.
+  const size_t kPageSize = 4096;
+  const size_t mask = ~(kPageSize - 1);
+  size = (size + kPageSize - 1) & mask;
+  int fd;
+  void* address;
+  if (!CreateAshmemRegion("", size, &fd, &address))
+    return scoped_ptr<DiscardableMemory>();
+  return scoped_ptr<DiscardableMemory>(
+      new DiscardableMemoryAndroid(fd, address, size));
 }
 
 // static