Make content::BufferQueue use scoped ptrs for AllocatedSurfaces

content/browser/compositor/buffer_queue.cc

Index: content/browser/compositor/buffer_queue.cc
diff --git a/content/browser/compositor/buffer_queue.cc b/content/browser/compositor/buffer_queue.cc
index c82741654b59cf4a259091a154d6e7324c7a0b49..a72fbcb3ec792bbefb3ef845aec30fa854068680 100644
--- a/content/browser/compositor/buffer_queue.cc
+++ b/content/browser/compositor/buffer_queue.cc
@@ -51,10 +51,12 @@ void BufferQueue::BindFramebuffer() {
   gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
   gl->BindFramebuffer(GL_FRAMEBUFFER, fbo_);
 
-  if (!current_surface_.texture) {
+  if (!current_surface_)
     current_surface_ = GetNextSurface();
+
+  if (current_surface_) {
     gl->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
-                             texture_target_, current_surface_.texture, 0);
+                             texture_target_, current_surface_->texture, 0);
   }
 }
 
@@ -69,33 +71,43 @@ void BufferQueue::CopyBufferDamage(int texture,
 }
 
 void BufferQueue::UpdateBufferDamage(const gfx::Rect& damage) {
-  displayed_surface_.damage.Union(damage);
-  for (size_t i = 0; i < available_surfaces_.size(); i++)
-    available_surfaces_[i].damage.Union(damage);
-
-  for (std::deque<AllocatedSurface>::iterator it =
-           in_flight_surfaces_.begin();
-       it != in_flight_surfaces_.end();
-       ++it)
-    it->damage.Union(damage);
+  if (displayed_surface_)
+    displayed_surface_->damage.Union(damage);
+  for (auto& surface : available_surfaces_)
+    surface->damage.Union(damage);
+  for (auto& surface : in_flight_surfaces_) {
+    if (surface)
+      surface->damage.Union(damage);
+  }
 }
 
 void BufferQueue::SwapBuffers(const gfx::Rect& damage) {
-  if (!damage.IsEmpty() && damage != gfx::Rect(size_)) {
-    // We must have a frame available to copy from.
-    DCHECK(!in_flight_surfaces_.empty() || displayed_surface_.texture);
-    unsigned int texture_id = !in_flight_surfaces_.empty()
-                                  ? in_flight_surfaces_.back().texture
-                                  : displayed_surface_.texture;
-
-    CopyBufferDamage(current_surface_.texture, texture_id, damage,
-                     current_surface_.damage);
+  if (current_surface_) {
+    if (!damage.IsEmpty() && damage != gfx::Rect(size_)) {
+      // Copy damage from the most recently swapped buffer. In the event that
+      // the buffer was destroyed and failed to recreate, pick from the most
+      // recently available buffer.
+      unsigned int texture_id = 0;
+      for (auto it = in_flight_surfaces_.rbegin();

[danakj, 2015/12/10 19:46:42]

you could use

for (auto& surface : base::Reversed(in_flight_surfaces)) {}

[ccameron, 2015/12/11 21:14:23]

done

+           it != in_flight_surfaces_.rend(); ++it) {
+        auto& surface = *it;
+        if (surface) {
+          texture_id = surface->texture;
+          break;
+        }
+      }
+      if (!texture_id && displayed_surface_)
+        texture_id = displayed_surface_->texture;
+
+      if (texture_id) {
+        CopyBufferDamage(current_surface_->texture, texture_id, damage,
+                         current_surface_->damage);
+      }
+    }
+    current_surface_->damage = gfx::Rect();
   }
   UpdateBufferDamage(damage);
-  current_surface_.damage = gfx::Rect();
-  in_flight_surfaces_.push_back(current_surface_);
-  current_surface_.texture = 0;
-  current_surface_.image = 0;
+  in_flight_surfaces_.push_back(std::move(current_surface_));
   // Some things reset the framebuffer (CopySubBufferDamage, some GLRenderer
   // paths), so ensure we restore it here.
   context_provider_->ContextGL()->BindFramebuffer(GL_FRAMEBUFFER, fbo_);
@@ -108,7 +120,7 @@ void BufferQueue::Reshape(const gfx::Size& size, float scale_factor) {
   // is cause for concern or if it is benign.
   // http://crbug.com/524624
 #if !defined(OS_MACOSX)
-  DCHECK(!current_surface_.texture);
+  DCHECK(!current_surface_);
 #endif
   size_ = size;
 
@@ -126,80 +138,76 @@ void BufferQueue::RecreateBuffers() {
   // presentable on the device anymore.
   // Unused buffers can be freed directly, they will be re-allocated as needed.
   // Any in flight, current or displayed surface must be replaced.
-  for (size_t i = 0; i < available_surfaces_.size(); i++)
-    FreeSurface(&available_surfaces_[i]);
   available_surfaces_.clear();
 
+  // Recreate all in-flight surfaces and put the recreated copies in the queue.
   for (auto& surface : in_flight_surfaces_)
-    surface = RecreateBuffer(&surface);
+    surface = RecreateBuffer(std::move(surface));
 
-  current_surface_ = RecreateBuffer(&current_surface_);
-  displayed_surface_ = RecreateBuffer(&displayed_surface_);
+  current_surface_ = RecreateBuffer(std::move(current_surface_));
+  displayed_surface_ = RecreateBuffer(std::move(displayed_surface_));
 
-  if (current_surface_.texture) {
+  if (current_surface_) {
     // If we have a texture bound, we will need to re-bind it.
     gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
     gl->BindFramebuffer(GL_FRAMEBUFFER, fbo_);
     gl->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
-                             texture_target_, current_surface_.texture, 0);
+                             texture_target_, current_surface_->texture, 0);
   }
 }
 
-BufferQueue::AllocatedSurface BufferQueue::RecreateBuffer(
-    AllocatedSurface* surface) {
-  if (!surface->texture)
-    return *surface;
-  AllocatedSurface new_surface = GetNextSurface();
-  new_surface.damage = surface->damage;
+scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::RecreateBuffer(
+    scoped_ptr<AllocatedSurface> surface) {
+  if (!surface)
+    return nullptr;
+
+  scoped_ptr<AllocatedSurface> new_surface(GetNextSurface());
+  if (!new_surface)
+    return nullptr;
+
+  new_surface->damage = surface->damage;
 
   // Copy the entire texture.
-  CopyBufferDamage(new_surface.texture, surface->texture, gfx::Rect(),
+  CopyBufferDamage(new_surface->texture, surface->texture, gfx::Rect(),
                    gfx::Rect(size_));
-
-  FreeSurface(surface);
   return new_surface;
 }
 
 void BufferQueue::PageFlipComplete() {
   DCHECK(!in_flight_surfaces_.empty());
-
-  if (displayed_surface_.texture)
-    available_surfaces_.push_back(displayed_surface_);
-
-  displayed_surface_ = in_flight_surfaces_.front();
+  if (displayed_surface_)
+    available_surfaces_.push_back(std::move(displayed_surface_));
+  displayed_surface_ = std::move(in_flight_surfaces_.front());
   in_flight_surfaces_.pop_front();
 }
 
 void BufferQueue::FreeAllSurfaces() {
-  FreeSurface(&displayed_surface_);
-  FreeSurface(&current_surface_);
+  displayed_surface_.reset();
+  current_surface_.reset();
   // This is intentionally not emptied since the swap buffers acks are still
   // expected to arrive.
   for (auto& surface : in_flight_surfaces_)
-    FreeSurface(&surface);
-
-  for (size_t i = 0; i < available_surfaces_.size(); i++)
-    FreeSurface(&available_surfaces_[i]);
-
+    surface = nullptr;
   available_surfaces_.clear();
 }
 
-void BufferQueue::FreeSurface(AllocatedSurface* surface) {
-  if (surface->texture) {
-    gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
-    gl->BindTexture(texture_target_, surface->texture);
-    gl->ReleaseTexImage2DCHROMIUM(texture_target_, surface->image);
-    gl->DeleteTextures(1, &surface->texture);
-    gl->DestroyImageCHROMIUM(surface->image);
-    surface->image = 0;
-    surface->texture = 0;
-    allocated_count_--;
-  }
+void BufferQueue::FreeSurfaceResources(AllocatedSurface* surface) {
+  if (!surface->texture)
+    return;
+
+  gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
+  gl->BindTexture(texture_target_, surface->texture);
+  gl->ReleaseTexImage2DCHROMIUM(texture_target_, surface->image);
+  gl->DeleteTextures(1, &surface->texture);
+  gl->DestroyImageCHROMIUM(surface->image);
+  surface->buffer.reset();
+  allocated_count_--;
 }
 
-BufferQueue::AllocatedSurface BufferQueue::GetNextSurface() {
+scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface() {
   if (!available_surfaces_.empty()) {
-    AllocatedSurface surface = available_surfaces_.back();
+    scoped_ptr<AllocatedSurface> surface =
+        std::move(available_surfaces_.back());
     available_surfaces_.pop_back();
     return surface;
   }
@@ -208,7 +216,7 @@ BufferQueue::AllocatedSurface BufferQueue::GetNextSurface() {
   gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
   gl->GenTextures(1, &texture);
   if (!texture)
-    return AllocatedSurface();
+    return nullptr;
 
   // We don't want to allow anything more than triple buffering.
   DCHECK_LT(allocated_count_, 4U);
@@ -221,33 +229,39 @@ BufferQueue::AllocatedSurface BufferQueue::GetNextSurface() {
   if (!buffer.get()) {
     gl->DeleteTextures(1, &texture);
     DLOG(ERROR) << "Failed to allocate GPU memory buffer";
-    return AllocatedSurface();
+    return nullptr;
   }
 
   unsigned int id = gl->CreateImageCHROMIUM(
       buffer->AsClientBuffer(), size_.width(), size_.height(),
       internal_format_);
-
   if (!id) {
     LOG(ERROR) << "Failed to allocate backing image surface";
     gl->DeleteTextures(1, &texture);
-    return AllocatedSurface();
+    return nullptr;
   }
+
   allocated_count_++;
   gl->BindTexture(texture_target_, texture);
   gl->BindTexImage2DCHROMIUM(texture_target_, id);
-  return AllocatedSurface(buffer.Pass(), texture, id, gfx::Rect(size_));
+  return make_scoped_ptr(new AllocatedSurface(this, std::move(buffer), texture,
+                                              id, gfx::Rect(size_)));
 }
 
-BufferQueue::AllocatedSurface::AllocatedSurface() : texture(0), image(0) {}
-
 BufferQueue::AllocatedSurface::AllocatedSurface(
+    BufferQueue* buffer_queue,
     scoped_ptr<gfx::GpuMemoryBuffer> buffer,
     unsigned int texture,
     unsigned int image,
     const gfx::Rect& rect)
-    : buffer(buffer.release()), texture(texture), image(image), damage(rect) {}
-
-BufferQueue::AllocatedSurface::~AllocatedSurface() {}
+    : buffer_queue(buffer_queue),
+      buffer(buffer.release()),
+      texture(texture),
+      image(image),
+      damage(rect) {}
+
+BufferQueue::AllocatedSurface::~AllocatedSurface() {
+  buffer_queue->FreeSurfaceResources(this);
+}
 
 }  // namespace content