cc: Do not cleanup tiles with raster tasks.

cc/resources/tile_manager.cc

Index: cc/resources/tile_manager.cc
diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
index aff534bcbb482984fbbe9af268e41263230aac49..dc571090f454c75244610817724b279dc8d8dcc4 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -389,18 +389,18 @@ TileManager::~TileManager() {
   // our memory usage to drop to zero.
   global_state_ = GlobalStateThatImpactsTilePriority();
 
-  CleanUpReleasedTiles();
-  DCHECK_EQ(0u, tiles_.size());
-
   RasterTaskQueue empty;
   rasterizer_->ScheduleTasks(&empty);
-  orphan_raster_tasks_.clear();
 
   // This should finish all pending tasks and release any uninitialized
   // resources.
   rasterizer_->Shutdown();
   rasterizer_->CheckForCompletedTasks();
 
+  FreeResourcesForReleasedTiles();
+  CleanUpReleasedTiles();
+  DCHECK_EQ(0u, tiles_.size());
+
   DCHECK_EQ(0u, bytes_releasable_);
   DCHECK_EQ(0u, resources_releasable_);
 }
@@ -418,18 +418,27 @@ bool TileManager::ShouldForceTasksRequiredForActivationToComplete() const {
   return global_state_.tree_priority != SMOOTHNESS_TAKES_PRIORITY;
 }
 
-void TileManager::CleanUpReleasedTiles() {
+void TileManager::FreeResourcesForReleasedTiles() {
   for (std::vector<Tile*>::iterator it = released_tiles_.begin();
        it != released_tiles_.end();
        ++it) {
     Tile* tile = *it;
-    ManagedTileState& mts = tile->managed_state();
-
-    for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) {
+    for (int mode = 0; mode < NUM_RASTER_MODES; ++mode)
       FreeResourceForTile(tile, static_cast<RasterMode>(mode));
-      orphan_raster_tasks_.push_back(mts.tile_versions[mode].raster_task_);
+  }
+}
+
+void TileManager::CleanUpReleasedTiles() {

[reveman, 2014/07/17 15:10:50]

How about adding a DCHECK(prioritized_tiles_dirty_) here to prevent this from
being called in a way that could potentially cause a crash?

[sohanjg, 2014/07/17 15:58:33]

Done.

+  std::vector<Tile*>::iterator it = released_tiles_.begin();
+  while (it != released_tiles_.end()) {
+    Tile* tile = *it;
+
+    if (TileHasRasterTask(tile)) {
+      ++it;
+      continue;
     }
 
+    DCHECK(!tile->HasResources());
     DCHECK(tiles_.find(tile->id()) != tiles_.end());
     tiles_.erase(tile->id());
 
@@ -442,15 +451,15 @@ void TileManager::CleanUpReleasedTiles() {
     }
 
     delete tile;
+    it = released_tiles_.erase(it);
   }
-
-  released_tiles_.clear();
 }
 
 void TileManager::UpdatePrioritizedTileSetIfNeeded() {
   if (!prioritized_tiles_dirty_)
     return;
 
+  FreeResourcesForReleasedTiles();
   CleanUpReleasedTiles();
 
   prioritized_tiles_.Clear();
@@ -484,6 +493,8 @@ void TileManager::DidFinishRunningTasks() {
     return;
   }
 
+  FreeResourcesForReleasedTiles();
+  CleanUpReleasedTiles();

[reveman, 2014/07/17 15:10:50]

I don't think you should do the cleanup here as that leaves |prioritized_tiles_|
invalid and there's currently no guarantee that it will be rebuilt before being
used again afaict.

Just calling FreeResourcesForReleasedTiles() here should be enough though.
Cleaning up the actual instances can wait until
UpdatePrioritizedTileSetIfNeeded() or the dtor is called.

[sohanjg, 2014/07/17 15:58:33]

Done.

   resource_pool_->ReduceResourceUsage();
 
   // We don't reserve memory for required-for-activation tiles during
@@ -971,11 +982,6 @@ void TileManager::ScheduleTasks(
   // in |raster_tasks_|.
   rasterizer_->ScheduleTasks(&raster_queue_);
 
-  // It's now safe to clean up orphan tasks as raster worker pool is not
-  // allowed to keep around unreferenced raster tasks after ScheduleTasks() has
-  // been called.
-  orphan_raster_tasks_.clear();
-
   did_check_for_completed_tasks_since_last_schedule_tasks_ = false;
 }
 
@@ -1070,17 +1076,12 @@ void TileManager::OnRasterTaskCompleted(
     const PicturePileImpl::Analysis& analysis,
     bool was_canceled) {
   TileMap::iterator it = tiles_.find(tile_id);
-  if (it == tiles_.end()) {
-    ++update_visible_tiles_stats_.canceled_count;
-    resource_pool_->ReleaseResource(resource.Pass());
-    return;
-  }
+  DCHECK(it != tiles_.end());
 
   Tile* tile = it->second;
   ManagedTileState& mts = tile->managed_state();
   ManagedTileState::TileVersion& tile_version = mts.tile_versions[raster_mode];
   DCHECK(tile_version.raster_task_);
-  orphan_raster_tasks_.push_back(tile_version.raster_task_);
   tile_version.raster_task_ = NULL;
 
   if (was_canceled) {
@@ -1583,4 +1584,14 @@ void TileManager::CheckIfReadyToActivate() {
     client_->NotifyReadyToActivate();
 }
 
+// static
+bool TileManager::TileHasRasterTask(const Tile* tile) {
+  const ManagedTileState& mts = tile->managed_state();
+  for (int mode = 0; mode < NUM_RASTER_MODES; ++mode) {
+    if (mts.tile_versions[mode].raster_task_)
+      return true;
+  }
+  return false;
+}
+
 }  // namespace cc