Revert "Import GLFence in EndOverlayAccess for AHB"

This reverts commit e8f078f3.

Reason for revert:
BeginWriteSkia() call is moved out of critical path. This change is not
necessary anymore. For code health revert it. 

Original change's description:
> Import GLFence in EndOverlayAccess for AHB
> 
> When using SurfaceControl after surface has been returned we need to
> wait until fence provided by system will be signaled. When using GL
> for compositing, this is implemented as importing provided file
> descriptor as egl fence and issue ServerWait().
> 
> GLRenderer does it when we get surface back. With SkiaRenderer it
> happens on next BeginWrite for the SharedImage. Import is expensive
> operation and BeginWrite is on critical path.
> 
> This CL moves import to EndOverlayAccess as we will do it anyway.
> Then at BeginWrite time we only issue ServerWait().
> 
> In local tests with scrolling simple page it saves 200us in
> DrawToSwapUs metric.
> 
> Change-Id: I6efb2783d0e287dc5e4fbb5707c507328e75a32f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2019607
> Reviewed-by: Jonathan Backer <backer@chromium.org>
> Reviewed-by: Ken Buchanan <kenrb@chromium.org>
> Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#735577}

TBR=backer@chromium.org,kenrb@chromium.org,vasilyt@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

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

gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer.cc

@@ -191,8 +191,7 @@ class SharedImageBackingAHB : public ClearTrackingSharedImageBacking {
                         base::android::ScopedHardwareBufferHandle handle,
                         size_t estimated_size,
                         bool is_thread_safe,
-                        base::ScopedFD initial_upload_fd,
-                        bool using_gl);
+                        base::ScopedFD initial_upload_fd);
 
   ~SharedImageBackingAHB() override;
 
@@ -214,7 +213,6 @@ class SharedImageBackingAHB : public ClearTrackingSharedImageBacking {
                base::ScopedFD end_read_fd);
   gl::GLImage* BeginOverlayAccess();
   void EndOverlayAccess();
