Virtual context MakeCurrent tweaks.

Do not allow GLContextVirtual::MakeCurrent() without a decoder (state
restorer), since this only leads to state bugs.

For this reason remove the necessity of having a current context during surface destruction (it's ugly anyway, since GLSurface is RefCounted).

BUG=248395

Review URL: https://chromiumcodereview.appspot.com/15925007

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

2 files changed, 22 insertions, 13 deletions

diff --git a/content/common/gpu/gpu_command_buffer_stub.cc b/content/common/gpu/gpu_command_buffer_stub.cc
index e0600c7..59591eb 100644
--- a/content/common/gpu/gpu_command_buffer_stub.cc
+++ b/content/common/gpu/gpu_command_buffer_stub.cc
@@ -359,23 +359,15 @@ void GpuCommandBufferStub::Destroy() {
                     destruction_observers_,
                     OnWillDestroyStub());
 
-  scoped_refptr<gfx::GLContext> context;
   if (decoder_) {
-    context = decoder_->GetGLContext();
     decoder_->Destroy(have_context);
     decoder_.reset();
   }
 
   command_buffer_.reset();
 
-  // Make sure that context_ is current while we destroy surface_, because
-  // surface_ may have GL resources that it needs to destroy, and will need
-  // context_ to be current in order to not leak these resources.
-  if (context.get())
-    context->MakeCurrent(surface_.get());
+  // Remove this after crbug.com/248395 is sorted out.
   surface_ = NULL;
-  if (context.get())
-    context->ReleaseCurrent(NULL);
 }
 
 void GpuCommandBufferStub::OnInitializeFailed(IPC::Message* reply_message) {
diff --git a/content/common/gpu/image_transport_surface_mac.cc b/content/common/gpu/image_transport_surface_mac.cc
index c651104..00bc86c 100644
--- a/content/common/gpu/image_transport_surface_mac.cc
+++ b/content/common/gpu/image_transport_surface_mac.cc
@@ -6,6 +6,7 @@
 
 #include "base/mac/scoped_cftyperef.h"
 #include "base/memory/scoped_ptr.h"
+#include "content/common/gpu/gpu_command_buffer_stub.h"
 #include "content/common/gpu/gpu_messages.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gl/gl_bindings.h"
@@ -33,8 +34,10 @@ int RoundUpSurfaceDimension(int number) {
 
 // We are backed by an offscreen surface for the purposes of creating
 // a context, but use FBOs to render to texture backed IOSurface
-class IOSurfaceImageTransportSurface : public gfx::NoOpGLSurfaceCGL,
-                                       public ImageTransportSurface {
+class IOSurfaceImageTransportSurface
+    : public gfx::NoOpGLSurfaceCGL,
+      public ImageTransportSurface,
+      public GpuCommandBufferStub::DestructionObserver {
  public:
   IOSurfaceImageTransportSurface(GpuChannelManager* manager,
                                  GpuCommandBufferStub* stub,
@@ -62,6 +65,9 @@ class IOSurfaceImageTransportSurface : public gfx::NoOpGLSurfaceCGL,
   virtual void OnResize(gfx::Size size, float scale_factor) OVERRIDE;
   virtual void SetLatencyInfo(const ui::LatencyInfo&) OVERRIDE;
 
+  // GpuCommandBufferStub::DestructionObserver implementation.
+  virtual void OnWillDestroyStub() OVERRIDE;
+
  private:
   virtual ~IOSurfaceImageTransportSurface() OVERRIDE;
 
@@ -138,7 +144,6 @@ IOSurfaceImageTransportSurface::IOSurfaceImageTransportSurface(
 }
 
 IOSurfaceImageTransportSurface::~IOSurfaceImageTransportSurface() {
-  Destroy();
 }
 
 bool IOSurfaceImageTransportSurface::Initialize() {
@@ -151,7 +156,14 @@ bool IOSurfaceImageTransportSurface::Initialize() {
 
   if (!helper_->Initialize())
     return false;
-  return NoOpGLSurfaceCGL::Initialize();
+
+  if (!NoOpGLSurfaceCGL::Initialize()) {
+    helper_->Destroy();
+    return false;
+  }
+
+  helper_->stub()->AddDestructionObserver(this);
+  return true;
 }
 
 void IOSurfaceImageTransportSurface::Destroy() {
@@ -313,6 +325,11 @@ void IOSurfaceImageTransportSurface::SetLatencyInfo(
   latency_info_ = latency_info;
 }
 
+void IOSurfaceImageTransportSurface::OnWillDestroyStub() {
+  helper_->stub()->RemoveDestructionObserver(this);
+  Destroy();
+}
+
 void IOSurfaceImageTransportSurface::UnrefIOSurface() {
   // If we have resources to destroy, then make sure that we have a current
   // context which we can use to delete the resources.