cc: Avoid acquiring the context lock if unnecessary.

Acquiring the context lock marks the context as busy and can delay
cleanup of gpu memory. Avoid acquiring it if there are no pending raster
queries.

R=ericrk@chromium.org

Bug: 896260, 894200
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I91b9ea85ef11b0b31e9d14599800efb0d2ca00e0
Reviewed-on: https://chromium-review.googlesource.com/c/1287931
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600824}

cc/raster/gpu_raster_buffer_provider.cc

@@ -456,6 +456,43 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThread(
     const gfx::AxisTransform2d& transform,
     const RasterSource::PlaybackSettings& playback_settings,
     const GURL& url) {
+  PendingRasterQuery query;
+  gpu::SyncToken raster_finished_token = PlaybackOnWorkerThreadInternal(
+      mailbox, texture_target, texture_is_overlay_candidate, sync_token,
+      resource_size, resource_format, color_space,
+      resource_has_previous_content, raster_source, raster_full_rect,
+      raster_dirty_rect, new_content_id, transform, playback_settings, url,
+      &query);
+
+  {
+    // Note that it is important to scope the raster context lock to
+    // PlaybackOnWorkerThreadInternal and release it before acquiring this lock
+    // to avoid a deadlock in CheckRasterFinishedQueries which acquires the
+    // raster context lock while holding this lock.
+    base::AutoLock hold(pending_raster_queries_lock_);
+    pending_raster_queries_.push_back(query);
+  }
+
+  return raster_finished_token;
+}
+
+gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThreadInternal(
+    gpu::Mailbox* mailbox,
+    GLenum texture_target,
+    bool texture_is_overlay_candidate,
+    const gpu::SyncToken& sync_token,
+    const gfx::Size& resource_size,
+    viz::ResourceFormat resource_format,
+    const gfx::ColorSpace& color_space,
+    bool resource_has_previous_content,
+    const RasterSource* raster_source,
+    const gfx::Rect& raster_full_rect,
+    const gfx::Rect& raster_dirty_rect,
+    uint64_t new_content_id,
+    const gfx::AxisTransform2d& transform,
+    const RasterSource::PlaybackSettings& playback_settings,
+    const GURL& url,
+    PendingRasterQuery* query) {
   viz::RasterContextProvider::ScopedRasterContextLock scoped_context(
       worker_context_provider_, url.possibly_invalid_spec().c_str());
   gpu::raster::RasterInterface* ri = scoped_context.RasterInterface();
@@ -483,10 +520,8 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThread(
   }
 
   // Use a query to time the GPU side work for rasterizing this tile.
-  pending_raster_queries_.emplace_back();
-  auto& query = pending_raster_queries_.back();
-  ri->GenQueriesEXT(1, &query.query_id);
-  ri->BeginQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM, query.query_id);
+  ri->GenQueriesEXT(1, &query->query_id);
+  ri->BeginQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM, query->query_id);
 
   {
     base::ElapsedTimer timer;
@@ -507,7 +542,7 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThread(
                       ShouldUnpremultiplyAndDitherResource(resource_format),
                       max_tile_size_);
     }
-    query.worker_duration = timer.Elapsed();
+    query->worker_duration = timer.Elapsed();
   }
 
   ri->EndQueryEXT(GL_COMMANDS_ISSUED_CHROMIUM);
@@ -532,6 +567,10 @@ bool GpuRasterBufferProvider::ShouldUnpremultiplyAndDitherResource(
       base::TimeDelta::FromMilliseconds(100), 100);
 
 bool GpuRasterBufferProvider::CheckRasterFinishedQueries() {
+  base::AutoLock hold(pending_raster_queries_lock_);
+  if (pending_raster_queries_.empty())
+    return false;
+
   viz::RasterContextProvider::ScopedRasterContextLock scoped_context(
       worker_context_provider_);
   auto* ri = scoped_context.RasterInterface();

cc/raster/gpu_raster_buffer_provider.h

@@ -114,7 +114,32 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider {
     DISALLOW_COPY_AND_ASSIGN(RasterBufferImpl);
   };
 
+  struct PendingRasterQuery {
+    // The id for querying the duration in executing the GPU side work.
+    GLuint query_id = 0u;
+
+    // The duration for executing the work on the raster worker thread.
+    base::TimeDelta worker_duration;
+  };
+
   bool ShouldUnpremultiplyAndDitherResource(viz::ResourceFormat format) const;
+  gpu::SyncToken PlaybackOnWorkerThreadInternal(
+      gpu::Mailbox* mailbox,
+      GLenum texture_target,
+      bool texture_is_overlay_candidate,
+      const gpu::SyncToken& sync_token,
+      const gfx::Size& resource_size,
+      viz::ResourceFormat resource_format,
+      const gfx::ColorSpace& color_space,
+      bool resource_has_previous_content,
+      const RasterSource* raster_source,
+      const gfx::Rect& raster_full_rect,
+      const gfx::Rect& raster_dirty_rect,
+      uint64_t new_content_id,
+      const gfx::AxisTransform2d& transform,
+      const RasterSource::PlaybackSettings& playback_settings,
+      const GURL& url,
+      PendingRasterQuery* query);
 
   viz::ContextProvider* const compositor_context_provider_;
   viz::RasterContextProvider* const worker_context_provider_;
@@ -125,15 +150,11 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider {
   const bool unpremultiply_and_dither_low_bit_depth_tiles_;
   const bool enable_oop_rasterization_;
 
-  struct PendingRasterQuery {
-    // The id for querying the duration in executing the GPU side work.
-    GLuint query_id = 0u;
-
-    // The duration for executing the work on the raster worker thread.
-    base::TimeDelta worker_duration;
-  };
-  // This should only be accessed with the context lock acquired.
-  base::circular_deque<PendingRasterQuery> pending_raster_queries_;
+  // Note that this lock should never be acquired while holding the raster
+  // context lock.
+  base::Lock pending_raster_queries_lock_;
+  base::circular_deque<PendingRasterQuery> pending_raster_queries_
+      GUARDED_BY(pending_raster_queries_lock_);
 
   DISALLOW_COPY_AND_ASSIGN(GpuRasterBufferProvider);
 };