Extract colors from wallpaper and dynamically update login screen overlay

chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc

Index: chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
diff --git a/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc b/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
index 675f657833b61b87a09e1b6a674d56bc97d0d1ac..9dfc3b9855fa07dbcf458d15c6381e21567643ec 100644
--- a/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
+++ b/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
@@ -204,6 +204,7 @@ void SetKnownUserWallpaperFilesId(
 // A helper to set the wallpaper image for Classic Ash and Mash.
 void SetWallpaper(const gfx::ImageSkia& image,
                   wallpaper::WallpaperLayout layout) {
+  WallpaperManager::Get()->CalculateProminentColor(image);
   if (ash_util::IsRunningInMash()) {
     // In mash, connect to the WallpaperController interface via mojo.
     service_manager::Connector* connector =
@@ -427,6 +428,24 @@ bool WallpaperManager::IsPendingWallpaper(uint32_t image_id) {
   return false;
 }
 
+void WallpaperManager::CalculateProminentColor(const gfx::ImageSkia& image) {
+  // Cancel in-flight color calculations, if any.
+  if (color_calculator_) {
+    color_calculator_->RemoveObserver(this);
+    color_calculator_.reset();
+  }
+
+  color_calculator_ = base::MakeUnique<wallpaper::WallpaperColorCalculator>(
+      image, color_utils::LumaRange::DARK, color_utils::SaturationRange::MUTED,

[xdai1, 2017/06/13 23:04:24]

Is the color scheme for the login screen fixed? Can we use the same color scheme
as in the shelf? If so, maybe they can share codes?

[Wenzhao (Colin) Zang, 2017/06/14 00:44:50]

The color scheme for the shelf is derived from the command line, as in 
(please open in a new tab)

https://cs.chromium.org/chromium/src/ash/wallpaper/wallpaper_controller.cc?dr...

However the login screen always has a fixed color scheme, so they are not the
same.

[xdai1, 2017/06/14 18:23:40]

The code logic of WallpaperColorCalculator here is almost the same as in
WallpaperController, except the color scheme (even for the color scheme, the
command switch in the shelf might be only used for developing/testing purpose. I
don't think we set different shelf color scheme for different device board). And
the color seems calculated twice in the same function SetWallpaper()
 (WallpaperController::CalculateWallpaperColors() is called in the sub-function
of SetWallpaper()), which seems redundant?

So instead of making SigninScreenHandler an observer of WallpaperManagerBase, it
might be better to make it an observer of WallpaperController?

[xiyuan, 2017/06/14 18:28:26]

WallpaperController is part of ash. If we do that, we need to do it via mojo to
be ready for mash.

[Wenzhao (Colin) Zang, 2017/06/15 05:02:08]

I understand the concerns. Before writing this CL I already realized it's better
to make it an observer of WallpaperController. So please advise on the following
options:

1) We may not need to fix crbug.com/733391 for now, because this CL, including
most other codes related to login screen will be discarded very soon, since the
login screen will be completely re-implemented. So this CL and some other CLs
I'm working on are temporary codes in order to meet the hard deadline - M61.
This is the last major chunk of codes before completing the MD login screen
re-design, and the UI review team already complains about the lack of color
extraction. So are these codes acceptable for just two months?

2) If the codes are unacceptable, and we want to start working on
crbug.com/733391 now (but the codes are likely to be discarded in two months
too), I really need an alternative way to extract colors which can be used
before the mojo calls are ready. Are there any suggestions? Because the
background color is an integral part of the login screen, the earlier the
UI-review team can see the color extraction results the more time I'll have to
fix other potential issues before M61.  

Thanks.

+      task_runner_);
+  color_calculator_->AddObserver(this);
+  if (!color_calculator_->StartCalculation()) {

[xdai1, 2017/06/13 23:04:24]

If the calculation fails to be initiated, why do we need to inform the observer
the complete event?

[Wenzhao (Colin) Zang, 2017/06/14 00:44:50]

Because the front end needs to be notified of the failure, and it will check
whether the optional value exists to determine the status. The function name may
wrongly imply that the calculation always completes, so the name is changed.

+    prominent_color_.reset();
+    for (auto& observer : observers_)
+      observer.OnColorCalculationComplete();
+  }
+}
+
 WallpaperManager::WallpaperResolution
 WallpaperManager::GetAppropriateResolution() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -911,6 +930,15 @@ void WallpaperManager::OnWindowDestroying(aura::Window* window) {
       user_manager::UserManager::Get()->GetActiveUser()->username_hash());
 }
 
+void WallpaperManager::OnColorCalculationComplete() {
+  if (color_calculator_->prominent_color() == prominent_color_)
+    return;

[xiyuan, 2017/06/13 23:23:48]

|color_calculator_|.reset() is skipped when this happens.

[Wenzhao (Colin) Zang, 2017/06/14 00:44:50]

Done. Thanks for pointing it out.

+  prominent_color_ = color_calculator_->prominent_color();
+  color_calculator_.reset();
+  for (auto& observer : observers_)
+    observer.OnColorCalculationComplete();
+}
+
 // WallpaperManager, private: --------------------------------------------------
 
 WallpaperManager::WallpaperManager()