Refactoring AudioDirectiveHandler to support testing

Review URL: https://codereview.chromium.org/690213004

Cr-Commit-Position: refs/heads/master@{#302531}

components/copresence.gypi

@@ -24,8 +24,9 @@
         'copresence/copresence_manager_impl.cc',
         'copresence/copresence_switches.cc',
         'copresence/copresence_switches.h',
-        'copresence/handlers/audio/audio_directive_handler.cc',
         'copresence/handlers/audio/audio_directive_handler.h',
+        'copresence/handlers/audio/audio_directive_handler_impl.cc',
+        'copresence/handlers/audio/audio_directive_handler_impl.h',
         'copresence/handlers/audio/audio_directive_list.cc',
         'copresence/handlers/audio/audio_directive_list.h',
         'copresence/handlers/audio/tick_clock_ref_counted.cc',

components/copresence/BUILD.gn

@@ -8,8 +8,9 @@ static_library("copresence") {
     "copresence_manager_impl.cc",
     "copresence_switches.cc",
     "copresence_switches.h",
-    "handlers/audio/audio_directive_handler.cc",
     "handlers/audio/audio_directive_handler.h",
+    "handlers/audio/audio_directive_handler_impl.cc",
+    "handlers/audio/audio_directive_handler_impl.h",
     "handlers/audio/audio_directive_list.cc",
     "handlers/audio/audio_directive_list.h",
     "handlers/audio/tick_clock_ref_counted.cc",

components/copresence/handlers/audio/audio_directive_handler.h

@@ -7,86 +7,38 @@
 
 #include <string>
 
-#include "base/basictypes.h"
-#include "base/macros.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/time/time.h"
-#include "components/copresence/handlers/audio/audio_directive_list.h"
 #include "components/copresence/mediums/audio/audio_manager.h"
-#include "components/copresence/proto/data.pb.h"
+#include "components/copresence/public/copresence_constants.h"
 
 namespace base {
-class Timer;
-}
-
-namespace media {
-class AudioBusRefCounted;
+class TimeDelta;
 }
 
 namespace copresence {
 
-class TickClockRefCounted;
+class TokenInstruction;
 
 // The AudioDirectiveHandler handles audio transmit and receive instructions.
-// TODO(rkc): Currently since WhispernetClient can only have one token encoded
-// callback at a time, we need to have both the audible and inaudible in this
-// class. Investigate a better way to do this; a few options are abstracting
-// out token encoding to a separate class, or allowing whispernet to have
-// multiple callbacks for encoded tokens being sent back and have two versions
-// of this class.
-class AudioDirectiveHandler final {
+class AudioDirectiveHandler {
  public:
-  AudioDirectiveHandler();
-  ~AudioDirectiveHandler();
+  virtual ~AudioDirectiveHandler() {}
 
   // Do not use this class before calling this.
-  void Initialize(const AudioManager::DecodeSamplesCallback& decode_cb,
-                  const AudioManager::EncodeTokenCallback& encode_cb);
+  virtual void Initialize(
+      const AudioManager::DecodeSamplesCallback& decode_cb,
+      const AudioManager::EncodeTokenCallback& encode_cb) = 0;
 
   // Adds an instruction to our handler. The instruction will execute and be
   // removed after the ttl expires.
-  void AddInstruction(const copresence::TokenInstruction& instruction,
-                      const std::string& op_id,
-                      base::TimeDelta ttl_ms);
+  virtual void AddInstruction(const TokenInstruction& instruction,
+                              const std::string& op_id,
+                              base::TimeDelta ttl_ms) = 0;
 
   // Removes all instructions associated with this operation id.
-  void RemoveInstructions(const std::string& op_id);
+  virtual void RemoveInstructions(const std::string& op_id) = 0;
 
   // Returns the currently playing token.
-  const std::string PlayingToken(AudioType type) const;
-
-  // The manager being passed in needs to be uninitialized.
-  void set_audio_manager_for_testing(scoped_ptr<AudioManager> manager) {
-    audio_manager_ = manager.Pass();
-  }
-
-  void set_clock_for_testing(const scoped_refptr<TickClockRefCounted>& clock);
-  void set_timer_for_testing(scoped_ptr<base::Timer> timer);
-
- private:
-  // Processes the next active instruction, updating our audio manager state
-  // accordingly.
-  void ProcessNextInstruction();
-
-  // Returns the time that an instruction expires at. This will always return
-  // the earliest expiry time among all the active receive and transmit
-  // instructions. In case we don't have any active instructions, this method
-  // returns false.
-  bool GetNextInstructionExpiry(base::TimeTicks* next_event);
-
-  scoped_ptr<AudioManager> audio_manager_;
-
-  // Audible and inaudible lists.
-  // AUDIBLE = 0, INAUDIBLE = 1 (see copresence_constants.h).
-  AudioDirectiveList transmits_list_[2];
-  AudioDirectiveList receives_list_[2];
-
-  scoped_ptr<base::Timer> audio_event_timer_;
-
-  scoped_refptr<TickClockRefCounted> clock_;
-
-  DISALLOW_COPY_AND_ASSIGN(AudioDirectiveHandler);
+  virtual const std::string PlayingToken(AudioType type) const = 0;
 };
 
 }  // namespace copresence

components/copresence/handlers/audio/audio_directive_handler.cc → components/copresence/handlers/audio/audio_directive_handler_impl.cc

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/copresence/handlers/audio/audio_directive_handler.h"
+#include "components/copresence/handlers/audio/audio_directive_handler_impl.h"
+
+#include <algorithm>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -10,6 +12,7 @@
 #include "base/time/default_tick_clock.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "components/copresence/handlers/audio/audio_directive_list.h"
 #include "components/copresence/handlers/audio/tick_clock_ref_counted.h"
 #include "components/copresence/mediums/audio/audio_manager_impl.h"
 #include "components/copresence/proto/data.pb.h"
@@ -25,36 +28,53 @@ base::TimeTicks GetEarliestEventTime(AudioDirectiveList* list,
   if (!list->GetActiveDirective())
     return event_time;
 
-  if (event_time.is_null())
-    return list->GetActiveDirective()->end_time;
-  else
-    return std::min(list->GetActiveDirective()->end_time, event_time);
+  return event_time.is_null() ?
+      list->GetActiveDirective()->end_time :
+      std::min(list->GetActiveDirective()->end_time, event_time);
 }
 
 }  // namespace
 
-// Public methods.
 
-AudioDirectiveHandler::AudioDirectiveHandler()
-    : audio_event_timer_(new base::OneShotTimer<AudioDirectiveHandler>),
-      clock_(new TickClockRefCounted(
-          make_scoped_ptr(new base::DefaultTickClock))) {
-}
+// Public functions.
 
-AudioDirectiveHandler::~AudioDirectiveHandler() {
-}
+AudioDirectiveHandlerImpl::AudioDirectiveHandlerImpl()
+    : audio_manager_(new AudioManagerImpl),
+      audio_event_timer_(new base::OneShotTimer<AudioDirectiveHandler>),
+      clock_(new TickClockRefCounted(new base::DefaultTickClock)) {}
+
+// TODO(ckehoe): Merge these two constructors when
+//               delegating constructors are allowed.
+AudioDirectiveHandlerImpl::AudioDirectiveHandlerImpl(
+    scoped_ptr<AudioManager> audio_manager,
+    scoped_ptr<base::Timer> timer,
+    const scoped_refptr<TickClockRefCounted>& clock)
+    : audio_manager_(audio_manager.Pass()),
+      audio_event_timer_(timer.Pass()),
+      clock_(clock) {}
+
+AudioDirectiveHandlerImpl::~AudioDirectiveHandlerImpl() {}
 
-void AudioDirectiveHandler::Initialize(
+void AudioDirectiveHandlerImpl::Initialize(
     const AudioManager::DecodeSamplesCallback& decode_cb,
     const AudioManager::EncodeTokenCallback& encode_cb) {
   if (!audio_manager_)
     audio_manager_.reset(new AudioManagerImpl());
   audio_manager_->Initialize(decode_cb, encode_cb);
+
+  DCHECK(transmits_lists_.empty());
+  transmits_lists_.push_back(new AudioDirectiveList(clock_));
+  transmits_lists_.push_back(new AudioDirectiveList(clock_));
+
+  DCHECK(receives_lists_.empty());
+  receives_lists_.push_back(new AudioDirectiveList(clock_));
+  receives_lists_.push_back(new AudioDirectiveList(clock_));
 }
 
-void AudioDirectiveHandler::AddInstruction(const TokenInstruction& instruction,
-                                           const std::string& op_id,
-                                           base::TimeDelta ttl) {
+void AudioDirectiveHandlerImpl::AddInstruction(
+    const TokenInstruction& instruction,
+    const std::string& op_id,
+    base::TimeDelta ttl) {
   switch (instruction.token_instruction_type()) {
     case TRANSMIT:
       DVLOG(2) << "Audio Transmit Directive received. Token: "
@@ -63,11 +83,11 @@ void AudioDirectiveHandler::AddInstruction(const TokenInstruction& instruction,
                << " with TTL=" << ttl.InMilliseconds();
       switch (instruction.medium()) {
         case AUDIO_ULTRASOUND_PASSBAND:
-          transmits_list_[INAUDIBLE].AddDirective(op_id, ttl);
+          transmits_lists_[INAUDIBLE]->AddDirective(op_id, ttl);
           audio_manager_->SetToken(INAUDIBLE, instruction.token_id());
           break;
         case AUDIO_AUDIBLE_DTMF:
-          transmits_list_[AUDIBLE].AddDirective(op_id, ttl);
+          transmits_lists_[AUDIBLE]->AddDirective(op_id, ttl);
           audio_manager_->SetToken(AUDIBLE, instruction.token_id());
           break;
         default:
@@ -80,10 +100,10 @@ void AudioDirectiveHandler::AddInstruction(const TokenInstruction& instruction,
                << " with TTL=" << ttl.InMilliseconds();
       switch (instruction.medium()) {
         case AUDIO_ULTRASOUND_PASSBAND:
-          receives_list_[INAUDIBLE].AddDirective(op_id, ttl);
+          receives_lists_[INAUDIBLE]->AddDirective(op_id, ttl);
           break;
         case AUDIO_AUDIBLE_DTMF:
-          receives_list_[AUDIBLE].AddDirective(op_id, ttl);
+          receives_lists_[AUDIBLE]->AddDirective(op_id, ttl);
           break;
         default:
           NOTREACHED();
@@ -97,60 +117,47 @@ void AudioDirectiveHandler::AddInstruction(const TokenInstruction& instruction,
   ProcessNextInstruction();
 }
 
-void AudioDirectiveHandler::RemoveInstructions(const std::string& op_id) {
-  transmits_list_[AUDIBLE].RemoveDirective(op_id);
-  transmits_list_[INAUDIBLE].RemoveDirective(op_id);
-  receives_list_[AUDIBLE].RemoveDirective(op_id);
-  receives_list_[INAUDIBLE].RemoveDirective(op_id);
+void AudioDirectiveHandlerImpl::RemoveInstructions(const std::string& op_id) {
+  transmits_lists_[AUDIBLE]->RemoveDirective(op_id);
+  transmits_lists_[INAUDIBLE]->RemoveDirective(op_id);
+  receives_lists_[AUDIBLE]->RemoveDirective(op_id);
+  receives_lists_[INAUDIBLE]->RemoveDirective(op_id);
 
   ProcessNextInstruction();
 }
 
-const std::string AudioDirectiveHandler::PlayingToken(AudioType type) const {
+const std::string AudioDirectiveHandlerImpl::PlayingToken(
+    AudioType type) const {
   return audio_manager_->GetToken(type);
 }
 
-void AudioDirectiveHandler::set_clock_for_testing(
-    const scoped_refptr<TickClockRefCounted>& clock) {
-  clock_ = clock;
-
-  transmits_list_[AUDIBLE].set_clock_for_testing(clock);
-  transmits_list_[INAUDIBLE].set_clock_for_testing(clock);
-  receives_list_[AUDIBLE].set_clock_for_testing(clock);
-  receives_list_[INAUDIBLE].set_clock_for_testing(clock);
-}
-
-void AudioDirectiveHandler::set_timer_for_testing(
-    scoped_ptr<base::Timer> timer) {
-  audio_event_timer_.swap(timer);
-}
 
-// Private methods.
+// Private functions.
 
-void AudioDirectiveHandler::ProcessNextInstruction() {
+void AudioDirectiveHandlerImpl::ProcessNextInstruction() {
   DCHECK(audio_event_timer_);
   audio_event_timer_->Stop();
 
   // Change |audio_manager_| state for audible transmits.
-  if (transmits_list_[AUDIBLE].GetActiveDirective())
+  if (transmits_lists_[AUDIBLE]->GetActiveDirective())
     audio_manager_->StartPlaying(AUDIBLE);
   else
     audio_manager_->StopPlaying(AUDIBLE);
 
   // Change audio_manager_ state for inaudible transmits.
-  if (transmits_list_[INAUDIBLE].GetActiveDirective())
+  if (transmits_lists_[INAUDIBLE]->GetActiveDirective())
     audio_manager_->StartPlaying(INAUDIBLE);
   else
     audio_manager_->StopPlaying(INAUDIBLE);
 
   // Change audio_manager_ state for audible receives.
-  if (receives_list_[AUDIBLE].GetActiveDirective())
+  if (receives_lists_[AUDIBLE]->GetActiveDirective())
     audio_manager_->StartRecording(AUDIBLE);
   else
     audio_manager_->StopRecording(AUDIBLE);
 
   // Change audio_manager_ state for inaudible receives.
-  if (receives_list_[INAUDIBLE].GetActiveDirective())
+  if (receives_lists_[INAUDIBLE]->GetActiveDirective())
     audio_manager_->StartRecording(INAUDIBLE);
   else
     audio_manager_->StopRecording(INAUDIBLE);
@@ -160,18 +167,19 @@ void AudioDirectiveHandler::ProcessNextInstruction() {
     audio_event_timer_->Start(
         FROM_HERE,
         next_event_time - clock_->NowTicks(),
-        base::Bind(&AudioDirectiveHandler::ProcessNextInstruction,
+        base::Bind(&AudioDirectiveHandlerImpl::ProcessNextInstruction,
                    base::Unretained(this)));
   }
 }
 
-bool AudioDirectiveHandler::GetNextInstructionExpiry(base::TimeTicks* expiry) {
+bool AudioDirectiveHandlerImpl::GetNextInstructionExpiry(
+    base::TimeTicks* expiry) {
   DCHECK(expiry);
 
-  *expiry = GetEarliestEventTime(&transmits_list_[AUDIBLE], base::TimeTicks());
-  *expiry = GetEarliestEventTime(&transmits_list_[INAUDIBLE], *expiry);
-  *expiry = GetEarliestEventTime(&receives_list_[AUDIBLE], *expiry);
-  *expiry = GetEarliestEventTime(&receives_list_[INAUDIBLE], *expiry);
+  *expiry = GetEarliestEventTime(transmits_lists_[AUDIBLE], base::TimeTicks());
+  *expiry = GetEarliestEventTime(transmits_lists_[INAUDIBLE], *expiry);
+  *expiry = GetEarliestEventTime(receives_lists_[AUDIBLE], *expiry);
+  *expiry = GetEarliestEventTime(receives_lists_[INAUDIBLE], *expiry);
 
   return !expiry->is_null();
 }

components/copresence/handlers/audio/audio_directive_handler_impl.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_COPRESENCE_HANDLERS_AUDIO_AUDIO_DIRECTIVE_HANDLER_IMPL_H_
+#define COMPONENTS_COPRESENCE_HANDLERS_AUDIO_AUDIO_DIRECTIVE_HANDLER_IMPL_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "components/copresence/handlers/audio/audio_directive_handler.h"
+
+namespace base {
+class TimeTicks;
+class Timer;
+}
+
+namespace media {
+class AudioBusRefCounted;
+}
+
+namespace copresence {
+
+class AudioDirectiveList;
+class TickClockRefCounted;
+
+// The AudioDirectiveHandler handles audio transmit and receive instructions.
+// TODO(rkc): Currently since WhispernetClient can only have one token encoded
+// callback at a time, we need to have both the audible and inaudible in this
+// class. Investigate a better way to do this; a few options are abstracting
+// out token encoding to a separate class, or allowing whispernet to have
+// multiple callbacks for encoded tokens being sent back and have two versions
+// of this class.
+class AudioDirectiveHandlerImpl final : public AudioDirectiveHandler {
+ public:
+  AudioDirectiveHandlerImpl();
+  AudioDirectiveHandlerImpl(scoped_ptr<AudioManager> audio_manager,
+                            scoped_ptr<base::Timer> timer,
+                            const scoped_refptr<TickClockRefCounted>& clock);
+
+  ~AudioDirectiveHandlerImpl();
+
+  // Do not use this class before calling this.
+  void Initialize(const AudioManager::DecodeSamplesCallback& decode_cb,
+                  const AudioManager::EncodeTokenCallback& encode_cb) override;
+
+  // Adds an instruction to our handler. The instruction will execute and be
+  // removed after the ttl expires.
+  void AddInstruction(const copresence::TokenInstruction& instruction,
+                      const std::string& op_id,
+                      base::TimeDelta ttl_ms) override;
+
+  // Removes all instructions associated with this operation id.
+  void RemoveInstructions(const std::string& op_id) override;
+
+  // Returns the currently playing token.
+  const std::string PlayingToken(AudioType type) const override;
+
+ private:
+  // Processes the next active instruction,
+  // updating our audio manager state accordingly.
+  void ProcessNextInstruction();
+
+  // Returns the time that an instruction expires at. This will always return
+  // the earliest expiry time among all the active receive and transmit
+  // instructions. If we don't have any active instructions, returns false.
+  bool GetNextInstructionExpiry(base::TimeTicks* next_event);
+
+  scoped_ptr<AudioManager> audio_manager_;
+
+  scoped_ptr<base::Timer> audio_event_timer_;
+  scoped_refptr<TickClockRefCounted> clock_;
+
+  // Lists of transmits and receives, for both audible and inaudible tokens.
+  // AUDIBLE = element 0, INAUDIBLE = element 1 (see copresence_constants.h).
+  ScopedVector<AudioDirectiveList> transmits_lists_;
+  ScopedVector<AudioDirectiveList> receives_lists_;
+
+
+  DISALLOW_COPY_AND_ASSIGN(AudioDirectiveHandlerImpl);
+};
+
+}  // namespace copresence
+
+#endif  // COMPONENTS_COPRESENCE_HANDLERS_AUDIO_AUDIO_DIRECTIVE_HANDLER_IMPL_H_

components/copresence/handlers/audio/audio_directive_handler_unittest.cc

@@ -2,15 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/copresence/handlers/audio/audio_directive_handler.h"
-
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "base/timer/mock_timer.h"
+#include "components/copresence/handlers/audio/audio_directive_handler_impl.h"
 #include "components/copresence/handlers/audio/tick_clock_ref_counted.h"
 #include "components/copresence/mediums/audio/audio_manager.h"
+#include "components/copresence/proto/data.pb.h"
 #include "components/copresence/test/audio_test_support.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -55,33 +55,34 @@ class AudioManagerStub final : public AudioManager {
 
 class AudioDirectiveHandlerTest : public testing::Test {
  public:
-  AudioDirectiveHandlerTest()
-      : directive_handler_(new AudioDirectiveHandler()) {
-    scoped_ptr<AudioManagerStub> manager(new AudioManagerStub);
-    manager_ptr_ = manager.get();
-    directive_handler_->set_audio_manager_for_testing(manager.Pass());
+  AudioDirectiveHandlerTest() {
+    manager_ptr_ = new AudioManagerStub;
+    timer_ptr_ = new base::MockTimer(false, false);
+    clock_ptr_ = new base::SimpleTestTickClock;
+
+    directive_handler_.reset(new AudioDirectiveHandlerImpl(
+        make_scoped_ptr<AudioManager>(manager_ptr_),
+        make_scoped_ptr<base::Timer>(timer_ptr_),
+        make_scoped_refptr(new TickClockRefCounted(clock_ptr_))));
     directive_handler_->Initialize(base::Bind(&DecodeSamples),
                                    base::Bind(&EncodeToken));
   }
   ~AudioDirectiveHandlerTest() override {}
 
-  void DirectiveAdded() {}
-
  protected:
-  copresence::TokenInstruction CreateTransmitInstruction(
-      const std::string& token,
-      bool audible) {
-    copresence::TokenInstruction instruction;
-    instruction.set_token_instruction_type(copresence::TRANSMIT);
+  TokenInstruction CreateTransmitInstruction(const std::string& token,
+                                             bool audible) {
+    TokenInstruction instruction;
+    instruction.set_token_instruction_type(TRANSMIT);
     instruction.set_token_id(token);
     instruction.set_medium(audible ? AUDIO_AUDIBLE_DTMF
                                    : AUDIO_ULTRASOUND_PASSBAND);
     return instruction;
   }
 
-  copresence::TokenInstruction CreateReceiveInstruction(bool audible) {
-    copresence::TokenInstruction instruction;
-    instruction.set_token_instruction_type(copresence::RECEIVE);
+  TokenInstruction CreateReceiveInstruction(bool audible) {
+    TokenInstruction instruction;
+    instruction.set_token_instruction_type(RECEIVE);
     instruction.set_medium(audible ? AUDIO_AUDIBLE_DTMF
                                    : AUDIO_ULTRASOUND_PASSBAND);
     return instruction;
@@ -96,8 +97,11 @@ class AudioDirectiveHandlerTest : public testing::Test {
   // from the directive handler ctor) will post tasks.
   base::MessageLoop message_loop_;
   scoped_ptr<AudioDirectiveHandler> directive_handler_;
+
   // Unowned.
   AudioManagerStub* manager_ptr_;
+  base::MockTimer* timer_ptr_;
+  base::SimpleTestTickClock* clock_ptr_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AudioDirectiveHandlerTest);
@@ -139,17 +143,6 @@ TEST_F(AudioDirectiveHandlerTest, Basic) {
 }
 
 TEST_F(AudioDirectiveHandlerTest, Timed) {
-  scoped_ptr<base::SimpleTestTickClock> clock(new base::SimpleTestTickClock());
-  base::SimpleTestTickClock* clock_ptr = clock.get();
-
-  scoped_refptr<TickClockRefCounted> clock_proxy =
-      new TickClockRefCounted(clock.Pass());
-  directive_handler_->set_clock_for_testing(clock_proxy);
-
-  scoped_ptr<base::Timer> timer(new base::MockTimer(false, false));
-  base::MockTimer* timer_ptr = static_cast<base::MockTimer*>(timer.get());
-  directive_handler_->set_timer_for_testing(timer.Pass());
-
   const base::TimeDelta kTtl1 = base::TimeDelta::FromMilliseconds(1337);
   directive_handler_->AddInstruction(
       CreateTransmitInstruction("token", true), "op_id1", kTtl1);
@@ -169,7 +162,7 @@ TEST_F(AudioDirectiveHandlerTest, Timed) {
   // We *have* to call an operation on the directive handler after we advance
   // time to trigger the next set of operations, so ensure that after calling
   // advance, we are also calling another operation.
-  clock_ptr->Advance(kTtl3 + base::TimeDelta::FromMilliseconds(1));
+  clock_ptr_->Advance(kTtl3 + base::TimeDelta::FromMilliseconds(1));
 
   // We are now at base + 1337ms.
   // This instruction expires at base + (1337 + 1337 = 2674)
@@ -180,25 +173,25 @@ TEST_F(AudioDirectiveHandlerTest, Timed) {
   EXPECT_TRUE(IsRecording(AUDIBLE));
   EXPECT_FALSE(IsRecording(INAUDIBLE));
 
-  clock_ptr->Advance(base::TimeDelta::FromMilliseconds(1));
+  clock_ptr_->Advance(base::TimeDelta::FromMilliseconds(1));
 
   // We are now at base + 1338ms.
-  timer_ptr->Fire();
+  timer_ptr_->Fire();
   EXPECT_FALSE(IsPlaying(AUDIBLE));
   EXPECT_TRUE(IsPlaying(INAUDIBLE));
   EXPECT_TRUE(IsRecording(AUDIBLE));
 
-  clock_ptr->Advance(base::TimeDelta::FromMilliseconds(1));
+  clock_ptr_->Advance(base::TimeDelta::FromMilliseconds(1));
 
   // We are now at base + 1339ms.
-  timer_ptr->Fire();
+  timer_ptr_->Fire();
   EXPECT_FALSE(IsPlaying(INAUDIBLE));
   EXPECT_TRUE(IsRecording(AUDIBLE));
 
-  clock_ptr->Advance(kTtl3);
+  clock_ptr_->Advance(kTtl3);
 
   // We are now at base + 2676ms.
-  timer_ptr->Fire();
+  timer_ptr_->Fire();
   EXPECT_FALSE(IsRecording(AUDIBLE));
 }
 

components/copresence/handlers/audio/audio_directive_list.cc

@@ -4,40 +4,28 @@
 
 #include "components/copresence/handlers/audio/audio_directive_list.h"
 
-#include "base/bind.h"
-#include "base/logging.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/time/default_tick_clock.h"
-#include "base/time/time.h"
-#include "components/copresence/handlers/audio/tick_clock_ref_counted.h"
-
 namespace copresence {
 
-// Public methods.
+// Public functions.
 
-AudioDirective::AudioDirective() {
-}
+AudioDirective::AudioDirective() {}
 
 AudioDirective::AudioDirective(const std::string& op_id,
                                base::TimeTicks end_time)
-    : op_id(op_id), end_time(end_time) {
-}
+    : op_id(op_id), end_time(end_time) {}
 
-AudioDirectiveList::AudioDirectiveList()
-    : clock_(new TickClockRefCounted(
-          make_scoped_ptr(new base::DefaultTickClock))) {
-}
+AudioDirectiveList::AudioDirectiveList(
+    const scoped_refptr<TickClockRefCounted>& clock)
+    : clock_(clock) {}
 
-AudioDirectiveList::~AudioDirectiveList() {
-}
+AudioDirectiveList::~AudioDirectiveList() {}
 
 void AudioDirectiveList::AddDirective(const std::string& op_id,
                                       base::TimeDelta ttl) {
   base::TimeTicks end_time = clock_->NowTicks() + ttl;
 
-  // In case this op is already in the list, update it instead of adding
-  // it again.
-  std::vector<AudioDirective>::iterator it = FindDirectiveByOpId(op_id);
+  // If this op is already in the list, update it instead of adding it again.
+  auto it = FindDirectiveByOpId(op_id);
   if (it != active_directives_.end()) {
     it->end_time = end_time;
     std::make_heap(active_directives_.begin(),
@@ -53,7 +41,7 @@ void AudioDirectiveList::AddDirective(const std::string& op_id,
 }
 
 void AudioDirectiveList::RemoveDirective(const std::string& op_id) {
-  std::vector<AudioDirective>::iterator it = FindDirectiveByOpId(op_id);
+  auto it = FindDirectiveByOpId(op_id);
   if (it != active_directives_.end())
     active_directives_.erase(it);
 
@@ -63,30 +51,24 @@ void AudioDirectiveList::RemoveDirective(const std::string& op_id) {
 }
 
 scoped_ptr<AudioDirective> AudioDirectiveList::GetActiveDirective() {
-  // The top is always the instruction that is ending the latest. If that time
-  // has passed, means all our previous instructions have expired too, hence
-  // clear the list.
-  if (!active_directives_.empty() &&
+  // The top is always the instruction that is ending the latest.
+  // If that time has passed, all our previous instructions have expired too.
+  // So we clear the list.
+  if (active_directives_.empty() ||
       active_directives_.front().end_time < clock_->NowTicks()) {
     active_directives_.clear();
-  }
-
-  if (active_directives_.empty())
     return scoped_ptr<AudioDirective>().Pass();
+  }
 
   return make_scoped_ptr(new AudioDirective(active_directives_.front()));
 }
 
-void AudioDirectiveList::set_clock_for_testing(
-    const scoped_refptr<TickClockRefCounted>& clock) {
-  clock_ = clock;
-}
 
-// Private methods.
+// Private functions.
 
 std::vector<AudioDirective>::iterator AudioDirectiveList::FindDirectiveByOpId(
     const std::string& op_id) {
-  for (std::vector<AudioDirective>::iterator it = active_directives_.begin();
+  for (auto it = active_directives_.begin();
        it != active_directives_.end();
        ++it) {
     if (it->op_id == op_id)

components/copresence/handlers/audio/audio_directive_list.h

@@ -8,11 +8,12 @@
 #include <string>
 #include <vector>
 
-#include "base/callback.h"
-#include "base/macros.h"
+#include "base/callback_forward.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/time/default_tick_clock.h"
 #include "base/time/time.h"
+#include "components/copresence/handlers/audio/tick_clock_ref_counted.h"
 
 namespace media {
 class AudioBusRefCounted;
@@ -41,7 +42,8 @@ struct AudioDirective final {
 // classes from it.
 class AudioDirectiveList {
  public:
-  AudioDirectiveList();
+  explicit AudioDirectiveList(const scoped_refptr<TickClockRefCounted>& clock =
+      make_scoped_refptr(new TickClockRefCounted(new base::DefaultTickClock)));
   ~AudioDirectiveList();
 
   void AddDirective(const std::string& op_id, base::TimeDelta ttl);
@@ -49,8 +51,6 @@ class AudioDirectiveList {
 
   scoped_ptr<AudioDirective> GetActiveDirective();
 
-  void set_clock_for_testing(const scoped_refptr<TickClockRefCounted>& clock);
-
  private:
   // Comparator for comparing end_times on audio tokens.
   class LatestFirstComparator {

components/copresence/handlers/audio/audio_directive_list_unittest.cc

@@ -15,9 +15,7 @@ namespace copresence {
 
 class AudioDirectiveListTest : public testing::Test {
  public:
-  AudioDirectiveListTest() : directive_list_(new AudioDirectiveList()) {}
-
-  ~AudioDirectiveListTest() override {}
+  AudioDirectiveListTest() : directive_list_(new AudioDirectiveList) {}
 
  protected:
   base::MessageLoop message_loop_;

components/copresence/handlers/audio/tick_clock_ref_counted.cc

@@ -9,14 +9,15 @@
 namespace copresence {
 
 TickClockRefCounted::TickClockRefCounted(scoped_ptr<base::TickClock> clock)
-    : clock_(clock.Pass()) {
-}
+    : clock_(clock.Pass()) {}
+
+TickClockRefCounted::TickClockRefCounted(base::TickClock* clock)
+    : clock_(make_scoped_ptr(clock)) {}
 
-base::TimeTicks TickClockRefCounted::NowTicks() {
+base::TimeTicks TickClockRefCounted::NowTicks() const {
   return clock_->NowTicks();
 }
 
-TickClockRefCounted::~TickClockRefCounted() {
-}
+TickClockRefCounted::~TickClockRefCounted() {}
 
 }  // namespace copresence

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);
+
+  base::TimeTicks NowTicks() const;
 
  private:
   friend class base::RefCountedThreadSafe<TickClockRefCounted>;

components/copresence/handlers/directive_handler.cc

@@ -7,7 +7,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/time/time.h"
-#include "components/copresence/handlers/audio/audio_directive_handler.h"
+#include "components/copresence/handlers/audio/audio_directive_handler_impl.h"
 #include "components/copresence/proto/data.pb.h"
 
 namespace copresence {
@@ -15,7 +15,7 @@ namespace copresence {
 // Public functions
 
 DirectiveHandler::DirectiveHandler()
-  : audio_handler_(new AudioDirectiveHandler),
+  : audio_handler_(new AudioDirectiveHandlerImpl),
     whispernet_client_(nullptr) {}
 
 DirectiveHandler::~DirectiveHandler() {}