Clean up tile deletion

cc/tiles/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/tiles/tile_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 75 matching lines @@
                  scoped_refptr<RasterSource> raster_source,
                  const RasterSource::PlaybackSettings& playback_settings,
                  TileResolution tile_resolution,
                  gfx::Rect invalidated_rect,
                  uint64_t source_prepare_tiles_id,
                  std::unique_ptr<RasterBuffer> raster_buffer,
                  TileTask::Vector* dependencies,
                  bool is_gpu_rasterization)
       : TileTask(!is_gpu_rasterization, dependencies),
         tile_manager_(tile_manager),
-        tile_(tile),
+        tile_id_(tile->id()),
         resource_(resource),
         raster_source_(std::move(raster_source)),
         content_rect_(tile->content_rect()),
         invalid_content_rect_(invalidated_rect),
         raster_scales_(tile->raster_scales()),
         playback_settings_(playback_settings),
         tile_resolution_(tile_resolution),
         layer_id_(tile->layer_id()),
         source_prepare_tiles_id_(source_prepare_tiles_id),
         tile_tracing_id_(static_cast<void*>(tile)),
@@ skipping 24 matching lines @@
                              raster_scales_, playback_settings_);
   }
 
   // Overridden from TileTask:
   void OnTaskCompleted() override {
     DCHECK(origin_thread_checker_.CalledOnValidThread());
 
     // Here calling state().IsCanceled() is thread-safe, because this task is
     // already concluded as FINISHED or CANCELLED and no longer will be worked
     // upon by task graph runner.
-    tile_manager_->OnRasterTaskCompleted(std::move(raster_buffer_), tile_,
+    tile_manager_->OnRasterTaskCompleted(std::move(raster_buffer_), tile_id_,
                                          resource_, state().IsCanceled());
   }
 
  protected:
   ~RasterTaskImpl() override {
     DCHECK(origin_thread_checker_.CalledOnValidThread());
     DCHECK(!raster_buffer_);
   }
 
  private:
   base::ThreadChecker origin_thread_checker_;
 
   // The following members are needed for processing completion of this task on
   // origin thread. These are not thread-safe and should be accessed only in
   // origin thread. Ensure their access by checking CalledOnValidThread().
   TileManager* tile_manager_;
-  Tile* tile_;
+  Tile::Id tile_id_;
   Resource* resource_;
 
   // The following members should be used for running the task.
   scoped_refptr<RasterSource> raster_source_;
   gfx::Rect content_rect_;
   gfx::Rect invalid_content_rect_;
   gfx::SizeF raster_scales_;
   RasterSource::PlaybackSettings playback_settings_;
   TileResolution tile_resolution_;
   int layer_id_;
@@ skipping 212 matching lines @@
   global_state_ = GlobalStateThatImpactsTilePriority();
 
   // This cancels tasks if possible, finishes pending tasks, and release any
   // uninitialized resources.
   tile_task_manager_->Shutdown();
 
   raster_buffer_provider_->Shutdown();
 
   tile_task_manager_->CheckForCompletedTasks();
 
-  FreeResourcesForReleasedTiles();
-  CleanUpReleasedTiles();
-
   tile_task_manager_ = nullptr;
   resource_pool_ = nullptr;
   more_tiles_need_prepare_check_notifier_.Cancel();
   signals_check_notifier_.Cancel();
   task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
   raster_buffer_provider_ = nullptr;
 
   image_controller_.SetImageDecodeCache(nullptr);
   locked_image_tasks_.clear();
 }
 
 void TileManager::SetResources(ResourcePool* resource_pool,
                                ImageDecodeCache* image_decode_cache,
                                TaskGraphRunner* task_graph_runner,
                                RasterBufferProvider* raster_buffer_provider,
                                size_t scheduled_raster_task_limit,
                                bool use_gpu_rasterization) {
   DCHECK(!tile_task_manager_);
   DCHECK(task_graph_runner);
 
   use_gpu_rasterization_ = use_gpu_rasterization;
   scheduled_raster_task_limit_ = scheduled_raster_task_limit;
   resource_pool_ = resource_pool;
   image_controller_.SetImageDecodeCache(image_decode_cache);
   tile_task_manager_ = TileTaskManagerImpl::Create(task_graph_runner);
   raster_buffer_provider_ = raster_buffer_provider;
 }
 
 void TileManager::Release(Tile* tile) {
-  released_tiles_.push_back(tile);
-}
-
-void TileManager::FreeResourcesForReleasedTiles() {
-  for (auto* tile : released_tiles_)
-    FreeResourcesForTile(tile);
-}
-
-void TileManager::CleanUpReleasedTiles() {
-  std::vector<Tile*> tiles_to_retain;
-  for (auto* tile : released_tiles_) {
-    if (tile->HasRasterTask()) {
-      tiles_to_retain.push_back(tile);
-      continue;
-    }
-
-    DCHECK(!tile->draw_info().has_resource());
-    DCHECK(tiles_.find(tile->id()) != tiles_.end());
-    tiles_.erase(tile->id());
-
-    delete tile;
-  }
-  released_tiles_.swap(tiles_to_retain);
+  FreeResourcesForTile(tile);
+  tiles_.erase(tile->id());
 }
 
 void TileManager::DidFinishRunningTileTasksRequiredForActivation() {
   TRACE_EVENT0("cc",
                "TileManager::DidFinishRunningTileTasksRequiredForActivation");
   TRACE_EVENT_ASYNC_STEP_INTO1("cc", "ScheduledTasks", this, "running", "state",
                                ScheduledTasksStateAsValue());
   // TODO(vmpstr): Temporary check to debug crbug.com/642927.
   CHECK(tile_task_manager_);
   signals_.ready_to_activate = true;
@@ skipping 49 matching lines @@
   signals_.reset();
   global_state_ = state;
 
   // We need to call CheckForCompletedTasks() once in-between each call
   // to ScheduleTasks() to prevent canceled tasks from being scheduled.
   if (!did_check_for_completed_tasks_since_last_schedule_tasks_) {
     tile_task_manager_->CheckForCompletedTasks();
     did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
   }
 
-  FreeResourcesForReleasedTiles();
-  CleanUpReleasedTiles();
-
   PrioritizedWorkToSchedule prioritized_work = AssignGpuMemoryToTiles();
 
   // Inform the client that will likely require a draw if the highest priority
   // tile that will be rasterized is required for draw.
   client_->SetIsLikelyToRequireADraw(
       !prioritized_work.tiles_to_raster.empty() &&
       prioritized_work.tiles_to_raster.front().tile()->required_for_draw());
 
   // Schedule tile tasks.
   ScheduleTasks(prioritized_work);
@@ skipping 514 matching lines @@
           resource, resource_content_id, tile->invalidated_id());
   return make_scoped_refptr(new RasterTaskImpl(
       this, tile, resource, prioritized_tile.raster_source(), playback_settings,
       prioritized_tile.priority().resolution, invalidated_rect,
       prepare_tiles_count_, std::move(raster_buffer), &decode_tasks,
       use_gpu_rasterization_));
 }
 
 void TileManager::OnRasterTaskCompleted(
     std::unique_ptr<RasterBuffer> raster_buffer,
-    Tile* tile,
+    Tile::Id tile_id,
     Resource* resource,
     bool was_canceled) {
-  DCHECK(tile);
-  DCHECK(tiles_.find(tile->id()) != tiles_.end());
   raster_buffer_provider_->ReleaseBufferForRaster(std::move(raster_buffer));
 
-  TileDrawInfo& draw_info = tile->draw_info();
-  DCHECK(tile->raster_task_.get());
-  tile->raster_task_ = nullptr;
+  auto found = tiles_.find(tile_id);
+  Tile* tile = found != tiles_.end() ? found->second : nullptr;

[vmpstr, 2017/01/09 20:02:09]

nit: can you structure it with one less conditional like:

Tile* tile = nullptr;
if (found != tiles_.end()) {
  tile = found->second;
  DCHECK(...);
  tile->raster_task_ = nullptr;
}

[ericrk, 2017/01/09 21:35:22]

Done.

+
+  if (tile) {
+    DCHECK(tile->raster_task_.get());
+    tile->raster_task_ = nullptr;
+  }
 
   // Unref all the images.
-  auto images_it = scheduled_draw_images_.find(tile->id());
+  auto images_it = scheduled_draw_images_.find(tile_id);
   image_controller_.UnrefImages(images_it->second);
   scheduled_draw_images_.erase(images_it);
 
   if (was_canceled) {
     ++flush_stats_.canceled_count;
     resource_pool_->ReleaseResource(resource);
     return;
   }
 
-  resource_pool_->OnContentReplaced(resource->id(), tile->id());
+  resource_pool_->OnContentReplaced(resource->id(), tile_id);
   ++flush_stats_.completed_count;
 
-  draw_info.set_use_resource();
-  draw_info.resource_ = resource;
-  draw_info.contents_swizzled_ = DetermineResourceRequiresSwizzle(tile);
+  if (tile) {
+    TileDrawInfo& draw_info = tile->draw_info();
+    draw_info.set_use_resource();
+    draw_info.resource_ = resource;
+    draw_info.contents_swizzled_ = DetermineResourceRequiresSwizzle(tile);
 
-  DCHECK(draw_info.IsReadyToDraw());
-  draw_info.set_was_ever_ready_to_draw();
-
-  client_->NotifyTileStateChanged(tile);
+    DCHECK(draw_info.IsReadyToDraw());
+    draw_info.set_was_ever_ready_to_draw();
+    client_->NotifyTileStateChanged(tile);
+  }
 }
 