-  std::unique_ptr<gl::GLFence> TakeReadGLFence();
 
  protected:
   std::unique_ptr<SharedImageRepresentationGLTexture> ProduceGLTexture(
@@ -247,11 +245,8 @@ class SharedImageBackingAHB : public ClearTrackingSharedImageBacking {
   base::flat_set<const SharedImageRepresentation*> active_readers_
       GUARDED_BY(lock_);
 
-  std::unique_ptr<gl::GLFence> read_gl_fence_ GUARDED_BY(lock_);
-
   scoped_refptr<OverlayImage> overlay_image_ GUARDED_BY(lock_);
   bool is_overlay_accessing_ GUARDED_BY(lock_) = false;
-  const bool using_gl_;
 
   DISALLOW_COPY_AND_ASSIGN(SharedImageBackingAHB);
 };
@@ -288,15 +283,8 @@ class SharedImageRepresentationGLTextureAHB
       if (!ahb_backing()->BeginWrite(&sync_fd))
         return false;
 
-      auto gl_fence = ahb_backing()->TakeReadGLFence();
-      if (gl_fence) {
-        gl_fence->ServerWait();
-      }
-
-      if (sync_fd.is_valid()) {
-        if (!InsertEglFenceAndWait(std::move(sync_fd)))
-          return false;
-      }
+      if (!InsertEglFenceAndWait(std::move(sync_fd)))
+        return false;
     }
 
     if (mode == GL_SHARED_IMAGE_ACCESS_MODE_READ_CHROMIUM) {
@@ -560,8 +548,7 @@ SharedImageBackingAHB::SharedImageBackingAHB(
     base::android::ScopedHardwareBufferHandle handle,
     size_t estimated_size,
     bool is_thread_safe,
-    base::ScopedFD initial_upload_fd,
-    bool using_gl)
+    base::ScopedFD initial_upload_fd)
     : ClearTrackingSharedImageBacking(mailbox,
                                       format,
                                       size,
@@ -570,8 +557,7 @@ SharedImageBackingAHB::SharedImageBackingAHB(
                                       estimated_size,
                                       is_thread_safe),
       hardware_buffer_handle_(std::move(handle)),
-      write_sync_fd_(std::move(initial_upload_fd)),
-      using_gl_(using_gl) {
+      write_sync_fd_(std::move(initial_upload_fd)) {
   DCHECK(hardware_buffer_handle_.is_valid());
 }
 
@@ -797,15 +783,6 @@ void SharedImageBackingAHB::EndOverlayAccess() {
 
   auto fence_fd = overlay_image_->TakeEndFence();
   read_sync_fd_ = gl::MergeFDs(std::move(read_sync_fd_), std::move(fence_fd));
-
-  if (using_gl_ && read_sync_fd_.is_valid()) {
-    read_gl_fence_ = CreateEglFence(std::move(read_sync_fd_));
-  }
-}
-
-std::unique_ptr<gl::GLFence> SharedImageBackingAHB::TakeReadGLFence() {
-  AutoLock auto_lock(this);
-  return std::move(read_gl_fence_);
 }
 
 gles2::Texture* SharedImageBackingAHB::GenGLTexture() {
@@ -867,9 +844,7 @@ gles2::Texture* SharedImageBackingAHB::GenGLTexture() {
 
 SharedImageBackingFactoryAHB::SharedImageBackingFactoryAHB(
     const GpuDriverBugWorkarounds& workarounds,
-    const GpuFeatureInfo& gpu_feature_info,
-    SharedContextState* context_state)
-    : context_state_(context_state) {
+    const GpuFeatureInfo& gpu_feature_info) {
   scoped_refptr<gles2::FeatureInfo> feature_info =
       new gles2::FeatureInfo(workarounds, gpu_feature_info);
   feature_info->Initialize(ContextType::CONTEXT_TYPE_OPENGLES2, false,
@@ -1083,11 +1058,9 @@ std::unique_ptr<SharedImageBacking> SharedImageBackingFactoryAHB::MakeBacking(
     initial_upload_fd = base::ScopedFD(fence);
   }
 
-  bool using_gl = context_state_ && context_state_->GrContextIsGL();
-
   auto backing = std::make_unique<SharedImageBackingAHB>(
       mailbox, format, size, color_space, usage, std::move(handle),
-      estimated_size, is_thread_safe, std::move(initial_upload_fd), using_gl);
+      estimated_size, is_thread_safe, std::move(initial_upload_fd));
 
   // If we uploaded initial data, set the backing as cleared.
   if (!pixel_data.empty())
@@ -1168,12 +1141,10 @@ SharedImageBackingFactoryAHB::CreateSharedImage(
     return nullptr;
   }
 
-  bool using_gl = context_state_ && context_state_->GrContextIsGL();
-
   return std::make_unique<SharedImageBackingAHB>(
       mailbox, resource_format, size, color_space, usage,
       std::move(handle.android_hardware_buffer), estimated_size, false,
-      base::ScopedFD(), using_gl);
+      base::ScopedFD());
 }
 
 }  // namespace gpu

gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer.h

@@ -17,7 +17,6 @@ class ColorSpace;
 }  // namespace gfx
 
 namespace gpu {
-class SharedContextState;
 class SharedImageBacking;
 class GpuDriverBugWorkarounds;
 struct GpuFeatureInfo;
@@ -29,8 +28,7 @@ class GPU_GLES2_EXPORT SharedImageBackingFactoryAHB
     : public SharedImageBackingFactory {
  public:
   SharedImageBackingFactoryAHB(const GpuDriverBugWorkarounds& workarounds,
-                               const GpuFeatureInfo& gpu_feature_info,
-                               SharedContextState* context_state);
+                               const GpuFeatureInfo& gpu_feature_info);
   ~SharedImageBackingFactoryAHB() override;
 
   // SharedImageBackingFactory implementation.
@@ -97,8 +95,6 @@ class GPU_GLES2_EXPORT SharedImageBackingFactoryAHB
   // Used to limit the max size of AHardwareBuffer.
   int32_t max_gl_texture_size_ = 0;
 
-  SharedContextState* const context_state_;
-
   DISALLOW_COPY_AND_ASSIGN(SharedImageBackingFactoryAHB);
 };
 

gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer_unittest.cc

@@ -65,7 +65,7 @@ class SharedImageBackingFactoryAHBTest : public testing::Test {
     context_state_->InitializeGL(GpuPreferences(), std::move(feature_info));
 
     backing_factory_ = std::make_unique<SharedImageBackingFactoryAHB>(
-        workarounds, GpuFeatureInfo(), context_state_.get());
+        workarounds, GpuFeatureInfo());
 
     memory_type_tracker_ = std::make_unique<MemoryTypeTracker>(nullptr);
     shared_image_representation_factory_ =

gpu/command_buffer/service/shared_image_factory.cc

@@ -105,7 +105,7 @@ SharedImageFactory::SharedImageFactory(
         std::make_unique<ExternalVkImageFactory>(context_state);
   }
   interop_backing_factory_ = std::make_unique<SharedImageBackingFactoryAHB>(
-      workarounds, gpu_feature_info, context_state);
+      workarounds, gpu_feature_info);
 #elif defined(OS_MACOSX)
   // OSX
   DCHECK(!using_vulkan_);

