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 dab3f5562823f5eadbdac2e7d7488726dee14f14..f7ead20b5bdaa82d68f4f356750a538d24903bcc 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -389,9 +389,6 @@ 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();
@@ -401,11 +398,16 @@ TileManager::~TileManager() {
   rasterizer_->Shutdown();
   rasterizer_->CheckForCompletedTasks();
 
+  FreeResourcesForReleasedTiles();
+  released_tiles_.clear();

[vmpstr, 2014/07/25 16:29:25]

You should call CleanUp... here instead and dcheck that release_tiles_.empty().
Reason being is that released tiles are raw pointers and would leak otherwise.

[reveman, 2014/07/25 17:00:55]

The problem is that we can't have the DCHECK(prioritized_tiles_dirty_) check in
::CleanUpReleasedTiles() without awkwardly setting prioritized_tiles_dirty_ to
true here before the call.

released_tiles_.clear() is obviously wrong as it will leak the pointers but if
released_tiles_ was a ScopedVector instead I think all would be good as we don't
actually have to do all the work CleanUpReleasedTiles() does here. I think
making released_tiles_ a ScopedVector is good idea in general.

[sohanjg, 2014/07/26 09:36:01]

Done.

+
   DCHECK_EQ(0u, bytes_releasable_);
   DCHECK_EQ(0u, resources_releasable_);
 }
 
 void TileManager::Release(Tile* tile) {
+  DCHECK(TilePriority() == tile->combined_priority());
+
   prioritized_tiles_dirty_ = true;
   released_tiles_.push_back(tile);
 }
@@ -418,18 +420,29 @@ 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();
+    FreeResourcesForTile(tile);
+  }
+}
 
-    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() {
+  // Ensure the prioritized set is rebuild.
+  DCHECK(prioritized_tiles_dirty_);
+
+  std::vector<Tile*>::iterator it = released_tiles_.begin();
+  while (it != released_tiles_.end()) {
+    Tile* tile = *it;
+
+    if (tile->HasRasterTask()) {
+      ++it;
+      continue;
     }
 
+    DCHECK(!tile->HasResources());
     DCHECK(tiles_.find(tile->id()) != tiles_.end());
     tiles_.erase(tile->id());
 
@@ -442,15 +455,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 +497,8 @@ void TileManager::DidFinishRunningTasks() {
     return;
   }
 
+  FreeResourcesForReleasedTiles();
+
   resource_pool_->ReduceResourceUsage();
 
   // We don't reserve memory for required-for-activation tiles during
@@ -1071,14 +1086,9 @@ void TileManager::OnRasterTaskCompleted(
     RasterMode raster_mode,
     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(tiles_.find(tile_id) != tiles_.end());
 
-  Tile* tile = it->second;
+  Tile* tile = tiles_[tile_id];
   ManagedTileState& mts = tile->managed_state();
   ManagedTileState::TileVersion& tile_version = mts.tile_versions[raster_mode];
   DCHECK(tile_version.raster_task_);