Make EventHandler::ActiveIntervalTimer mockable.

Source/platform/Timer.h

 /*
  * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 87 matching lines @@
 
 #ifndef NDEBUG
     ThreadIdentifier m_thread;
 #endif
 
     friend class ThreadTimers;
     friend class TimerHeapLessThanFunction;
     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*);
 
     Timer(TimerFiredClass* o, TimerFiredFunction f)
         : m_object(o), m_function(f) { }
 
 private:
     virtual void fired() { (m_object->*m_function)(this); }
 
     TimerFiredClass* m_object;
     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());
     return m_nextFireTime;
 }
 
 template <typename TimerFiredClass>
 class DeferrableOneShotTimer : private TimerBase {
 public:
     typedef void (TimerFiredClass::*TimerFiredFunction)(DeferrableOneShotTimer*);
@@ skipping 37 matching lines @@
     TimerFiredClass* m_object;
     TimerFiredFunction m_function;
 
     double m_delay;
     bool m_shouldRestartWhenTimerFires;
 };
 
 }
 
 #endif