gpu/ipc/common/android/android_image_reader_utils.cc

@@ -51,9 +51,16 @@ bool DeleteAImageAsync(AImage* image,
   return true;
 }
 
-std::unique_ptr<gl::GLFence> CreateEglFence(base::ScopedFD acquire_fence_fd) {
+bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd) {
   int fence_fd = acquire_fence_fd.release();
 
+  // If fence_fd is -1, we do not need synchronization fence and image is ready
+  // to be used immediately. Also we dont need to close any fd. Else we need to
+  // create a sync fence which is used to signal when the buffer is ready to be
+  // consumed.
+  if (fence_fd == -1)
+    return true;
+
   // Create attribute-value list with the fence_fd.
   EGLint attribs[] = {EGL_SYNC_NATIVE_FENCE_FD_ANDROID, fence_fd, EGL_NONE};
 
@@ -65,30 +72,16 @@ std::unique_ptr<gl::GLFence> CreateEglFence(base::ScopedFD acquire_fence_fd) {
 
   // If above method fails to create an egl_fence, we need to close the file
   // descriptor.
-  if (!egl_fence) {
+  if (egl_fence == nullptr) {
     // Create a scoped FD to close fence_fd.
     base::ScopedFD temp_fd(fence_fd);
     LOG(ERROR) << " Failed to created egl fence object ";
+    return false;
   }
 
-  return egl_fence;
-}
-
-bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd) {
-  // If acquire_fence_fd is not valid we do not need synchronization fence and
-  // image is ready to be used immediately. Also we dont need to close any fd.
-  // Else we need to create a sync fence which is used to signal when the buffer
-  // is ready to be consumed.
-  if (!acquire_fence_fd.is_valid())
-    return true;
-
-  auto egl_fence = CreateEglFence(std::move(acquire_fence_fd));
-  if (egl_fence) {
-    // Make the server wait and not the client.
-    egl_fence->ServerWait();
-    return true;
-  }
-  return false;
+  // Make the server wait and not the client.
+  egl_fence->ServerWait();
+  return true;
 }
 
 bool CreateAndBindEglImage(const AImage* image,

gpu/ipc/common/android/android_image_reader_utils.h

@@ -12,10 +12,6 @@
 #include "gpu/gpu_export.h"
 #include "ui/gl/gl_bindings.h"
 
-namespace gl {
-class GLFence;
-}
-
 namespace gpu {
 
 // Create and inserts an egl fence and exports a ScopedFD from it.
@@ -25,13 +21,8 @@ GPU_EXPORT base::ScopedFD CreateEglFenceAndExportFd();
 bool DeleteAImageAsync(AImage* image,
                        base::android::AndroidImageReader* loader);
 
-// Create and insert an EGL fence and imports the provided fence fd and issues
-// ServerWait on it.
-GPU_EXPORT bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd);
-
 // Create and insert an EGL fence and imports the provided fence fd.
-GPU_EXPORT std::unique_ptr<gl::GLFence> CreateEglFence(
-    base::ScopedFD acquire_fence_fd);
+GPU_EXPORT bool InsertEglFenceAndWait(base::ScopedFD acquire_fence_fd);
 
 // Create an EGL image from the AImage via AHardwarebuffer. Bind this EGL image
 // to the texture target target_id. This changes the texture binding on the