Make sure we create/use/delete buffer on the same context.

This is for GLImageMemory's optimization path.

BUG=1087047
TEST=manual
R=kbr@chromium.org,khushalsagar@chromium.org

Change-Id: I82d3ceeac36644f95e3a52b2a943c4ecaeb651d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2226926Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774927}

ui/gl/gl_context.h

@@ -13,6 +13,7 @@
 #include "base/cancelable_callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/synchronization/atomic_flag.h"
 #include "build/build_config.h"
 #include "ui/gfx/extension_set.h"
@@ -90,7 +91,8 @@ struct GL_EXPORT GLContextAttribs {
 };
 
 // Encapsulates an OpenGL context, hiding platform specific management.
-class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
+class GL_EXPORT GLContext : public base::RefCounted<GLContext>,
+                            public base::SupportsWeakPtr<GLContext> {
  public:
   explicit GLContext(GLShareGroup* share_group);
 

ui/gl/gl_image_memory.cc

@@ -21,8 +21,10 @@
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_enums.h"
+#include "ui/gl/gl_surface.h"
 #include "ui/gl/gl_version_info.h"
 #include "ui/gl/scoped_binders.h"
+#include "ui/gl/scoped_make_current.h"
 
 using gfx::BufferFormat;
 
@@ -227,8 +229,11 @@ GLImageMemory::GLImageMemory(const gfx::Size& size)
       stride_(0) {}
 
 GLImageMemory::~GLImageMemory() {
-  if (buffer_)
+  if (buffer_ && original_context_ && original_surface_) {
+    ui::ScopedMakeCurrent make_current(original_context_.get(),
+                                       original_surface_.get());
     glDeleteBuffersARB(1, &buffer_);
+  }
 }
 
 // static
@@ -262,6 +267,7 @@ bool GLImageMemory::Initialize(const unsigned char* memory,
   tex_image_from_pbo_is_slow = true;
 #endif  // OS_WIN
   GLContext* context = GLContext::GetCurrent();
+  DCHECK(context);
   if (!tex_image_from_pbo_is_slow && SupportsPBO(context) &&
       (SupportsMapBuffer(context) || SupportsMapBufferRange(context))) {
     constexpr size_t kTaskBytes = 1024 * 1024;
@@ -273,6 +279,10 @@ bool GLImageMemory::Initialize(const unsigned char* memory,
       ScopedBufferBinder binder(GL_PIXEL_UNPACK_BUFFER, buffer_);
       glBufferData(GL_PIXEL_UNPACK_BUFFER, buffer_bytes_, nullptr,
                    GL_DYNAMIC_DRAW);
+      original_context_ = context->AsWeakPtr();
+      GLSurface* surface = GLSurface::GetCurrent();
+      DCHECK(surface);
+      original_surface_ = surface->AsWeakPtr();
     }
   }
 
@@ -332,7 +342,9 @@ bool GLImageMemory::CopyTexImage(unsigned target) {
   GLint data_row_length = DataRowLength(stride_, format_);
   base::Optional<std::vector<uint8_t>> gles2_data;
 
-  if (GLContext::GetCurrent()->GetVersionInfo()->is_es) {
+  GLContext* context = GLContext::GetCurrent();
+  DCHECK(context);
+  if (context->GetVersionInfo()->is_es) {
     gles2_data = GLES2Data(size_, format_, stride_, memory_, &data_format,
                            &data_type, &data_row_length);
   }
@@ -350,17 +362,18 @@ bool GLImageMemory::CopyTexImage(unsigned target) {
     size = buffer_bytes_;
   }
 
-  if (buffer_) {
+  bool uploaded = false;
+  if (buffer_ && original_context_.get() == context) {
     glTexImage2D(target, 0, GetInternalFormat(), size_.width(), size_.height(),
                  0, data_format, data_type, nullptr);
 
     ScopedBufferBinder binder(GL_PIXEL_UNPACK_BUFFER, buffer_);
 
     void* dst = nullptr;
-    if (SupportsMapBuffer(GLContext::GetCurrent())) {
+    if (SupportsMapBuffer(context)) {
       dst = glMapBuffer(GL_PIXEL_UNPACK_BUFFER, GL_WRITE_ONLY);
     } else {
-      DCHECK(SupportsMapBufferRange(GLContext::GetCurrent()));
+      DCHECK(SupportsMapBufferRange(context));
       dst = glMapBufferRange(GL_PIXEL_UNPACK_BUFFER, 0, size, GL_MAP_WRITE_BIT);
     }
 
@@ -381,13 +394,14 @@ bool GLImageMemory::CopyTexImage(unsigned target) {
 
       glTexSubImage2D(target, 0, 0, 0, size_.width(), size_.height(),
                       data_format, data_type, 0);
+      uploaded = true;
     } else {
       glDeleteBuffersARB(1, &buffer_);
       buffer_ = 0;
     }
   }
 
-  if (!buffer_) {
+  if (!uploaded) {
     glTexImage2D(target, 0, GetInternalFormat(), size_.width(), size_.height(),
                  0, data_format, data_type, src);
   }

ui/gl/gl_image_memory.h

@@ -10,11 +10,14 @@
 #include <stddef.h>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/numerics/safe_math.h"
 #include "ui/gfx/buffer_types.h"
 #include "ui/gl/gl_export.h"
 
 namespace gl {
+class GLContext;
+class GLSurface;
 
 class GL_EXPORT GLImageMemory : public GLImage {
  public:
@@ -65,6 +68,9 @@ class GL_EXPORT GLImageMemory : public GLImage {
   size_t stride_;
 
   unsigned buffer_ = 0;
+  // The context/surface from which the |buffer_| is created.
+  base::WeakPtr<GLContext> original_context_;
+  base::WeakPtr<GLSurface> original_surface_;
   size_t buffer_bytes_ = 0;
   int memcpy_tasks_ = 0;
 

ui/gl/gl_surface.h

@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "build/build_config.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_f.h"
@@ -44,7 +45,8 @@ class EGLTimestampClient;
 
 // Encapsulates a surface that can be rendered to with GL, hiding platform
 // specific management.
-class GL_EXPORT GLSurface : public base::RefCounted<GLSurface> {
+class GL_EXPORT GLSurface : public base::RefCounted<GLSurface>,
+                            public base::SupportsWeakPtr<GLSurface> {
  public:
   GLSurface();