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?

[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());
     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