cc: Clean up PBRWP::ScheduleTasks a bit.

cc/resources/pixel_buffer_raster_worker_pool.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/pixel_buffer_raster_worker_pool.h"
 
 #include "base/containers/stack_container.h"
 #include "base/debug/trace_event.h"
 #include "base/values.h"
 #include "cc/debug/traced_value.h"
@@ skipping 130 matching lines @@
   TRACE_EVENT0("cc", "PixelBufferRasterWorkerPool::ScheduleTasks");
 
   RasterWorkerPool::SetRasterTasks(queue);
 
   if (!should_notify_client_if_no_tasks_are_pending_)
     TRACE_EVENT_ASYNC_BEGIN0("cc", "ScheduledTasks", this);
 
   should_notify_client_if_no_tasks_are_pending_ = true;
   should_notify_client_if_no_tasks_required_for_activation_are_pending_ = true;
 
+  tasks_required_for_activation_.clear();
+
   // Build new pixel buffer task set.
   TaskMap new_pixel_buffer_tasks;
   for (RasterTaskVector::const_iterator it = raster_tasks().begin();
        it != raster_tasks().end(); ++it) {
     internal::RasterWorkerPoolTask* task = it->get();
     DCHECK(new_pixel_buffer_tasks.find(task) == new_pixel_buffer_tasks.end());
     DCHECK(!task->HasCompleted());
 
-    // Use existing pixel buffer task if available.
-    TaskMap::iterator pixel_buffer_it = pixel_buffer_tasks_.find(task);
-    if (pixel_buffer_it == pixel_buffer_tasks_.end()) {
-      new_pixel_buffer_tasks[task] = NULL;
-      continue;
-    }
+    new_pixel_buffer_tasks[task] = pixel_buffer_tasks_[task];
+    pixel_buffer_tasks_.erase(task);
 
-    new_pixel_buffer_tasks[task] = pixel_buffer_it->second;
-    pixel_buffer_tasks_.erase(task);
+    if (IsRasterTaskRequiredForActivation(task))
+      tasks_required_for_activation_.insert(task);
   }
 
   // Transfer remaining pixel buffer tasks to |new_pixel_buffer_tasks|
   // and cancel all remaining inactive tasks.
   for (TaskMap::iterator it = pixel_buffer_tasks_.begin();
        it != pixel_buffer_tasks_.end(); ++it) {
     internal::RasterWorkerPoolTask* task = it->first;
     internal::WorkerPoolTask* pixel_buffer_task = it->second.get();
 
     // Move task to |new_pixel_buffer_tasks|
     new_pixel_buffer_tasks[task] = pixel_buffer_task;
 
     // Inactive task can be canceled.
     if (!pixel_buffer_task && !task->HasFinishedRunning()) {
       task->DidRun(true);
       DCHECK(std::find(completed_tasks_.begin(),
                        completed_tasks_.end(),
                        task) == completed_tasks_.end());
       completed_tasks_.push_back(task);
+    } else if (IsRasterTaskRequiredForActivation(task)) {
+      tasks_required_for_activation_.insert(task);
     }
   }
 
-  tasks_required_for_activation_.clear();
-  for (TaskMap::iterator it = new_pixel_buffer_tasks.begin();
-       it != new_pixel_buffer_tasks.end(); ++it) {
-    internal::RasterWorkerPoolTask* task = it->first;
-    if (IsRasterTaskRequiredForActivation(task))
-      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);
+  if (!tasks_required_for_activation_.empty()) {

[reveman, 2013/09/13 16:51:46]

this is just an optimization, right? can you instead break out of the loop below
as soon as |tasks_required_for_activation_| is empty?

[vmpstr, 2013/09/16 16:09:16]

Done. I did an explicit if/break, but it could also be a part of the for loop
condition. Let me know if you prefer that instead.

+    // |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
   // priorities might have changed and this maximizes the number
   // of top priority tasks that are scheduled.
   RasterWorkerPool::CheckForCompletedTasks();
   CheckForCompletedUploads();
   FlushUploads();
@@ skipping 459 matching lines @@
 
   throttle_state->SetInteger("bytes_available_for_upload",
                              max_bytes_pending_upload_ - bytes_pending_upload_);
   throttle_state->SetInteger("bytes_pending_upload", bytes_pending_upload_);
   throttle_state->SetInteger("scheduled_raster_task_count",
                              scheduled_raster_task_count_);
   return throttle_state.PassAs<base::Value>();
 }
 
 }  // namespace cc