Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2072)

Unified Diff: cc/resources/pixel_buffer_raster_worker_pool.cc

Issue 18614012: cc: Fix incorrect completion of tiles and missing "ready to activate" signal. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: vmpstr's review Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | cc/resources/raster_worker_pool.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: cc/resources/pixel_buffer_raster_worker_pool.cc
diff --git a/cc/resources/pixel_buffer_raster_worker_pool.cc b/cc/resources/pixel_buffer_raster_worker_pool.cc
index 574065e23f59e88ab8599b885e43f1568ddc840b..a961d3b631afbac36eb18b1dac77ee37e30813a9 100644
--- a/cc/resources/pixel_buffer_raster_worker_pool.cc
+++ b/cc/resources/pixel_buffer_raster_worker_pool.cc
@@ -97,6 +97,11 @@ void AddDependenciesToGraphNode(
}
}
+// Only used as std::find_if predicate for DCHECKs.
+bool WasCanceled(const internal::RasterWorkerPoolTask* task) {
+ return task->WasCanceled();
+}
+
} // namespace
PixelBufferRasterWorkerPool::PixelBufferRasterWorkerPool(
@@ -200,6 +205,14 @@ void PixelBufferRasterWorkerPool::ScheduleTasks(RasterTask::Queue* queue) {
tasks_required_for_activation_.insert(task);
}
+ // |tasks_required_for_activation_| contains all tasks that need to
+ // complete before we can send a "ready to activate" signal. Tasks
+ // that have already completed should not be part of this set.
+ for (TaskDeque::const_iterator it = completed_tasks_.begin();
+ it != completed_tasks_.end(); ++it) {
+ tasks_required_for_activation_.erase(*it);
+ }
+
pixel_buffer_tasks_.swap(new_pixel_buffer_tasks);
// Check for completed tasks when ScheduleTasks() is called as
@@ -409,10 +422,16 @@ void PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() {
ScheduleCheckForCompletedRasterTasks();
// Generate client notifications.
- if (will_notify_client_that_no_tasks_required_for_activation_are_pending)
+ if (will_notify_client_that_no_tasks_required_for_activation_are_pending) {
+ DCHECK(std::find_if(raster_tasks_required_for_activation().begin(),
+ raster_tasks_required_for_activation().end(),
+ WasCanceled) ==
+ raster_tasks_required_for_activation().end());
client()->DidFinishedRunningTasksRequiredForActivation();
+ }
if (will_notify_client_that_no_tasks_are_pending) {
TRACE_EVENT_ASYNC_END0("cc", "ScheduledTasks", this);
+ DCHECK(!HasPendingTasksRequiredForActivation());
client()->DidFinishedRunningTasks();
}
}
@@ -582,6 +601,20 @@ void PixelBufferRasterWorkerPool::OnRasterTaskCompleted(
if (!needs_upload) {
resource_provider()->ReleasePixelBuffer(task->resource()->id());
+
+ if (was_canceled) {
+ // When priorites change, a raster task can be canceled as a result of
+ // no longer being of high enough priority to fit in our throttled
+ // raster task budget. The task has not yet completed in this case.
+ RasterTaskVector::const_iterator it = std::find(raster_tasks().begin(),
+ raster_tasks().end(),
+ task);
+ if (it != raster_tasks().end()) {
+ pixel_buffer_tasks_[task.get()] = NULL;
+ return;
+ }
+ }
+
task->DidRun(was_canceled);
DCHECK(std::find(completed_tasks_.begin(),
completed_tasks_.end(),
@@ -591,6 +624,8 @@ void PixelBufferRasterWorkerPool::OnRasterTaskCompleted(
return;
}
+ DCHECK(!was_canceled);
vmpstr 2013/07/11 17:55:12 nit: I would also prefer DCHECK(!was_canceled || !
vmpstr 2013/07/11 18:12:55 Nevermind this, I missed the fact that the if retu
+
resource_provider()->BeginSetPixels(task->resource()->id());
has_performed_uploads_since_last_flush_ = true;
« no previous file with comments | « no previous file | cc/resources/raster_worker_pool.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698