Redesign the copresence audio handlers.

components/copresence/handlers/audio/audio_directive_handler_unittest.cc

Index: components/copresence/handlers/audio/audio_directive_handler_unittest.cc
diff --git a/components/copresence/handlers/audio/audio_directive_handler_unittest.cc b/components/copresence/handlers/audio/audio_directive_handler_unittest.cc
index 31c5f58adb911f6e5c7ccdeebf939b366de15c86..0d5afd01dc04ae256af05311e3c1ecdefc287889 100644
--- a/components/copresence/handlers/audio/audio_directive_handler_unittest.cc
+++ b/components/copresence/handlers/audio/audio_directive_handler_unittest.cc
@@ -5,74 +5,75 @@
 #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 "components/copresence/mediums/audio/audio_player.h"
-#include "components/copresence/mediums/audio/audio_recorder.h"
+#include "components/copresence/mediums/audio/audio_manager.h"
 #include "components/copresence/test/audio_test_support.h"
-#include "media/base/audio_bus.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using ::testing::_;
-using ::testing::Le;
-
 namespace copresence {
 
-class TestAudioPlayer : public AudioPlayer {
+namespace {
+
+class TestAudioManager final : public AudioManager {

[Daniel Erat, 2014/10/21 19:43:22]

nit: AudioManagerStub

[rkc, 2014/10/22 00:06:20]

Done.

  public:
-  TestAudioPlayer() {}
-  virtual ~TestAudioPlayer() {}
-
-  // AudioPlayer overrides:
-  virtual void Initialize() override {}
-  virtual void Play(
-      const scoped_refptr<media::AudioBusRefCounted>& /* samples */) override {
-    set_is_playing(true);
+  TestAudioManager() {}
+  virtual ~TestAudioManager() {}
+
+  // AudioManager overrides:
+  virtual void Initialize(const DecodeSamplesCallback& decode_cb,
+                          const EncodeTokenCallback& encode_cb) override {}
+  virtual void StartPlaying(AudioType type) override { playing_[type] = true; }
+  virtual void StopPlaying(AudioType type) override { playing_[type] = false; }
+  virtual void StartRecording(AudioType type) override {
+    recording_[type] = true;
+  }
+  virtual void StopRecording(AudioType type) override {
+    recording_[type] = false;
+  }
+  virtual void SetToken(AudioType type,
+                        const std::string& url_unsafe_token) override{};

[Daniel Erat, 2014/10/21 19:43:22]

nit: "override {}"

[rkc, 2014/10/22 00:06:20]

Done.

+  virtual const std::string& get_token(AudioType type) override {
+    return token_;
   }
-  virtual void Stop() override { set_is_playing(false); }
-  virtual void Finalize() override { delete this; }
+  virtual bool IsRecording(AudioType type) override { return recording_[type]; }
+  virtual bool IsPlaying(AudioType type) override { return playing_[type]; }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(TestAudioPlayer);
-};
-
-class TestAudioRecorder : public AudioRecorder {
- public:
-  TestAudioRecorder() : AudioRecorder(AudioRecorder::DecodeSamplesCallback()) {}
-  virtual ~TestAudioRecorder() {}
+  // Indexed using enum AudioType.
+  bool playing_[2];
+  bool recording_[2];
 
-  // AudioRecorder overrides:
-  virtual void Initialize() override {}
-  virtual void Record() override { set_is_recording(true); }
-  virtual void Stop() override { set_is_recording(false); }
-  virtual void Finalize() override { delete this; }
+  std::string token_;

[Daniel Erat, 2014/10/21 19:43:22]

did you mean to set this in SetToken()? it's always empty right now.

[rkc, 2014/10/22 00:06:20]

This returns a reference pointer. It needs a real object to return. The value is
not used anywhere in tests.

 
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestAudioRecorder);
+  DISALLOW_COPY_AND_ASSIGN(TestAudioManager);
 };
 
+}  // namespace
+
 class AudioDirectiveHandlerTest : public testing::Test {
  public:
   AudioDirectiveHandlerTest()
-      : directive_handler_(new AudioDirectiveHandler(
-            AudioRecorder::DecodeSamplesCallback(),
-            base::Bind(&AudioDirectiveHandlerTest::EncodeToken,
-                       base::Unretained(this)))) {
-    directive_handler_->set_player_audible_for_testing(new TestAudioPlayer());
-    directive_handler_->set_player_inaudible_for_testing(new TestAudioPlayer());
-    directive_handler_->set_recorder_for_testing(new TestAudioRecorder());
+      : directive_handler_(new AudioDirectiveHandler()) {
+    scoped_ptr<TestAudioManager> manager(new TestAudioManager);
+    manager_ptr_ = manager.get();
+    directive_handler_->set_audio_manager_for_testing(manager.Pass());
+    directive_handler_->Initialize(
+        base::Bind(&AudioDirectiveHandlerTest::DecodeSamples,

[Daniel Erat, 2014/10/21 19:43:22]

just pass a callback around an empty function in the anon namespace if you never
intend to check that these are called?

[rkc, 2014/10/22 00:06:20]

Done.

+                   base::Unretained(this)),
+        base::Bind(&AudioDirectiveHandlerTest::EncodeToken,
+                   base::Unretained(this)));
   }
   virtual ~AudioDirectiveHandlerTest() {}
 
   void DirectiveAdded() {}
 
  protected:
-  void EncodeToken(const std::string& token,
-                   bool audible,
-                   const AudioDirectiveHandler::SamplesCallback& callback) {
-    callback.Run(
-        token, audible, CreateRandomAudioRefCounted(0x1337, 1, 0x7331));
-  }
+  // Callback stubs to pass into the directive handler.
+  void DecodeSamples(AudioType, const std::string&) {}
+  void EncodeToken(const std::string&,
+                   AudioType,
+                   const AudioManager::SamplesCallback& callback) {}
 
   copresence::TokenInstruction CreateTransmitInstruction(
       const std::string& token,
@@ -85,17 +86,24 @@ class AudioDirectiveHandlerTest : public testing::Test {
     return instruction;
   }
 
-  copresence::TokenInstruction CreateReceiveInstruction() {
+  copresence::TokenInstruction CreateReceiveInstruction(bool audible) {
     copresence::TokenInstruction instruction;
     instruction.set_token_instruction_type(copresence::RECEIVE);
+    instruction.set_medium(audible ? AUDIO_AUDIBLE_DTMF
+                                   : AUDIO_ULTRASOUND_PASSBAND);
     return instruction;
   }
 
+  bool IsPlaying(AudioType type) { return manager_ptr_->IsPlaying(type); }
+
+  bool IsRecording(AudioType type) { return manager_ptr_->IsRecording(type); }
+
   // This order is important. We want the message loop to get created before
   // our the audio directive handler since the directive list ctor (invoked
   // from the directive handler ctor) will post tasks.
   base::MessageLoop message_loop_;
   scoped_ptr<AudioDirectiveHandler> directive_handler_;
+  TestAudioManager* manager_ptr_;

[Daniel Erat, 2014/10/21 19:43:22]

nit: document that this is unowned

[rkc, 2014/10/22 00:06:20]

Done.

 
  private:
   DISALLOW_COPY_AND_ASSIGN(AudioDirectiveHandlerTest);
@@ -110,30 +118,34 @@ TEST_F(AudioDirectiveHandlerTest, Basic) {
   directive_handler_->AddInstruction(
       CreateTransmitInstruction("token", false), "op_id2", kTtl);
   directive_handler_->AddInstruction(
-      CreateReceiveInstruction(), "op_id1", kTtl);
+      CreateReceiveInstruction(false), "op_id1", kTtl);
   directive_handler_->AddInstruction(
-      CreateReceiveInstruction(), "op_id2", kTtl);
+      CreateReceiveInstruction(true), "op_id2", kTtl);
   directive_handler_->AddInstruction(
-      CreateReceiveInstruction(), "op_id3", kTtl);
+      CreateReceiveInstruction(false), "op_id3", kTtl);
 
-  EXPECT_EQ(true, directive_handler_->player_audible_->IsPlaying());
-  EXPECT_EQ(true, directive_handler_->player_inaudible_->IsPlaying());
-  EXPECT_EQ(true, directive_handler_->recorder_->IsRecording());
+  EXPECT_TRUE(IsPlaying(AUDIBLE));
+  EXPECT_TRUE(IsPlaying(INAUDIBLE));
+  EXPECT_TRUE(IsRecording(AUDIBLE));
+  EXPECT_TRUE(IsRecording(INAUDIBLE));
 
   directive_handler_->RemoveInstructions("op_id1");
-  EXPECT_FALSE(directive_handler_->player_audible_->IsPlaying());
-  EXPECT_EQ(true, directive_handler_->player_inaudible_->IsPlaying());
-  EXPECT_EQ(true, directive_handler_->recorder_->IsRecording());
+  EXPECT_FALSE(IsPlaying(AUDIBLE));
+  EXPECT_TRUE(IsPlaying(INAUDIBLE));
+  EXPECT_TRUE(IsRecording(AUDIBLE));
+  EXPECT_TRUE(IsRecording(INAUDIBLE));
 
   directive_handler_->RemoveInstructions("op_id2");
-  EXPECT_FALSE(directive_handler_->player_inaudible_->IsPlaying());
-  EXPECT_EQ(true, directive_handler_->recorder_->IsRecording());
+  EXPECT_FALSE(IsPlaying(INAUDIBLE));
+  EXPECT_FALSE(IsRecording(AUDIBLE));
+  EXPECT_TRUE(IsRecording(INAUDIBLE));
 
   directive_handler_->RemoveInstructions("op_id3");
-  EXPECT_FALSE(directive_handler_->recorder_->IsRecording());
+  EXPECT_FALSE(IsRecording(INAUDIBLE));
 }
 
 // TODO(rkc): Write more tests that check more convoluted sequences of
 // transmits/receives.
+// TODO(rkc): Write tests to move time forward and test functionality.
 
 }  // namespace copresence