Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(87)

Unified Diff: components/wallpaper/wallpaper_color_calculator.cc

Issue 2824883006: [ash-md] Reduce thread hopping for cheap wallpaper color extraction. (Closed)
Patch Set: Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/wallpaper/wallpaper_color_calculator.cc
diff --git a/components/wallpaper/wallpaper_color_calculator.cc b/components/wallpaper/wallpaper_color_calculator.cc
index fdab3d8cd6d1a568352457a57ee7e16888130ecb..8be633312bbcd339cf241ea3e5ae3f049e7ceee8 100644
--- a/components/wallpaper/wallpaper_color_calculator.cc
+++ b/components/wallpaper/wallpaper_color_calculator.cc
@@ -15,6 +15,40 @@
namespace wallpaper {
+namespace {
+
+// The largest image size, in pixels, to synchronously calculate the prominent
+// color.
+const int kMaxPixelsForSynchronousCalculation = 100;
+
+// Wrapper for color_utils::CalculateProminentColorOfBitmap() that records
+// wallpaper specific metrics.
+SkColor CalculateWallpaperColor(const SkBitmap& bitmap,
+ color_utils::LumaRange luma,
+ color_utils::SaturationRange saturation) {
+ base::TimeTicks start_time = base::TimeTicks::Now();
+ const SkColor prominent_color =
+ color_utils::CalculateProminentColorOfBitmap(bitmap, luma, saturation);
+
+ UMA_HISTOGRAM_TIMES("Ash.Wallpaper.TimeSpentExtractingColors_v2",
+ base::TimeTicks::Now() - start_time);
+ UMA_HISTOGRAM_BOOLEAN("Ash.Wallpaper.ColorExtractionResult",
+ prominent_color != SK_ColorTRANSPARENT);
+
+ return prominent_color;
+}
+
+bool ShouldCalculateAsync(const gfx::ImageSkia& image) {
Evan Stade 2017/04/19 13:44:31 I think the name of this is backwards?
bruthig 2017/04/19 15:15:52 Nice catch, thx!
+ return kMaxPixelsForSynchronousCalculation >= image.width() * image.height();
Evan Stade 2017/04/19 13:44:31 nit: do you think this reads more clearly as width
bruthig 2017/04/19 15:15:52 Done.
+}
+
+void RecordCalculationExecutionType(bool was_synchronous) {
+ UMA_HISTOGRAM_BOOLEAN("Ash.Wallpaper.ColorExtraction.ExecutionType",
+ was_synchronous);
+}
+
+} // namespace
+
WallpaperColorCalculator::WallpaperColorCalculator(
const gfx::ImageSkia& image,
color_utils::LumaRange luma,
@@ -39,23 +73,33 @@ void WallpaperColorCalculator::RemoveObserver(
}
bool WallpaperColorCalculator::StartCalculation() {
- start_calculation_time_ = base::Time::Now();
-
- image_.MakeThreadSafe();
- if (base::PostTaskAndReplyWithResult(
- task_runner_.get(), FROM_HERE,
- base::Bind(&color_utils::CalculateProminentColorOfBitmap,
- *image_.bitmap(), luma_, saturation_),
- base::Bind(&WallpaperColorCalculator::NotifyCalculationComplete,
- weak_ptr_factory_.GetWeakPtr()))) {
+ if (ShouldCalculateAsync(image_)) {
+ RecordCalculationExecutionType(true);
+
+ const SkColor prominent_color =
+ CalculateWallpaperColor(*image_.bitmap(), luma_, saturation_);
+ WallpaperColorCalculator::NotifyCalculationComplete(prominent_color);
return true;
+ } else {
Evan Stade 2017/04/19 13:44:31 nit: no else after return.
bruthig 2017/04/19 15:15:52 Done.
+ RecordCalculationExecutionType(false);
+
+ image_.MakeThreadSafe();
+ if (base::PostTaskAndReplyWithResult(
+ task_runner_.get(), FROM_HERE,
+ base::Bind(&CalculateWallpaperColor, *image_.bitmap(), luma_,
+ saturation_),
+ base::Bind(&WallpaperColorCalculator::OnAsyncCalculationComplete,
+ weak_ptr_factory_.GetWeakPtr(),
+ base::TimeTicks::Now()))) {
+ return true;
+ }
+
+ LOG(WARNING) << "PostSequencedWorkerTask failed. "
+ << "Wallpaper promiment color may not be calculated.";
+
+ prominent_color_ = SK_ColorTRANSPARENT;
+ return false;
}
-
- LOG(WARNING) << "PostSequencedWorkerTask failed. "
- << "Wallpaper promiment color may not be calculated.";
-
- prominent_color_ = SK_ColorTRANSPARENT;
- return false;
}
void WallpaperColorCalculator::SetTaskRunnerForTest(
@@ -63,14 +107,16 @@ void WallpaperColorCalculator::SetTaskRunnerForTest(
task_runner_ = task_runner;
}
-void WallpaperColorCalculator::NotifyCalculationComplete(
+void WallpaperColorCalculator::OnAsyncCalculationComplete(
+ base::TimeTicks async_start_time,
SkColor prominent_color) {
- UMA_HISTOGRAM_MEDIUM_TIMES("Ash.Wallpaper.TimeSpentExtractingColors",
- base::Time::Now() - start_calculation_time_);
-
- UMA_HISTOGRAM_BOOLEAN("Ash.Wallpaper.ColorExtractionResult",
- prominent_color != SK_ColorTRANSPARENT);
+ UMA_HISTOGRAM_TIMES("Ash.Wallpaper.AsyncTimeSpentExtractingColors",
+ base::TimeTicks::Now() - async_start_time);
+ NotifyCalculationComplete(prominent_color);
+}
+void WallpaperColorCalculator::NotifyCalculationComplete(
+ SkColor prominent_color) {
prominent_color_ = prominent_color;
for (auto& observer : observers_)
observer.OnColorCalculationComplete();

Powered by Google App Engine
This is Rietveld 408576698