Make EventHandler::ActiveIntervalTimer mockable.

Source/platform/Timer.h

Index: Source/platform/Timer.h
diff --git a/Source/platform/Timer.h b/Source/platform/Timer.h
index 1f03cf11dc894643c79ec0b2ed0eda911cebf9fb..2ca6cce94d8d4144d9c023f9099896de2cc67624 100644
--- a/Source/platform/Timer.h
+++ b/Source/platform/Timer.h
@@ -105,8 +105,12 @@ private:
     friend class TimerHeapReference;
 };
 
+template <typename TimerFiredClass> class MockableTimer;
+
 template <typename TimerFiredClass>
 class Timer : public TimerBase {
+    friend class MockableTimer<TimerFiredClass>;
+
 public:
     typedef void (TimerFiredClass::*TimerFiredFunction)(Timer*);
 
@@ -120,6 +124,37 @@ private:
     TimerFiredFunction m_function;
 };
 
+// Just derive from TimerBase and skip the template-fu?

[Zeeshan Qureshi, 2013/12/13 02:05:58]

Deriving from TimerBase might just be cleaner.

[Rick Byers, 2013/12/13 22:52:25]

I like that you don't have to duplicate anything here (like the
m_object/m_function) - I think deriving from Timer is the right way to go.

+template <typename TimerFiredClass>
+class MockableTimer : public Timer<TimerFiredClass> {
+public:
+    typedef typename Timer<TimerFiredClass>::TimerFiredFunction TimerFiredFunction;
+
+    MockableTimer(TimerFiredClass* o, TimerFiredFunction f)
+        : Timer<TimerFiredClass>(o, f)
+        , m_suspended(false)
+    {
+    }
+
+    bool isSuspended() { return m_suspended; }
+
+    void suspend() { m_suspended = true; }

[Rick Byers, 2013/12/13 22:52:25]

I wonder if we should make these a little scarrier names that clarify their role
- eg. 'setManualModeForTesting' and 'manualFireForTesting' or something?

[Zeeshan Qureshi, 2014/01/07 19:35:09]

Yes!

+
+    void resume() { m_suspended = false; }
+
+    // Should this only work when timer is suspended?

[Zeeshan Qureshi, 2013/12/13 02:05:58]

Not sure, maybe only allow this when m_suspended?

[Rick Byers, 2013/12/13 22:52:25]

Yeah, probably a good idea to ASSERT(m_suspended) - we should try to avoid
accidental use of these APIs outside of testing.

I think you should also try to preserve the illusion that the timer really
fired.  Eg. if it's active, call stop.  Ideally you could also ASSERT that
'fire' is only being used in place of a single actual fired event - eg. either
the Timer isActive, or fired() was previously invoked and suppressed.

I'm just trying to avoid situations where a test can do something different from
what a real timer would have done.  The only power we want to give the test is
the ability to make the firing time predictable. 

[Zeeshan Qureshi, 2014/01/07 19:35:09]

Done, it's a little tricky to reason about pending fires for repeating timers so
I'm thinking that a manual fire clears all pending ones and thus the boolean
instead of counting them.

+    void fire() { Timer<TimerFiredClass>::fired(); }
+
+private:
+    bool m_suspended;
+
+    virtual void fired()
+    {
+        if (!m_suspended)
+            Timer<TimerFiredClass>::fired();
+    }
+};
+
 inline bool TimerBase::isActive() const
 {
     ASSERT(m_thread == currentThread());