[ash-md] Reduce thread hopping for cheap wallpaper color extraction.

components/wallpaper/wallpaper_color_calculator.cc

 // Copyright 2017 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 "components/wallpaper/wallpaper_color_calculator.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/task_runner.h"
 #include "base/task_runner_util.h"
 #include "components/wallpaper/wallpaper_color_calculator_observer.h"
 
 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 * height <= Max?

[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,
     color_utils::SaturationRange saturation,
     scoped_refptr<base::TaskRunner> task_runner)
     : image_(image),
       luma_(luma),
       saturation_(saturation),
       task_runner_(std::move(task_runner)),
       weak_ptr_factory_(this) {}
 
 WallpaperColorCalculator::~WallpaperColorCalculator() {}
 
 void WallpaperColorCalculator::AddObserver(
     WallpaperColorCalculatorObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void WallpaperColorCalculator::RemoveObserver(
     WallpaperColorCalculatorObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
 bool WallpaperColorCalculator::StartCalculation() {
-  start_calculation_time_ = base::Time::Now();
+  if (ShouldCalculateAsync(image_)) {
+    RecordCalculationExecutionType(true);
 
-  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()))) {
+    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(
     scoped_refptr<base::TaskRunner> task_runner) {
   task_runner_ = task_runner;
 }
 
+void WallpaperColorCalculator::OnAsyncCalculationComplete(
+    base::TimeTicks async_start_time,
+    SkColor prominent_color) {
+  UMA_HISTOGRAM_TIMES("Ash.Wallpaper.AsyncTimeSpentExtractingColors",
+                      base::TimeTicks::Now() - async_start_time);
+  NotifyCalculationComplete(prominent_color);
+}
+
 void WallpaperColorCalculator::NotifyCalculationComplete(
     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);
-
   prominent_color_ = prominent_color;
   for (auto& observer : observers_)
     observer.OnColorCalculationComplete();
 
   // This could be deleted!
 }
 
 }  // namespace wallpaper