cc: Do not cleanup tiles with raster tasks.

cc/resources/tile_manager.cc

 // Copyright 2012 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/tile_manager.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 
@@ skipping 371 matching lines @@
           base::Bind(&TileManager::CheckIfReadyToActivate,
                      base::Unretained(this))) {
   rasterizer_->SetClient(this);
 }
 
 TileManager::~TileManager() {
   // Reset global state and manage. This should cause
   // 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_);
 }
 
 void TileManager::Release(Tile* tile) {
   prioritized_tiles_dirty_ = true;
   released_tiles_.push_back(tile);
 }
 
 void TileManager::DidChangeTilePriority(Tile* tile) {
   prioritized_tiles_dirty_ = true;
 }
 
 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)
+      FreeResourceForTile(tile, static_cast<RasterMode>(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());
 
     LayerCountMap::iterator layer_it =
         used_layer_counts_.find(tile->layer_id());
     DCHECK_GT(layer_it->second, 0);
     if (--layer_it->second == 0) {
       used_layer_counts_.erase(layer_it);
       image_decode_tasks_.erase(tile->layer_id());
     }
 
     delete tile;
+    it = released_tiles_.erase(it);
   }
-
-  released_tiles_.clear();
 }
 
 void TileManager::UpdatePrioritizedTileSetIfNeeded() {
   if (!prioritized_tiles_dirty_)
     return;
 
+  FreeResourcesForReleasedTiles();
   CleanUpReleasedTiles();
 
   prioritized_tiles_.Clear();
   GetTilesWithAssignedBins(&prioritized_tiles_);
   prioritized_tiles_dirty_ = false;
 }
 
 void TileManager::DidFinishRunningTasks() {
   TRACE_EVENT0("cc", "TileManager::DidFinishRunningTasks");
 
@@ skipping 13 matching lines @@
   AssignGpuMemoryToTiles(&prioritized_tiles_,
                          &tiles_that_need_to_be_rasterized);
 
   // |tiles_that_need_to_be_rasterized| will be empty when we reach a
   // steady memory state. Keep scheduling tasks until we reach this state.
   if (!tiles_that_need_to_be_rasterized.empty()) {
     ScheduleTasks(tiles_that_need_to_be_rasterized);
     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
   // accelerated gestures, so we just postpone activation when we don't
   // have these tiles, and activate after the accelerated gesture.
   bool allow_rasterize_on_demand =
       global_state_.tree_priority != SMOOTHNESS_TAKES_PRIORITY;
 
   // Use on-demand raster for any required-for-activation tiles that have not
   // been been assigned memory after reaching a steady memory state. This
@@ skipping 467 matching lines @@
 
   // We must reduce the amount of unused resoruces before calling
   // ScheduleTasks to prevent usage from rising above limits.
   resource_pool_->ReduceResourceUsage();
 
   // Schedule running of |raster_tasks_|. This replaces any previously
   // scheduled tasks and effectively cancels all tasks not present
   // 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;
 }
 
 scoped_refptr<ImageDecodeTask> TileManager::CreateImageDecodeTask(
     Tile* tile,
     SkPixelRef* pixel_ref) {
   return make_scoped_refptr(new ImageDecodeTaskImpl(
       pixel_ref,
       tile->layer_id(),
       rendering_stats_instrumentation_,
@@ skipping 74 matching lines @@
     pixel_ref_tasks.erase(task_it);
 }
 
 void TileManager::OnRasterTaskCompleted(
     Tile::Id tile_id,
     scoped_ptr<ScopedResource> resource,
     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(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) {
     ++update_visible_tiles_stats_.canceled_count;
     resource_pool_->ReleaseResource(resource.Pass());
     return;
   }
 
   ++update_visible_tiles_stats_.completed_count;
 
@@ skipping 482 matching lines @@
 void TileManager::CheckIfReadyToActivate() {
   TRACE_EVENT0("cc", "TileManager::CheckIfReadyToActivate");
 
   rasterizer_->CheckForCompletedTasks();
   did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
 
   if (IsReadyToActivate())
     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