[Night Light] CL2: Ash and system tray work

ash/system/night_light/night_light_controller.h

+// 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.
+
+#ifndef ASH_SYSTEM_NIGHT_LIGHT_NIGHT_LIGHT_CONTROLLER_H_
+#define ASH_SYSTEM_NIGHT_LIGHT_NIGHT_LIGHT_CONTROLLER_H_
+
+#include "ash/ash_export.h"
+#include "ash/session/session_observer.h"
+#include "base/observer_list.h"
+
+class PrefRegistrySimple;
+
+namespace ash {
+
+class SessionController;
+
+class ASH_EXPORT NightLightController : public SessionObserver {

[James Cook, 2017/05/05 17:12:31]

nit: Please add a class comment with a one-line description of what this feature
is/does.

[afakhry, 2017/05/05 20:04:11]

Done.

+ public:
+  class Observer {
+   public:
+    Observer() {}

[James Cook, 2017/05/05 17:12:31]

nit: We usually write observers as:

class Observer {
 public:
  virtual void OnFoo() = 0;

 protected:
  virtual ~Observer() {}
}

[afakhry, 2017/05/05 20:04:10]

Done.

+    virtual ~Observer() {}
+
+    // Emitted when the night light status is changed.
+    virtual void OnStatusChanged(bool new_status) = 0;

[James Cook, 2017/05/05 17:12:31]

nit: Maybe OnNightLightChanged(bool enabled)? That would avoid future function
name collisions if other classes want to be observers, and make it more clear
what "status" is.

[afakhry, 2017/05/05 20:04:11]

Done.

+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Observer);
+  };
+
+  explicit NightLightController(SessionController* session_controller);
+  ~NightLightController() override;
+
+  static void RegisterPrefs(PrefRegistrySimple* registry);
+
+  static void DisableAnimationsForTests();

[James Cook, 2017/05/05 17:12:31]

nit: ForTest or ForTesting

Also, document which animations (casual readers might expect the animation is
related to the tray icon)

[afakhry, 2017/05/05 20:04:10]

Done.

+
+  float color_temperature() const { return color_temperature_; }
+  bool enabled() const { return enabled_; }
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  void Toggle();
+
+  void SetEnabled(bool enabled);

[James Cook, 2017/05/05 17:12:31]

Do you need both Toggle() and enabled()/SetEnabled() ?

[afakhry, 2017/05/05 20:04:10]

SetEnabled() is useful for initializing from prefs, to set it to a specific
value. Toggle() is just implemented in terms of SetEnabled() so it's not a
problem.

+
+  void SetColorTemperature(float temperature);

[James Cook, 2017/05/05 17:12:31]

nit: document expected values for temperature, or refer to member variable
below.

[afakhry, 2017/05/05 20:04:11]

Done.

+
+  // ash::SessionObserver:
+  void OnActiveUserSessionChanged(const AccountId& account_id) override;
+  void OnSessionStateChanged(session_manager::SessionState state) override;
+
+ private:
+  void Refresh();
+
+  void InitFromUserPrefs();
+
+  void PersistUserPrefs();
+
+  void NotifyStatusChanged();
+
+  // The observed session controller instance from which we know when to
+  // initialize the NightLight settings from the user preferences.
+  SessionController* const session_controller_;
+
+  // The applied color temperature value when NightLight is turned ON. It's a
+  // value from 0.0f (least warm -- NightLight is OFF) to 1.0f (most warm).

[James Cook, 2017/05/05 17:12:31]

Is 0.0 equivalent to nightlight off? If so, do you need the enabled flag?

[afakhry, 2017/05/05 20:04:11]

Yes 0.0f === off. But I still need enabled_. The idea is color_temperature_
should hold the value that will be set when Night Light is ON. So I don't change
it unless the user changes it. This keeps the status separate.

[James Cook, 2017/05/05 20:47:43]

So it's possible to turn Night Light on, but the color temperature doesn't
change from the screen's default? That seems odd.

Maybe I'm misreading the "NightLight is off" part of the comment?

[afakhry, 2017/05/06 00:49:34]

Oh I think I might need to remove that "OFF" comment if it's confusing. What I
was trying to say is that if temperature is set to 0.0f, from the ui::Layer
perspective, it's as if it's completely off. 

https://cs.chromium.org/chromium/src/ui/compositor/layer.cc?type=cs&q=SetLaye...

What I also meant to say is that we can remove |enabled_| and keep changing
|color_temperature_|, set it to 0.0f when we want to turn it off, but then we'd
want to keep re-reading the set user-desired color temperature from the prefs
whenever we want to turn it back on.

+  float color_temperature_ = 0.5f;
+
+  bool enabled_ = false;
+
+  base::ObserverList<Observer> observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(NightLightController);
+};
+
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_NIGHT_LIGHT_NIGHT_LIGHT_CONTROLLER_H_