RefCount texture in SharedImageBackingEglImage

After https://crrev.com/2390861 we started to use source texture for all
operations on original context. SharedImageBacking are ref-counted by
their representation, but the source_texture was deleted when client
deletes shared image (i.e when FactoryRepresentation is deleted). This
is problematic as shared image could be deleted before raster finished.

This CL adds ref-counted wrapper to source texture to keep it alive
until last representation on original context and thread is alive.

Bug: 1127320
Change-Id: I395f6177b752acc163f05f2d365073c67a25991a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2407182
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806502}

gpu/command_buffer/service/shared_image_backing_egl_image.cc

@@ -17,25 +17,43 @@
 
 namespace gpu {
 
+class SharedImageBackingEglImage::TextureHolder
+    : public base::RefCounted<TextureHolder> {
+ public:
+  TextureHolder(gles2::Texture* texture) : texture_(texture) {}
+
+  void MarkContextLost() { context_lost_ = true; }
+
+  gles2::Texture* texture() { return texture_; }
+
+ private:
+  friend class base::RefCounted<TextureHolder>;
+
+  ~TextureHolder() { texture_->RemoveLightweightRef(!context_lost_); }
+
+  gles2::Texture* const texture_;
+  bool context_lost_ = false;
+};
+
 // Implementation of SharedImageRepresentationGLTexture which uses GL texture
 // which is an EGLImage sibling.
 class SharedImageRepresentationEglImageGLTexture
     : public SharedImageRepresentationGLTexture {
  public:
-  SharedImageRepresentationEglImageGLTexture(SharedImageManager* manager,
-                                             SharedImageBacking* backing,
-                                             MemoryTypeTracker* tracker,
-                                             gles2::Texture* texture,
-                                             bool should_delete_texture)
+  using TextureHolder = SharedImageBackingEglImage::TextureHolder;
+  SharedImageRepresentationEglImageGLTexture(
+      SharedImageManager* manager,
+      SharedImageBacking* backing,
+      MemoryTypeTracker* tracker,
+      scoped_refptr<TextureHolder> texture_holder)
       : SharedImageRepresentationGLTexture(manager, backing, tracker),
-        texture_(texture),
-        should_delete_texture_(should_delete_texture) {}
+        texture_holder_(std::move(texture_holder)) {}
 
   ~SharedImageRepresentationEglImageGLTexture() override {
     EndAccess();
-
-    if (texture_ && should_delete_texture_)
-      texture_->RemoveLightweightRef(has_context());
+    if (!has_context())
+      texture_holder_->MarkContextLost();
+    texture_holder_.reset();
   }
 
   bool BeginAccess(GLenum mode) override {
@@ -69,7 +87,7 @@ class SharedImageRepresentationEglImageGLTexture
     mode_ = RepresentationAccessMode::kNone;
   }
 
-  gles2::Texture* GetTexture() override { return texture_; }
+  gles2::Texture* GetTexture() override { return texture_holder_->texture(); }
 
   bool SupportsMultipleConcurrentReadAccess() override { return true; }
 
@@ -78,8 +96,7 @@ class SharedImageRepresentationEglImageGLTexture
     return static_cast<SharedImageBackingEglImage*>(backing());
   }
 
-  gles2::Texture* texture_;
-  const bool should_delete_texture_;
+  scoped_refptr<TextureHolder> texture_holder_;
   RepresentationAccessMode mode_ = RepresentationAccessMode::kNone;
   DISALLOW_COPY_AND_ASSIGN(SharedImageRepresentationEglImageGLTexture);
 };
@@ -114,14 +131,16 @@ SharedImageBackingEglImage::SharedImageBackingEglImage(
   // On some GPUs (NVidia) keeping reference to egl image itself is not enough,
   // we must keep reference to at least one sibling.
   if (workarounds.dont_delete_source_texture_for_egl_image) {
-    source_texture_ = GenEGLImageSibling();
+    auto* texture = GenEGLImageSibling();
+    if (texture)
+      source_texture_holder_ = base::MakeRefCounted<TextureHolder>(texture);
   }
 }
 
 SharedImageBackingEglImage::~SharedImageBackingEglImage() {
   // Un-Register this backing from the |batch_access_manager_|.
   batch_access_manager_->UnregisterEglBacking(this);
-  DCHECK(!source_texture_);
+  DCHECK(!source_texture_holder_);
 }
 
 void SharedImageBackingEglImage::Update(
@@ -141,17 +160,21 @@ SharedImageBackingEglImage::ProduceGLTexture(SharedImageManager* manager,
   // On some GPUs (Mali, mostly Android 9, like J7) glTexSubImage fails on egl
   // image sibling. So we use the original texture if we're on the same gl
   // context. see https://crbug.com/1117370
-  if (source_texture_ && created_on_context_ == gl::g_current_gl_context) {
+  // If we're on the same context we're on the same thread, so
+  // source_texture_holder_ is accessed only from thread we created it and
+  // doesn't need lock.
+  if (created_on_context_ == gl::g_current_gl_context &&
+      source_texture_holder_) {
     return std::make_unique<SharedImageRepresentationEglImageGLTexture>(
-        manager, this, tracker, source_texture_,
-        /*should_delete_texture=*/false);
+        manager, this, tracker, source_texture_holder_);
   }
 
   auto* texture = GenEGLImageSibling();
   if (!texture)
     return nullptr;
+  auto texture_holder = base::MakeRefCounted<TextureHolder>(texture);
   return std::make_unique<SharedImageRepresentationEglImageGLTexture>(
-      manager, this, tracker, texture, /*should_delete_texture=*/true);
+      manager, this, tracker, std::move(texture_holder));
 }
 
 std::unique_ptr<SharedImageRepresentationSkia>
@@ -332,10 +355,9 @@ void SharedImageBackingEglImage::MarkForDestruction() {
   AutoLock auto_lock(this);
   DCHECK(!have_context() || created_on_context_ == gl::g_current_gl_context);
 
-  if (source_texture_) {
-    source_texture_->RemoveLightweightRef(have_context());
-    source_texture_ = nullptr;
-  }
+  if (!have_context())
+    source_texture_holder_->MarkContextLost();
+  source_texture_holder_.reset();
 }
 
 }  // namespace gpu

gpu/command_buffer/service/shared_image_backing_egl_image.h

@@ -73,6 +73,7 @@ class SharedImageBackingEglImage : public ClearTrackingSharedImageBacking {
  private:
   friend class SharedImageBatchAccessManager;
   friend class SharedImageRepresentationEglImageGLTexture;
+  class TextureHolder;
 
   // Use to create EGLImage texture target from the same EGLImage object.
   gles2::Texture* GenEGLImageSibling();
@@ -81,7 +82,7 @@ class SharedImageBackingEglImage : public ClearTrackingSharedImageBacking {
 
   const GLuint gl_format_;
   const GLuint gl_type_;
-  gles2::Texture* source_texture_ = nullptr;
+  scoped_refptr<TextureHolder> source_texture_holder_;
   gl::GLApi* created_on_context_;
 
   // This class encapsulates the EGLImage object for android.