-ScopedTilePtr TileManager::CreateTile(const Tile::CreateInfo& info,
-                                      int layer_id,
-                                      int source_frame_number,
-                                      int flags) {
+std::unique_ptr<Tile> TileManager::CreateTile(const Tile::CreateInfo& info,
+                                              int layer_id,
+                                              int source_frame_number,
+                                              int flags) {
   // We need to have a tile task worker pool to do anything meaningful with
   // tiles.
   DCHECK(tile_task_manager_);
-  ScopedTilePtr tile(
+  std::unique_ptr<Tile> tile(

[vmpstr, 2017/01/09 20:02:09]

nit: can you make this

auto tile = base::MakeUnique<Tile>(...);

[ericrk, 2017/01/09 21:35:22]

Constructor is private, which stops this from working :(

       new Tile(this, info, layer_id, source_frame_number, flags));
   DCHECK(tiles_.find(tile->id()) == tiles_.end());
 
   tiles_[tile->id()] = tile.get();
   return tile;
 }
 
 bool TileManager::AreRequiredTilesReadyToDraw(
     RasterTilePriorityQueue::Type type) const {
   std::unique_ptr<RasterTilePriorityQueue> raster_priority_queue(
@@ skipping 98 matching lines @@
   }
 
   // If we're not in SMOOTHNESS_TAKES_PRIORITY  mode, we should unlock all
   // images since we're technically going idle here at least for this frame.
   if (global_state_.tree_priority != SMOOTHNESS_TAKES_PRIORITY) {
     image_controller_.SetPredecodeImages(std::vector<DrawImage>(),
                                          ImageDecodeCache::TracingInfo());
     locked_image_tasks_.clear();
   }
 
-  FreeResourcesForReleasedTiles();
-
   resource_pool_->ReduceResourceUsage();
   image_controller_.ReduceMemoryUsage();
 
   // TODO(vmpstr): Temporary check to debug crbug.com/642927.
   CHECK(tile_task_manager_);
   signals_.all_tile_tasks_completed = true;
   signals_check_notifier_.Schedule();
 
   // We don't reserve memory for required-for-activation tiles during
   // accelerated gestures, so we just postpone activation when we don't
@@ skipping 157 matching lines @@
   all_tile_tasks_completed = false;
   did_notify_all_tile_tasks_completed = false;
 }
 
 TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule() = default;
 TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule(
     PrioritizedWorkToSchedule&& other) = default;
 TileManager::PrioritizedWorkToSchedule::~PrioritizedWorkToSchedule() = default;
 
 }  // namespace cc