Support impl-side painting in Mandoline.

components/html_viewer/discardable_memory_allocator.cc

Index: components/html_viewer/discardable_memory_allocator.cc
diff --git a/components/html_viewer/discardable_memory_allocator.cc b/components/html_viewer/discardable_memory_allocator.cc
index f76deeecc1148d448c4e1566c196471ab869b4d6..512169f29d41c60be0a2d6de8b64f526b1d254e3 100644
--- a/components/html_viewer/discardable_memory_allocator.cc
+++ b/components/html_viewer/discardable_memory_allocator.cc
@@ -5,45 +5,59 @@
 #include "components/html_viewer/discardable_memory_allocator.h"
 
 #include "base/memory/discardable_memory.h"
-#include "base/memory/weak_ptr.h"
 #include "base/stl_util.h"
 
 namespace html_viewer {
 
-// Represents an actual memory chunk. This is an object owned by
-// DiscardableMemoryAllocator. DiscardableMemoryChunkImpl are passed to the
-// rest of the program, and access this memory through a weak ptr.
-class DiscardableMemoryAllocator::OwnedMemoryChunk {
+// Interface to the rest of the program. These objects are owned outside of the
+// allocator.
+class DiscardableMemoryAllocator::DiscardableMemoryChunkImpl
+    : public base::DiscardableMemory {
  public:
-  OwnedMemoryChunk(size_t size, DiscardableMemoryAllocator* allocator)
+  DiscardableMemoryChunkImpl(size_t size, DiscardableMemoryAllocator* allocator)
       : is_locked_(true),
         size_(size),
         data_(new uint8_t[size]),
-        allocator_(allocator),
-        weak_factory_(this) {}
-  ~OwnedMemoryChunk() {}
+        allocator_(allocator) {}
+
+  ~DiscardableMemoryChunkImpl() override {
+    // Either the memory is discarded or the memory chunk is unlocked.
+    DCHECK(data_.get() || !is_locked_);

[danakj, 2015/06/15 17:47:34]

nit: you could DCHECK_IMPLIES(!data_, !is_locked_) might be easier to read?

nit: no .get() needed anyway

[jam, 2015/06/15 22:06:01]

I commented on the change that added DCHECK_IMPLIES :)
Done

+    if (!is_locked_ && data_.get())

[danakj, 2015/06/15 17:47:34]

nit: no get() throughout

[jam, 2015/06/15 22:06:01]

Done.

+      allocator_->NotifyDestructed(unlocked_position_);
+  }
 
-  void Lock() {
+  // Overridden from DiscardableMemoryChunk:
+  bool Lock() override {
     DCHECK(!is_locked_);
+    if (!data_.get())
+      return false;
+
     is_locked_ = true;
     allocator_->NotifyLocked(unlocked_position_);
+    return true;
   }
 
-  void Unlock() {
+  void Unlock() override {
     DCHECK(is_locked_);
+    DCHECK(data_.get());
     is_locked_ = false;
     unlocked_position_ = allocator_->NotifyUnlocked(this);
   }
 
-  bool is_locked() const { return is_locked_; }
-  size_t size() const { return size_; }
-  void* data() const {
-    DCHECK(is_locked_);
-    return data_.get();
+  void* data() const override {
+    if (data_.get()) {
+      DCHECK(is_locked_);
+      return data_.get();
+    }
+    return nullptr;
   }
 
-  base::WeakPtr<OwnedMemoryChunk> GetWeakPtr() {
-    return weak_factory_.GetWeakPtr();
+  size_t size() const { return size_; }
+
+  void Discard() {
+    DCHECK(!is_locked_);
+    data_.reset();
   }
 
  private:
@@ -52,57 +66,11 @@ class DiscardableMemoryAllocator::OwnedMemoryChunk {
   scoped_ptr<uint8_t[]> data_;
   DiscardableMemoryAllocator* allocator_;
 
-  std::list<OwnedMemoryChunk*>::iterator unlocked_position_;
-
-  base::WeakPtrFactory<OwnedMemoryChunk> weak_factory_;
-
-  DISALLOW_IMPLICIT_CONSTRUCTORS(OwnedMemoryChunk);
-};
-
-namespace {
-
-// Interface to the rest of the program. These objects are owned outside of the
-// allocator and wrap a weak ptr.
-class DiscardableMemoryChunkImpl : public base::DiscardableMemory {
- public:
-  explicit DiscardableMemoryChunkImpl(
-      base::WeakPtr<DiscardableMemoryAllocator::OwnedMemoryChunk> chunk)
-      : memory_chunk_(chunk) {}
-  ~DiscardableMemoryChunkImpl() override {
-    // Either the memory chunk is invalid (because the backing has gone away),
-    // or the memory chunk is unlocked (because leaving the chunk locked once
-    // we deallocate means the chunk will never get cleaned up).
-    DCHECK(!memory_chunk_ || !memory_chunk_->is_locked());
-  }
-
-  // Overridden from DiscardableMemoryChunk:
-  bool Lock() override {
-    if (!memory_chunk_)
-      return false;
-
-    memory_chunk_->Lock();
-    return true;
-  }
-
-  void Unlock() override {
-    DCHECK(memory_chunk_);
-    memory_chunk_->Unlock();
-  }
-
-  void* data() const override {
-    if (memory_chunk_)
-      return memory_chunk_->data();
-    return nullptr;
-  }
-
- private:
-  base::WeakPtr<DiscardableMemoryAllocator::OwnedMemoryChunk> memory_chunk_;
+  std::list<DiscardableMemoryChunkImpl*>::iterator unlocked_position_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(DiscardableMemoryChunkImpl);
 };
 
-}  // namespace
-
 DiscardableMemoryAllocator::DiscardableMemoryAllocator(
     size_t desired_max_memory)
     : desired_max_memory_(desired_max_memory),
@@ -117,7 +85,9 @@ DiscardableMemoryAllocator::~DiscardableMemoryAllocator() {
 
 scoped_ptr<base::DiscardableMemory>
 DiscardableMemoryAllocator::AllocateLockedDiscardableMemory(size_t size) {
-  OwnedMemoryChunk* chunk = new OwnedMemoryChunk(size, this);
+  base::AutoLock lock(lock_);
+  DiscardableMemoryChunkImpl* chunk =
+      new DiscardableMemoryChunkImpl(size, this);
   total_live_memory_ += size;
   locked_chunks_++;
 
@@ -128,24 +98,31 @@ DiscardableMemoryAllocator::AllocateLockedDiscardableMemory(size_t size) {
   while (total_live_memory_ > desired_max_memory_ &&
          it != live_unlocked_chunks_.end()) {
     total_live_memory_ -= (*it)->size();
-    delete *it;
+    (*it)->Discard();
     it = live_unlocked_chunks_.erase(it);
   }
 
-  return make_scoped_ptr(new DiscardableMemoryChunkImpl(chunk->GetWeakPtr()));
+  return make_scoped_ptr(chunk);

[sky, 2015/06/15 21:47:46]

nit: move scoped_ptr to 89 and return that.

[jam, 2015/06/15 22:44:54]

Done.

 }
 
-std::list<DiscardableMemoryAllocator::OwnedMemoryChunk*>::iterator
-DiscardableMemoryAllocator::NotifyUnlocked(
-    DiscardableMemoryAllocator::OwnedMemoryChunk* chunk) {
+std::list<DiscardableMemoryAllocator::DiscardableMemoryChunkImpl*>::iterator
+DiscardableMemoryAllocator::NotifyUnlocked(DiscardableMemoryChunkImpl* chunk) {
+  base::AutoLock lock(lock_);
   locked_chunks_--;
   return live_unlocked_chunks_.insert(live_unlocked_chunks_.end(), chunk);
 }
 
 void DiscardableMemoryAllocator::NotifyLocked(
-    std::list<OwnedMemoryChunk*>::iterator it) {
+    std::list<DiscardableMemoryChunkImpl*>::iterator it) {
+  base::AutoLock lock(lock_);
   locked_chunks_++;
   live_unlocked_chunks_.erase(it);
 }
 
+void DiscardableMemoryAllocator::NotifyDestructed(
+    std::list<DiscardableMemoryChunkImpl*>::iterator it) {
+  base::AutoLock lock(lock_);
+  live_unlocked_chunks_.erase(it);
+}
+
 }  // namespace html_viewer