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..67ca3cdb9d8d5ac922b587ca57d17d285393ae1e 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,15 @@ TileManager::~TileManager() {
   rasterizer_->Shutdown();
   rasterizer_->CheckForCompletedTasks();
 
+  FreeResourcesForReleasedTiles();
+
   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 +419,29 @@ bool TileManager::ShouldForceTasksRequiredForActivationToComplete() const {
   return global_state_.tree_priority != SMOOTHNESS_TAKES_PRIORITY;
 }
 
-void TileManager::CleanUpReleasedTiles() {
-  for (std::vector<Tile*>::iterator it = released_tiles_.begin();
+void TileManager::FreeResourcesForReleasedTiles() {
+  for (ScopedVector<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_);

[reveman, 2014/07/28 02:24:35]

Let's change this to DCHECK(prioritized_tiles_.Empty()) to be safe. I think
that's also easier to understand. And the comment can say "Make sure
|prioritized_tiles_| doesn't contain any of the tiles we're about to delete."

[sohanjg, 2014/07/28 10:10:19]

Are you sure we want this, because if we check for empty - if
(!tiles_[bin].empty()) return false..
there are test failures.
Wont UpdatePrioritizedTileSetIfNeeded, clear it right after we do
CleanUpReleasedTiles ? why do we need the check ? now that we have avoided the
path from dtor.

[reveman, 2014/07/28 15:50:53]

I want the check to prevent this from being broken in the future. If someone
decides to call this function from somewhere else, things could go very bad.
Please figure out why the tests are failing and change this.

[sohanjg, 2014/07/28 17:36:02]

OK.
The test fails when this is invoked from UpdatePrioritizedTileSetIfNeeded, there
are cases when we have prioritized_tiles_ as non empty when we make this call.
The thing is we do clear prioritized_tiles_ right after this, so do you want us
to move the clearing call before calling CleanUpReleasedTiles ? 
We still would protect this being called from somewhere else as you pointed.

[reveman, 2014/07/28 18:27:51]

yes, it's of course not going to work otherwise.
 

[sohanjg, 2014/07/30 11:12:56]

Done.

+
+  ScopedVector<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 +454,15 @@ void TileManager::CleanUpReleasedTiles() {
     }
 
     delete tile;
+    it = released_tiles_.weak_erase(it);

[reveman, 2014/07/28 02:24:35]

remove "delete tile" and use standard erase instead

[sohanjg, 2014/07/28 10:10:19]

Done.

   }
-
-  released_tiles_.clear();
 }
 
 void TileManager::UpdatePrioritizedTileSetIfNeeded() {
   if (!prioritized_tiles_dirty_)
     return;
 
+  FreeResourcesForReleasedTiles();
   CleanUpReleasedTiles();
 
   prioritized_tiles_.Clear();
@@ -484,6 +496,8 @@ void TileManager::DidFinishRunningTasks() {
     return;
   }
 
+  FreeResourcesForReleasedTiles();
+
   resource_pool_->ReduceResourceUsage();
 
   // We don't reserve memory for required-for-activation tiles during
@@ -1071,14 +1085,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_);