Only set on_finished_callback if read lock set

FrameResourceFence was introduced in
https://chromium-review.googlesource.com/c/chromium/src/+/1600617 to
properly support display of video from GpuMemoryBuffers. Idea is that
DisplayResourceProvider will signal that a fence is needed
(https://cs.chromium.org/chromium/src/components/viz/service/display/display_resource_provider.cc?rcl=b42e627df3d2285cccb73e90ace0c96667a42652&l=514)
and later check if the fence has passed before deleting the resource
(https://cs.chromium.org/chromium/src/components/viz/service/display/display_resource_provider.cc?rcl=b42e627df3d2285cccb73e90ace0c96667a42652&l=657).

This CL does 2 things.

(1) Fixes a bug where we were previously signaling the fence in
    DisplayResourceProvider::LockForRead, when we shouldn't
    have. We should only signal the fence once it is safe to delete.

(2) Only sets |on_finished_callback| if some resource needed a readlock
    fence. This microoptimization improves performance on
    RendererPerfTest.

Bug: 974359
Change-Id: If72985b566a6082382c0baf6887408fa42bb9299
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1775100Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691625}

components/viz/service/display/skia_renderer.cc

@@ -614,17 +614,23 @@ SkiaRenderer::SkiaRenderer(const RendererSettings* settings,
 
 SkiaRenderer::~SkiaRenderer() = default;
 
-class FrameResourceFence : public ResourceFence {
+class SkiaRenderer::FrameResourceFence : public ResourceFence {
  public:
   FrameResourceFence() = default;
 
   // ResourceFence implementation.
-  void Set() override { event_.Signal(); }
+  void Set() override { set_ = true; }
   bool HasPassed() override { return event_.IsSignaled(); }
 
+  bool WasSet() { return set_; }
+  void Signal() { event_.Signal(); }
+
  private:
   ~FrameResourceFence() override = default;
 
+  // Accessed only from compositor thread.
+  bool set_ = false;
+
   base::WaitableEvent event_;
 
   DISALLOW_COPY_AND_ASSIGN(FrameResourceFence);
@@ -655,8 +661,8 @@ void SkiaRenderer::BeginDrawingFrame() {
     read_lock_fence = sync_queries_->StartNewFrame();
     current_frame_resource_fence_ = nullptr;
   } else {
-    read_lock_fence = base::MakeRefCounted<FrameResourceFence>();
-    current_frame_resource_fence_ = read_lock_fence;
+    current_frame_resource_fence_ = base::MakeRefCounted<FrameResourceFence>();
+    read_lock_fence = current_frame_resource_fence_;
   }
   resource_provider_->SetReadLockFence(read_lock_fence.get());
 
@@ -679,6 +685,7 @@ void SkiaRenderer::FinishDrawingFrame() {
   if (sync_queries_) {
     sync_queries_->EndCurrentFrame();
   }
+  current_frame_resource_fence_ = nullptr;
   current_canvas_ = nullptr;
   current_surface_ = nullptr;
 
@@ -1909,10 +1916,12 @@ void SkiaRenderer::FinishDrawingQuadList() {
       // Signal |current_frame_resource_fence_| when the root render pass is
       // finished.
       if (current_frame_resource_fence_ &&
+          current_frame_resource_fence_->WasSet() &&
           current_frame()->current_render_pass ==
               current_frame()->root_render_pass) {
-        on_finished_callback = base::BindOnce(
-            &ResourceFence::Set, std::move(current_frame_resource_fence_));
+        on_finished_callback =
+            base::BindOnce(&FrameResourceFence::Signal,
+                           std::move(current_frame_resource_fence_));
       }
       gpu::SyncToken sync_token =
           skia_output_surface_->SubmitPaint(std::move(on_finished_callback));

components/viz/service/display/skia_renderer.h

@@ -205,7 +205,8 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer {
   SkCanvas* root_canvas_ = nullptr;
   SkCanvas* current_canvas_ = nullptr;
   SkSurface* current_surface_ = nullptr;
-  scoped_refptr<ResourceFence> current_frame_resource_fence_;
+  class FrameResourceFence;
+  scoped_refptr<FrameResourceFence> current_frame_resource_fence_;
 
   bool disable_picture_quad_image_filtering_ = false;
   bool is_scissor_enabled_ = false;