[Night Light] CL12: String changes and fix frequent requests.

chrome/browser/chromeos/night_light/night_light_client.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 CHROME_BROWSER_CHROMEOS_NIGHT_LIGHT_NIGHT_LIGHT_CLIENT_H_
 #define CHROME_BROWSER_CHROMEOS_NIGHT_LIGHT_NIGHT_LIGHT_CLIENT_H_
 
 #include "ash/public/interfaces/night_light_controller.mojom.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
 #include "chromeos/geolocation/simple_geolocation_provider.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
+namespace base {
+class Clock;
+class TickClock;
+}  // namespace base
+
 namespace net {
 class URLRequestContextGetter;
 }
 
 // Periodically requests the IP-based geolocation and provides it to the
 // NightLightController running in ash.
 class NightLightClient
     : public NON_EXPORTED_BASE(ash::mojom::NightLightClient) {
  public:
+  NightLightClient(net::URLRequestContextGetter* url_context_getter,
+                   base::TickClock* tick_clock,
+                   base::Clock* clock);
   explicit NightLightClient(net::URLRequestContextGetter* url_context_getter);

[stevenjb, 2017/07/01 00:02:35]

Avoid multiple constructors for high level classes like this.

Since it looks like this is for testing, prefer a SetClocksForTest() method
instead of a separate constructor. 

[James Cook, 2017/07/01 00:06:57]

Or make the parameters mandatory and use a SystemClock where you allocate the
object.

[afakhry, 2017/07/01 00:26:15]

The problem is base::Timer, for which we cannot change its TickClock once
constructed:
https://cs.chromium.org/chromium/src/base/timer/timer.h?q=base::Timer+package...,
so we need to have this in the constructor.

I removed the explicit constructor, and added a comment that passing nullptr
will result in using the default system clocks, which is the behavior of all the
Timers (ex:
https://cs.chromium.org/chromium/src/base/timer/timer.h?q=base::Timer+package...).

[stevenjb, 2017/07/01 00:31:26]

Could you use a unique_ptr<> for timer_?

[afakhry, 2017/07/01 00:57:44]

Done. So much for those tests! :(

   ~NightLightClient() override;
 
   // Starts watching changes in the Night Light schedule type in order to begin
   // periodically pushing user's IP-based geoposition to NightLightController as
   // long as the type is set to "sunset to sunrise".
   void Start();
 
   // ash::mojom::NightLightClient:
   void OnScheduleTypeChanged(
       ash::mojom::NightLightController::ScheduleType new_type) override;
 
+  const base::OneShotTimer& timer() const { return timer_; }
+
+  base::Time last_successful_geo_request_time() const {
+    return last_successful_geo_request_time_;
+  }
+
   bool using_geoposition() const { return using_geoposition_; }
 
+  static base::TimeDelta GetNextRequestDelayAfterSuccessForTesting();
+
   void SetNightLightControllerPtrForTesting(
       ash::mojom::NightLightControllerPtr controller);
 
   void FlushNightLightControllerForTesting();
 
  protected:
   void OnGeoposition(const chromeos::Geoposition& position,
                      bool server_error,
                      const base::TimeDelta elapsed);
 
  private:
+  base::Time GetNow() const;
+
+  // Sends the most recent valid geoposition to NightLightController in ash.
+  void SendCurrentGeoposition();
+
   void ScheduleNextRequest(base::TimeDelta delay);
 
   // Virtual so that it can be overriden by a fake implementation in unit tests
   // that doesn't request actual geopositions.
   virtual void RequestGeoposition();
 
   // The IP-based geolocation provider.
   chromeos::SimpleGeolocationProvider provider_;
 
   ash::mojom::NightLightControllerPtr night_light_controller_;
   mojo::Binding<ash::mojom::NightLightClient> binding_;
 
   // Delay after which a new request is retried after a failed one.
   base::TimeDelta backoff_delay_;
 
   base::OneShotTimer timer_;
 
+  // Optional Used in tests to override the time of "Now".
+  base::Clock* clock_;  // Not owned.
+
+  // Last successful geoposition coordinates and its timestamp.
+  base::Time last_successful_geo_request_time_;
+  double latitude_ = 0.0;
+  double longitude_ = 0.0;
+
   // True as long as the schedule type is set to "sunset to sunrise", which
   // means this client will be retrieving the IP-based geoposition once per day.
   bool using_geoposition_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(NightLightClient);
 };
 
 #endif  // CHROME_BROWSER_CHROMEOS_NIGHT_LIGHT_NIGHT_LIGHT_CLIENT_H_