Refactoring AudioDirectiveHandler to support testing

components/copresence/handlers/audio/tick_clock_ref_counted.h

Index: components/copresence/handlers/audio/tick_clock_ref_counted.h
diff --git a/components/copresence/handlers/audio/tick_clock_ref_counted.h b/components/copresence/handlers/audio/tick_clock_ref_counted.h
index 5bece752faaff08ae7b0fed869a312c80153b318..c461432feb0eb9a5db013d0cf90d5793ce1732f0 100644
--- a/components/copresence/handlers/audio/tick_clock_ref_counted.h
+++ b/components/copresence/handlers/audio/tick_clock_ref_counted.h
@@ -8,10 +8,10 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/time/time.h"
 
 namespace base {
 class TickClock;
+class TimeTicks;
 }
 
 namespace copresence {
@@ -20,7 +20,11 @@ class TickClockRefCounted
     : public base::RefCountedThreadSafe<TickClockRefCounted> {
  public:
   explicit TickClockRefCounted(scoped_ptr<base::TickClock> clock);
-  base::TimeTicks NowTicks();
+
+  // Takes ownership of the clock.
+  explicit TickClockRefCounted(base::TickClock* clock);

[rkc, 2014/11/03 20:14:43]

Why do we need this?

[Charlie, 2014/11/03 22:45:13]

There's already a lot of boilerplate to create one of these things. In the unit
test, for example, the current syntax is:

make_scoped_refptr(new TickClockRefCounted(clock_ptr_))

Without this convenience constructor, we would instead write:

make_scoped_refptr(new TickClockRefCounted(
    make_scoped_ptr<base::TickClock>(clock_ptr_)))

which is terrible to read. We just want a reference-counted clock!

Given that the only purpose of this class is to wrap a preexisting type for
reference counting, I don't think it's surprising that the constructor would
take ownership of the pointer passed in. But it's documented anyway.

[rkc, 2014/11/03 22:52:15]

That does look pretty ugly.

+
+  base::TimeTicks NowTicks() const;
 
  private:
   friend class base::RefCountedThreadSafe<TickClockRefCounted>;