Refactor net::BackoffEntry to not require subclassing

chromeos/network/portal_detector/network_portal_detector_strategy.h

Index: chromeos/network/portal_detector/network_portal_detector_strategy.h
diff --git a/chromeos/network/portal_detector/network_portal_detector_strategy.h b/chromeos/network/portal_detector/network_portal_detector_strategy.h
index ff6e890e97c6d620d591296e939ebb7490816562..fc1ec544e72ba8d71fd0b0e0e0acd133652282e3 100644
--- a/chromeos/network/portal_detector/network_portal_detector_strategy.h
+++ b/chromeos/network/portal_detector/network_portal_detector_strategy.h
@@ -9,6 +9,7 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "chromeos/chromeos_export.h"
 #include "net/base/backoff_entry.h"
@@ -23,9 +24,9 @@ class CHROMEOS_EXPORT PortalDetectorStrategy {
     STRATEGY_ID_SESSION
   };
 
-  class Delegate {
+  class Delegate : public base::TickClock {

[stevenjb, 2015/04/13 15:20:10]

I'm not actually very familiar with this code, but having a Delegate derive from
TickClock seems strange. +pneubeck@, +ygorshenin@ who are more familiar with
this code.

[pneubeck (no reviews), 2015/04/14 10:27:04]

Looking more into the usage of this, I think it might be best to have a
  // The returned object lives (at least) until the destruction of the delegate.
  base::TickClock* GetClock() = 0;

This allows one to pass _in_ an existing TickClock without having to know about
the actual TickClock interface.
E.g. NetworkPortalDetectorImpl::NowTicks should in turn take a TickClock for
testing and just pass it on, instead of maintaining a time_ticks_for_testing_
member.


As this is probably affecting more code than what the objective of this CL is,
feel free to go with your current implementation. Which I think is fine in this
corner case where TickClock has only a single method.

[johnme, 2015/04/20 15:52:44]

Acknowledged.

    public:
-    virtual ~Delegate() {}
+    ~Delegate() override;
 
     // Returns number of attempts in a row with NO RESPONSE result.
     // If last detection attempt has different result, returns 0.
@@ -34,8 +35,8 @@ class CHROMEOS_EXPORT PortalDetectorStrategy {
     // Returns time when current attempt was started.
     virtual base::TimeTicks AttemptStartTime() = 0;
 
-    // Returns current TimeTicks.
-    virtual base::TimeTicks GetCurrentTimeTicks() = 0;
+    // TickClock implementation.

[pneubeck (no reviews), 2015/04/14 10:27:04]

nit: not required to repeat abstract functions.
I'd just remove these two lines.

[johnme, 2015/04/20 15:52:44]

Done.

+    base::TimeTicks NowTicks() override = 0;
   };
 
   virtual ~PortalDetectorStrategy();
@@ -65,8 +66,6 @@ class CHROMEOS_EXPORT PortalDetectorStrategy {
   void OnDetectionCompleted();
 
  protected:
-  class BackoffEntryImpl;
-
   explicit PortalDetectorStrategy(Delegate* delegate);
 
   // Interface for subclasses:
@@ -74,7 +73,7 @@ class CHROMEOS_EXPORT PortalDetectorStrategy {
 
   Delegate* delegate_;
   net::BackoffEntry::Policy policy_;
-  scoped_ptr<BackoffEntryImpl> backoff_entry_;
+  scoped_ptr<net::BackoffEntry> backoff_entry_;
 
  private:
   friend class NetworkPortalDetectorImplTest;