cc: Move (partially) the predecode image tracking to ImageManager.
This patch moves some of the predecode image tracking to ImageManager.
The other part depends on crbug.com/647402. Once the TileTaskRunner
can keep a ref on scheduled tiles, then we can remove
TileManager::locked_image_tasks_ as well.
R=enne
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
Review-Url: https://codereview.chromium.org/2345833002
Cr-Commit-Position: refs/heads/master@{#419539}
diff --git a/cc/BUILD.gn b/cc/BUILD.gn
index 4e9df4ab..9708cb7 100644
--- a/cc/BUILD.gn
+++ b/cc/BUILD.gn
@@ -916,6 +916,7 @@
"test/ordered_simple_task_runner_unittest.cc",
"test/test_web_graphics_context_3d_unittest.cc",
"tiles/gpu_image_decode_controller_unittest.cc",
+ "tiles/image_manager_unittest.cc",
"tiles/mipmap_util_unittest.cc",
"tiles/picture_layer_tiling_set_unittest.cc",
"tiles/picture_layer_tiling_unittest.cc",
diff --git a/cc/tiles/image_manager.cc b/cc/tiles/image_manager.cc
index cb11e9ec..bec8ca20 100644
--- a/cc/tiles/image_manager.cc
+++ b/cc/tiles/image_manager.cc
@@ -10,6 +10,14 @@
ImageManager::~ImageManager() = default;
void ImageManager::SetImageDecodeController(ImageDecodeController* controller) {
+ // We can only switch from null to non-null and back.
+ DCHECK(controller || controller_);
+ DCHECK(!controller || !controller_);
+
+ if (!controller) {
+ SetPredecodeImages(std::vector<DrawImage>(),
+ ImageDecodeController::TracingInfo());
+ }
controller_ = controller;
}
@@ -43,4 +51,14 @@
controller_->ReduceCacheUsage();
}
+std::vector<scoped_refptr<TileTask>> ImageManager::SetPredecodeImages(
+ std::vector<DrawImage> images,
+ const ImageDecodeController::TracingInfo& tracing_info) {
+ std::vector<scoped_refptr<TileTask>> new_tasks;
+ GetTasksForImagesAndRef(&images, &new_tasks, tracing_info);
+ UnrefImages(predecode_locked_images_);
+ predecode_locked_images_ = std::move(images);
+ return new_tasks;
+}
+
} // namespace cc
diff --git a/cc/tiles/image_manager.h b/cc/tiles/image_manager.h
index cc6c64f..046e0948 100644
--- a/cc/tiles/image_manager.h
+++ b/cc/tiles/image_manager.h
@@ -28,9 +28,13 @@
const ImageDecodeController::TracingInfo& tracing_info);
void UnrefImages(const std::vector<DrawImage>& images);
void ReduceMemoryUsage();
+ std::vector<scoped_refptr<TileTask>> SetPredecodeImages(
+ std::vector<DrawImage> predecode_images,
+ const ImageDecodeController::TracingInfo& tracing_info);
private:
- ImageDecodeController* controller_;
+ ImageDecodeController* controller_ = nullptr;
+ std::vector<DrawImage> predecode_locked_images_;
DISALLOW_COPY_AND_ASSIGN(ImageManager);
};
diff --git a/cc/tiles/image_manager_unittest.cc b/cc/tiles/image_manager_unittest.cc
new file mode 100644
index 0000000..3d0fc37
--- /dev/null
+++ b/cc/tiles/image_manager_unittest.cc
@@ -0,0 +1,57 @@
+// Copyright 2016 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/image_decode_controller.h"
+#include "cc/tiles/image_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cc {
+
+class TestableController : public ImageDecodeController {
+ public:
+ bool GetTaskForImageAndRef(const DrawImage& image,
+ const TracingInfo& tracing_info,
+ scoped_refptr<TileTask>* task) override {
+ *task = nullptr;
+ ++number_of_refs_;
+ return true;
+ }
+
+ void UnrefImage(const DrawImage& image) override {
+ ASSERT_GT(number_of_refs_, 0);
+ --number_of_refs_;
+ }
+ DecodedDrawImage GetDecodedImageForDraw(const DrawImage& image) override {
+ return DecodedDrawImage(nullptr, kNone_SkFilterQuality);
+ }
+ void DrawWithImageFinished(const DrawImage& image,
+ const DecodedDrawImage& decoded_image) override {}
+ void ReduceCacheUsage() override {}
+ void SetShouldAggressivelyFreeResources(
+ bool aggressively_free_resources) override {}
+
+ int number_of_refs() const { return number_of_refs_; }
+
+ private:
+ int number_of_refs_ = 0;
+};
+
+TEST(ImageManagerTest, NullControllerUnrefsImages) {
+ TestableController controller;
+ ImageManager manager;
+ manager.SetImageDecodeController(&controller);
+
+ std::vector<DrawImage> images(10);
+ ImageDecodeController::TracingInfo tracing_info;
+
+ ASSERT_EQ(10u, images.size());
+ auto tasks = manager.SetPredecodeImages(std::move(images), tracing_info);
+ EXPECT_EQ(0u, tasks.size());
+ EXPECT_EQ(10, controller.number_of_refs());
+
+ manager.SetImageDecodeController(nullptr);
+ EXPECT_EQ(0, controller.number_of_refs());
+}
+
+} // namespace cc
diff --git a/cc/tiles/tile_manager.cc b/cc/tiles/tile_manager.cc
index 30ed9299..4baba72 100644
--- a/cc/tiles/tile_manager.cc
+++ b/cc/tiles/tile_manager.cc
@@ -361,8 +361,7 @@
signals_check_notifier_.Cancel();
task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
- image_manager_.UnrefImages(locked_images_);
- locked_images_.clear();
+ image_manager_.SetImageDecodeController(nullptr);
locked_image_tasks_.clear();
}
@@ -845,21 +844,25 @@
const std::vector<PrioritizedTile>& tiles_to_process_for_images =
work_to_schedule.tiles_to_process_for_images;
std::vector<DrawImage> new_locked_images;
- std::vector<scoped_refptr<TileTask>> new_locked_image_tasks;
for (const PrioritizedTile& prioritized_tile : tiles_to_process_for_images) {
Tile* tile = prioritized_tile.tile();
std::vector<DrawImage> images;
prioritized_tile.raster_source()->GetDiscardableImagesInRect(
tile->enclosing_layer_rect(), tile->contents_scale(), &images);
- ImageDecodeController::TracingInfo tracing_info(
- prepare_tiles_count_, prioritized_tile.priority().priority_bin);
- image_manager_.GetTasksForImagesAndRef(&images, &new_locked_image_tasks,
- tracing_info);
new_locked_images.insert(new_locked_images.end(), images.begin(),
images.end());
}
+ // TODO(vmpstr): SOON is misleading here, but these images can come from
+ // several diffent tiles. Rethink what we actually want to trace here. Note
+ // that I'm using SOON, since it can't be NOW (these are prepaint).
+ ImageDecodeController::TracingInfo tracing_info(prepare_tiles_count_,
+ TilePriority::SOON);
+ std::vector<scoped_refptr<TileTask>> new_locked_image_tasks =
+ image_manager_.SetPredecodeImages(std::move(new_locked_images),
+ tracing_info);
+
for (auto& task : new_locked_image_tasks) {
auto decode_it = std::find_if(graph_.nodes.begin(), graph_.nodes.end(),
[&task](const TaskGraph::Node& node) {
@@ -874,10 +877,11 @@
graph_.edges.push_back(TaskGraph::Edge(task.get(), all_done_task.get()));
}
- image_manager_.UnrefImages(locked_images_);
- // The old locked images have to stay around until past the ScheduleTasks call
- // below, so we do a swap instead of a move.
- locked_images_.swap(new_locked_images);
+ // The old locked images tasks have to stay around until past the
+ // ScheduleTasks call below, so we do a swap instead of a move.
+ // TODO(crbug.com/647402): Have the tile_task_manager keep a ref on the tasks,
+ // since it makes it awkward for the callers to keep refs on tasks that only
+ // exist within the task graph runner.
locked_image_tasks_.swap(new_locked_image_tasks);
// We must reduce the amount of unused resources before calling
@@ -997,8 +1001,7 @@
// Unref all the images.
auto images_it = scheduled_draw_images_.find(tile->id());
- const std::vector<DrawImage>& images = images_it->second;
- image_manager_.UnrefImages(images);
+ image_manager_.UnrefImages(images_it->second);
scheduled_draw_images_.erase(images_it);
if (was_canceled) {
@@ -1148,8 +1151,8 @@
// 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_manager_.UnrefImages(locked_images_);
- locked_images_.clear();
+ image_manager_.SetPredecodeImages(std::vector<DrawImage>(),
+ ImageDecodeController::TracingInfo());
locked_image_tasks_.clear();
}
diff --git a/cc/tiles/tile_manager.h b/cc/tiles/tile_manager.h
index 67744d2..c043f62 100644
--- a/cc/tiles/tile_manager.h
+++ b/cc/tiles/tile_manager.h
@@ -343,7 +343,6 @@
uint64_t next_tile_id_;
std::unordered_map<Tile::Id, std::vector<DrawImage>> scheduled_draw_images_;
- std::vector<DrawImage> locked_images_;
std::vector<scoped_refptr<TileTask>> locked_image_tasks_;
base::WeakPtrFactory<TileManager> task_set_finished_weak_ptr_factory_;