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..68c6436bde8a06db316523bc7fa2816a8cb895de 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,50 @@ private:
     TimerFiredFunction m_function;
 };
 
+// Just derive from TimerBase and skip the template-fu?

[Rick Byers, 2014/01/10 21:46:50]

I think you need the template-fu to get the timer-fired behavior wired up in a
way consistent with other uses of Timer.

[Zeeshan Qureshi, 2014/01/10 22:20:41]

I think we already decided on this, forgot to remove comment.

+template <typename TimerFiredClass>

[Rick Byers, 2014/01/10 21:46:50]

I'd add a comment here saying the idea is to allow specific timers to be
controlled manually from tests.  Maybe mention that ideally we'd have a full
mock-clock system that would make this unnecessary.

[Zeeshan Qureshi, 2014/01/10 22:20:41]

Yes, adding.

+class MockableTimer : public Timer<TimerFiredClass> {
+public:
+    typedef typename Timer<TimerFiredClass>::TimerFiredFunction TimerFiredFunction;
+
+    MockableTimer(TimerFiredClass* o, TimerFiredFunction f)
+        : Timer<TimerFiredClass>(o, f)
+        , m_manualMode(false)
+        , m_firePending(false)
+    {
+    }
+
+    bool inManualModeForTesting() const { return m_manualMode; }

[Rick Byers, 2014/01/10 21:46:50]

blink coding guidelines say to use bare words for getters that match the name of
the variable.  So I guess we want to rename the field to m_manualModeForTesting,
and make the getter 'bool manualModeForTesting()'.

Actually, this method is used only by the timer unit test, and in a pretty
trivial way, right?  Perhaps we should just remove it entirely - you can unit
test the timer by testing it's behavior, not it's internal state.

+
+    bool firePendingForTesting() const { return m_firePending; }

[Rick Byers, 2014/01/10 21:46:50]

Again, this is used only by the unit test, right?  Does it really add enough
value to justify the public surface area do you think?  You can mostly test the
behavior here, but I guess you can't really verify your ASSERT conditions
without this.  Up to you...

[Zeeshan Qureshi, 2014/01/10 22:20:41]

Removing the above methods seems fine, I was under the impression we needed to
have accessors for most of state.

[Rick Byers, 2014/01/10 22:24:36]

Nope, encapsulation is your friend!

+
+    void enterManualModeForTesting() { m_manualMode = true; }

[Rick Byers, 2014/01/10 21:46:50]

collapse enter/leave into a single 'setManualModeForTesting' function.

+
+    void leaveManualModeForTesting() { m_manualMode = false; }
+
+    void manualFireForTesting()
+    {
+        ASSERT(m_manualMode);
+        ASSERT(Timer<TimerFiredClass>::isActive() || m_firePending);
+        Timer<TimerFiredClass>::fired();
+        if (!Timer<TimerFiredClass>::repeatInterval())
+            Timer<TimerFiredClass>::stop();
+        m_firePending = false;
+    }
+
+private:
+    bool m_manualMode;
+    bool m_firePending;
+
+    virtual void fired()
+    {
+        if (m_manualMode)
+            m_firePending = true;
+        if (!m_manualMode)
+            Timer<TimerFiredClass>::fired();
+    }
+};
+
 inline bool TimerBase::isActive() const
 {
     ASSERT(m_thread == currentThread());