cc: Detect miss-behaving fence extensions when using 1-copy rasterizer.

This adds a wait_if_needed parameter to
ResourcePool::CheckBusyResources. CheckBusyResources will use existing
ResourceProvider API to wait for any read locks to complete if needed
by a resource prior to checking if it's still busy when this parameter
is true.

1-copy rasterizer will use this functionality after too many failed
attempts to check if copy operations have completed. This provides a
reliable method to detect drivers with miss-behaving fence extensions.

BUG=406404

Review URL: https://codereview.chromium.org/683263004

Cr-Commit-Position: refs/heads/master@{#301926}

5 files changed, 36 insertions, 13 deletions

diff --git a/cc/resources/one_copy_raster_worker_pool.cc b/cc/resources/one_copy_raster_worker_pool.cc
index bbf3136..c803262 100644
--- a/cc/resources/one_copy_raster_worker_pool.cc
+++ b/cc/resources/one_copy_raster_worker_pool.cc
@@ -87,6 +87,10 @@ const int kMaxCopyOperations = 16;
 // Delay been checking for copy operations to complete.
 const int kCheckForCompletedCopyOperationsTickRateMs = 1;
 
+// Number of failed attempts to allow before we perform a check that will
+// wait for copy operations to complete if needed.
+const int kFailedAttemptsBeforeWaitIfNeeded = 256;
+
 }  // namespace
 
 OneCopyRasterWorkerPool::CopyOperation::CopyOperation(
@@ -195,7 +199,7 @@ void OneCopyRasterWorkerPool::ScheduleTasks(RasterTaskQueue* queue) {
                    task_set));
   }
 
