Refactor DiscardableMemory for the upcoming DiscardableMemoryAllocator.

DiscardableMemory has the current limitation on Android that it's file-based
thus uses file descriptor thus is a limited resource. This will be worked
around in a future CL by adding DiscardMemoryAllocator operating on large
pieces of discardable memory (thus using less file descriptors) and allowing
the clients to operate on specific chunks.

This CL prepares this addition by refactoring the existing DiscardableMemory
implementation.
It makes it a pure interface to allow the upcoming DiscardableMemoryAllocator
to create instances of its own implementation of DiscardableMemory. This
implementation won't be backed by files, only the allocator will.

This also fixes some issues with the previous interface/implementation:                                                                                                                                                                                                                                                        
- The fact that initialization happened through an Init() method which must be
  called only once. The only benefit of this design is to allow objects to be
  constructed on the stack. In practice all the current clients are allocating
  instances in the heap anyway (although not doing so would be a simple
  change). This also made the interface hard to use correctly but easy to use 
  incorrectly. Implementation also systematically had to check that the object
  was correctly initialized.
- The fact that unlock() did an munmap() while the class was providing a
  Memory() accessor. If the client stored the pointer returned by Memory(), did 
  an Unlock(), then a Lock() (doing an mmap()) and accessed the memory through
  the previously saved pointer, he could have ended up doing uses after free.
  unmap() is not needed as part of memory unpinning AFAICT.
- Some error code paths contained resource (e.g. fd) leaks.

BUG=299828
TBR=tomhudson@chromium.org, darin@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@229838 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 16 insertions, 23 deletions

diff --git a/skia/ext/SkDiscardableMemory_chrome.cc b/skia/ext/SkDiscardableMemory_chrome.cc
index 2e78788..6639a5a 100644
--- a/skia/ext/SkDiscardableMemory_chrome.cc
+++ b/skia/ext/SkDiscardableMemory_chrome.cc
@@ -4,15 +4,10 @@
 
 #include "SkDiscardableMemory_chrome.h"
 
-SkDiscardableMemoryChrome::SkDiscardableMemoryChrome()
-    : discardable_(new base::DiscardableMemory()) {
-}
-
-SkDiscardableMemoryChrome::~SkDiscardableMemoryChrome() {
-}
+SkDiscardableMemoryChrome::~SkDiscardableMemoryChrome() {}
 
 bool SkDiscardableMemoryChrome::lock() {
-  base::LockDiscardableMemoryStatus status = discardable_->Lock();
+  const base::LockDiscardableMemoryStatus status = discardable_->Lock();
   switch (status) {
     case base::DISCARDABLE_MEMORY_SUCCESS:
       return true;
@@ -33,17 +28,17 @@ void SkDiscardableMemoryChrome::unlock() {
   discardable_->Unlock();
 }
 
-bool SkDiscardableMemoryChrome::InitializeAndLock(size_t bytes) {
-  return discardable_->InitializeAndLock(bytes);
+SkDiscardableMemoryChrome::SkDiscardableMemoryChrome(
+    scoped_ptr<base::DiscardableMemory> memory)
+    : discardable_(memory.Pass()) {
 }
 
 SkDiscardableMemory* SkDiscardableMemory::Create(size_t bytes) {
-  if (!base::DiscardableMemory::Supported()) {
+  if (!base::DiscardableMemory::Supported())
     return NULL;
-  }
-  scoped_ptr<SkDiscardableMemoryChrome> discardable(
-      new SkDiscardableMemoryChrome());
-  if (discardable->InitializeAndLock(bytes))
-    return discardable.release();
-  return NULL;
+  scoped_ptr<base::DiscardableMemory> discardable(
+      base::DiscardableMemory::CreateLockedMemory(bytes));
+  if (!discardable)
+    return NULL;
+  return new SkDiscardableMemoryChrome(discardable.Pass());
 }
diff --git a/skia/ext/SkDiscardableMemory_chrome.h b/skia/ext/SkDiscardableMemory_chrome.h
index 13dfef8..b251dc4 100644
--- a/skia/ext/SkDiscardableMemory_chrome.h
+++ b/skia/ext/SkDiscardableMemory_chrome.h
@@ -13,20 +13,18 @@
 // base::DiscardableMemory.
 class SK_API SkDiscardableMemoryChrome : public SkDiscardableMemory {
 public:
-  SkDiscardableMemoryChrome();
   virtual ~SkDiscardableMemoryChrome();
 
-  // Initialize the SkDiscardableMemoryChrome object and lock the memory.
-  // Returns true on success. No memory is allocated if this call returns
-  // false. This call should only be called once.
-  bool InitializeAndLock(size_t bytes);
-
-  // Implementation of SkDiscardableMemory interface.
+  // SkDiscardableMemory:
   virtual bool lock() OVERRIDE;
   virtual void* data() OVERRIDE;
   virtual void unlock() OVERRIDE;
 
 private:
+  friend class SkDiscardableMemory;
+
+  SkDiscardableMemoryChrome(scoped_ptr<base::DiscardableMemory> memory);
+
   scoped_ptr<base::DiscardableMemory> discardable_;
 };