-  resource_pool_->CheckBusyResources();
+  resource_pool_->CheckBusyResources(false);
 
   for (RasterTaskQueue::Item::Vector::const_iterator it = queue->items.begin();
        it != queue->items.end();
@@ -283,15 +287,22 @@ OneCopyRasterWorkerPool::PlaybackAndScheduleCopyOnWorkerThread(
   {
     base::AutoLock lock(lock_);
 
+    int failed_attempts = 0;
     while ((scheduled_copy_operation_count_ + issued_copy_operation_count_) >=
            kMaxCopyOperations) {
       // Ignore limit when shutdown is set.
       if (shutdown_)
         break;
 
+      ++failed_attempts;
+
+      // Schedule a check that will also wait for operations to complete
+      // after too many failed attempts.
+      bool wait_if_needed = failed_attempts > kFailedAttemptsBeforeWaitIfNeeded;
+
       // Schedule a check for completed copy operations if too many operations
       // are currently in-flight.
-      ScheduleCheckForCompletedCopyOperationsWithLockAcquired();
+      ScheduleCheckForCompletedCopyOperationsWithLockAcquired(wait_if_needed);
 
       {
         TRACE_EVENT0("cc", "WaitingForCopyOperationsToComplete");
@@ -420,7 +431,8 @@ void OneCopyRasterWorkerPool::IssueCopyOperations(int64 count) {
 }
 
 void OneCopyRasterWorkerPool::
-    ScheduleCheckForCompletedCopyOperationsWithLockAcquired() {
+    ScheduleCheckForCompletedCopyOperationsWithLockAcquired(
+        bool wait_if_needed) {
   lock_.AssertAcquired();
 
   if (check_for_completed_copy_operations_pending_)
@@ -440,7 +452,8 @@ void OneCopyRasterWorkerPool::
   task_runner_->PostDelayedTask(
       FROM_HERE,
       base::Bind(&OneCopyRasterWorkerPool::CheckForCompletedCopyOperations,
-                 weak_ptr_factory_.GetWeakPtr()),
+                 weak_ptr_factory_.GetWeakPtr(),
+                 wait_if_needed),
       next_check_for_completed_copy_operations_time - now);
 
   last_check_for_completed_copy_operations_time_ =
@@ -448,11 +461,14 @@ void OneCopyRasterWorkerPool::
   check_for_completed_copy_operations_pending_ = true;
 }
 
-void OneCopyRasterWorkerPool::CheckForCompletedCopyOperations() {
-  TRACE_EVENT0("cc",
-               "OneCopyRasterWorkerPool::CheckForCompletedCopyOperations");
+void OneCopyRasterWorkerPool::CheckForCompletedCopyOperations(
+    bool wait_if_needed) {
+  TRACE_EVENT1("cc",
+               "OneCopyRasterWorkerPool::CheckForCompletedCopyOperations",
+               "wait_if_needed",
+               wait_if_needed);
 
-  resource_pool_->CheckBusyResources();
+  resource_pool_->CheckBusyResources(wait_if_needed);
 
   {
     base::AutoLock lock(lock_);
diff --git a/cc/resources/one_copy_raster_worker_pool.h b/cc/resources/one_copy_raster_worker_pool.h
index 59c4236..1dc102f 100644
--- a/cc/resources/one_copy_raster_worker_pool.h
+++ b/cc/resources/one_copy_raster_worker_pool.h
@@ -94,8 +94,9 @@ class CC_EXPORT OneCopyRasterWorkerPool : public RasterWorkerPool,
   void OnRasterFinished(TaskSet task_set);
   void AdvanceLastFlushedCopyTo(CopySequenceNumber sequence);
   void IssueCopyOperations(int64 count);
-  void ScheduleCheckForCompletedCopyOperationsWithLockAcquired();
-  void CheckForCompletedCopyOperations();
+  void ScheduleCheckForCompletedCopyOperationsWithLockAcquired(
+      bool wait_if_needed);
+  void CheckForCompletedCopyOperations(bool wait_if_needed);
   scoped_refptr<base::debug::ConvertableToTraceFormat> StateAsValue() const;
   void StagingStateAsValueInto(base::debug::TracedValue* staging_state) const;
 
diff --git a/cc/resources/resource_pool.cc b/cc/resources/resource_pool.cc
index 31b9b27..45f36e2 100644
--- a/cc/resources/resource_pool.cc
+++ b/cc/resources/resource_pool.cc
@@ -105,12 +105,15 @@ bool ResourcePool::ResourceUsageTooHigh() {
   return false;
 }
 
-void ResourcePool::CheckBusyResources() {
+void ResourcePool::CheckBusyResources(bool wait_if_needed) {
   ResourceList::iterator it = busy_resources_.begin();
 
   while (it != busy_resources_.end()) {
     ScopedResource* resource = *it;
 
+    if (wait_if_needed)
+      resource_provider_->WaitReadLockIfNeeded(resource->id());
+
     if (resource_provider_->CanLockForWrite(resource->id())) {
       DidFinishUsingResource(resource);
       it = busy_resources_.erase(it);
diff --git a/cc/resources/resource_pool.h b/cc/resources/resource_pool.h
index 102b242..e1ee35a 100644
--- a/cc/resources/resource_pool.h
+++ b/cc/resources/resource_pool.h
@@ -34,7 +34,10 @@ class CC_EXPORT ResourcePool {
                               size_t max_resource_count);
 
   void ReduceResourceUsage();
-  void CheckBusyResources();
+  // This might block if |wait_if_needed| is true and one of the currently
+  // busy resources has a read lock fence that needs to be waited upon before
+  // it can be locked for write again.
+  void CheckBusyResources(bool wait_if_needed);
 
   size_t total_memory_usage_bytes() const { return memory_usage_bytes_; }
   size_t acquired_memory_usage_bytes() const {
diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
index 86874e7..c785cf6 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -541,7 +541,7 @@ void TileManager::AssignGpuMemoryToTiles(
   // If this operation becomes expensive too, only do this after some
   // resource(s) was returned. Note that in that case, one also need to
   // invalidate when releasing some resource from the pool.
-  resource_pool_->CheckBusyResources();
+  resource_pool_->CheckBusyResources(false);
 
   // Now give memory out to the tiles until we're out, and build
   // the needs-to-be-rasterized queue.