Factor out AudioOutputDelegate from AudioRendererHost.

content/browser/renderer_host/media/audio_output_delegate_unittest.cc

Index: content/browser/renderer_host/media/audio_output_delegate_unittest.cc
diff --git a/content/browser/renderer_host/media/audio_output_delegate_unittest.cc b/content/browser/renderer_host/media/audio_output_delegate_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7154e22f268b6666843c446746a3809688c55eca
--- /dev/null
+++ b/content/browser/renderer_host/media/audio_output_delegate_unittest.cc
@@ -0,0 +1,379 @@
+// Copyright (c) 2012 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.
+
+#include "content/browser/renderer_host/media/audio_output_delegate.h"
+
+#include <stdint.h>
+
+#include <memory>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/run_loop.h"
+#include "content/browser/media/capture/audio_mirroring_manager.h"
+#include "content/browser/renderer_host/media/media_stream_manager.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "media/audio/fake_audio_log_factory.h"
+#include "media/audio/fake_audio_manager.h"
+#include "media/base/media_switches.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::InSequence;
+using ::testing::NiceMock;
+using ::testing::NotNull;
+
+// Not tested:

[o1ka, 2016/11/22 13:24:03]

add "yet", it will look cooler ;)

[Max Morin, 2016/11/22 16:04:29]

Done.

+// - Interactions with AudioStreamMonitor (goes through WebContentsImpl,
+//   so it's a bit tricky).
+// - Logging (small risk of bugs, not worth the effort).
+// - That the returned socket/memory is correctly set up.
+
+namespace content {
+
+namespace {
+
+const int kRenderProcessId = 1;
+const int kRenderFrameId = 5;
+const int kStreamId = 50;
+const char kDefaultDeviceId[] = "";
+
+struct MockAudioMirroringManager : public AudioMirroringManager {
+  MockAudioMirroringManager() : AudioMirroringManager() {}

[o1ka, 2016/11/22 13:24:03]

just =default?

[Max Morin, 2016/11/22 16:04:29]

Done.

+  virtual ~MockAudioMirroringManager() {}

[o1ka, 2016/11/22 13:24:03]

override instead

[Max Morin, 2016/11/22 16:04:29]

Done.

+
+  MOCK_METHOD3(AddDiverter,
+               void(int render_process_id,
+                    int render_frame_id,
+                    Diverter* diverter));
+  MOCK_METHOD1(RemoveDiverter, void(Diverter* diverter));
+};
+
+struct MockObserver : public content::MediaObserver {
+  void OnAudioCaptureDevicesChanged() override {}
+  void OnVideoCaptureDevicesChanged() override {}
+  void OnMediaRequestStateChanged(int render_process_id,
+                                  int render_frame_id,
+                                  int page_request_id,
+                                  const GURL& security_origin,
+                                  MediaStreamType stream_type,
+                                  MediaRequestState state) override {}
+  void OnSetCapturingLinkSecured(int render_process_id,
+                                 int render_frame_id,
+                                 int page_request_id,
+                                 MediaStreamType stream_type,
+                                 bool is_secure) override {}
+
+  MOCK_METHOD2(OnCreatingAudioStream,
+               void(int render_process_id, int render_frame_id));
+};
+
+struct MockEventHandler : public AudioOutputDelegate::EventHandler {
+  MOCK_METHOD1(OnStreamStateChanged, void(bool playing));
+  MOCK_METHOD3(OnStreamCreated,
+               void(int stream_id,
+                    base::SharedMemory* shared_memory,
+                    base::CancelableSyncSocket* socket));
+  MOCK_METHOD1(OnStreamError, void(int stream_id));
+};
+
+}  // namespace
+
+class AudioOutputDelegateTest : public testing::Test {
+ public:
+  AudioOutputDelegateTest() {
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kUseFakeDeviceForMediaStream);
+
+    thread_bundle_ = base::MakeUnique<TestBrowserThreadBundle>(
+        TestBrowserThreadBundle::Options::REAL_IO_THREAD);
+    audio_thread_ = base::MakeUnique<base::Thread>("AudioThread");
+
+// Audio manager creation stolen from content/browser/browser_main_loop.cc.
+#if defined(OS_WIN)
+    audio_thread_->init_com_with_mta(true);
+#endif  // defined(OS_WIN)
+    CHECK(audio_thread_->Start());
+
+#if defined(OS_MACOSX)
+    scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner =
+        base::ThreadTaskRunnerHandle::Get();
+#else
+    scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner =
+        audio_thread_->task_runner();
+#endif  // defined(OS_MACOSX)
+
+    audio_manager_.reset(new media::FakeAudioManager(
+        audio_task_runner, audio_thread_->task_runner(), &log_factory_));
+    media_stream_manager_ =
+        base::MakeUnique<MediaStreamManager>(audio_manager_.get());
+  }
+
+  ~AudioOutputDelegateTest() override {}
+
+  AudioOutputDelegate::UniquePtr CreateDelegate() {
+    AudioOutputDelegate::UniquePtr delegate;
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&AudioOutputDelegateTest::CreateDelegateOnIO,
+                   base::Unretained(this), base::Unretained(&delegate)));
+    SyncWithAllThreads();
+    return delegate;
+  }
+
+  void CreateDelegateOnIO(AudioOutputDelegate::UniquePtr* delegate) {
+    *delegate = AudioOutputDelegate::Create(
+        &event_handler_, audio_manager_.get(),
+        log_factory_.CreateAudioLog(
+            media::AudioLogFactory::AUDIO_OUTPUT_CONTROLLER),
+        &mirroring_manager_, &media_observer_, kStreamId, kRenderFrameId,
+        kRenderProcessId, audio_manager_->GetDefaultOutputStreamParameters(),
+        kDefaultDeviceId);
+  }
+
+ protected:
+  void SyncWithAllThreads() {
+    // New tasks might be posted while we are syncing, but in every iteration at
+    // least one task will be run. 20 iterations should be enough for our code.
+    for (int i = 0; i < 20; ++i) {
+      base::RunLoop().RunUntilIdle();
+      SyncWith(BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
+      SyncWith(audio_thread_->task_runner());
+    }
+  }
+
+  // MediaStreamManager uses a DestructionObserver, so it must outlive the
+  // TestBrowserThreadBundle.
+  std::unique_ptr<MediaStreamManager> media_stream_manager_;
+  std::unique_ptr<TestBrowserThreadBundle> thread_bundle_;
+  std::unique_ptr<base::Thread> audio_thread_;
+  media::ScopedAudioManagerPtr audio_manager_;
+  NiceMock<MockAudioMirroringManager> mirroring_manager_;

[o1ka, 2016/11/22 13:24:04]

Question: why NiceMocks are needed here?

[Max Morin, 2016/11/22 16:04:29]

The test output is hard to read if there is a bunch of gmock spam.

[o1ka, 2016/11/22 21:48:16]

But should not you set all the appropriate expectations instead?

[Max Morin, 2016/11/25 15:32:35]

Ok then. In my opinion, this makes it less clear what each test case is really
supposed to test.

+  NiceMock<MockEventHandler> event_handler_;
+  NiceMock<MockObserver> media_observer_;
+  media::FakeAudioLogFactory log_factory_;
+
+ private:
+  void SyncWith(scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+    CHECK(task_runner);
+    CHECK(!task_runner->BelongsToCurrentThread());
+    base::WaitableEvent e = {base::WaitableEvent::ResetPolicy::MANUAL,
+                             base::WaitableEvent::InitialState::NOT_SIGNALED};
+    task_runner->PostTask(FROM_HERE, base::Bind(&base::WaitableEvent::Signal,
+                                                base::Unretained(&e)));
+    e.Wait();
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(AudioOutputDelegateTest);
+};
+
+TEST_F(AudioOutputDelegateTest, Create_AlertsObserver) {
+  EXPECT_CALL(media_observer_,
+              OnCreatingAudioStream(kRenderProcessId, kRenderFrameId))
+      .Times(1);
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();

[o1ka, 2016/11/22 13:24:03]

On which thread delegates are created in real life? I suspect it's on IO thread.

[Max Morin, 2016/11/22 16:04:29]

Yes. This method actually created them on the IO thread. I renamed the function.

+  // Delete delegate on IO thread:
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, Create_CallsHandler) {
+  EXPECT_CALL(event_handler_, OnStreamCreated(kStreamId, NotNull(), NotNull()));
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest,
+       Create_RegistersAndUnregistersControllerWithMirroringManager) {
+  {
+    InSequence s;
+    EXPECT_CALL(mirroring_manager_,
+                AddDiverter(kRenderProcessId, kRenderFrameId, NotNull()))
+        .Times(1);
+    EXPECT_CALL(mirroring_manager_, RemoveDiverter(NotNull())).Times(1);
+  }
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, Play_CallsHandler) {
+  {
+    InSequence s;
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(true)).Times(1);
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(false)).Times(1);
+  }
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPlayStream,
+                                     base::Unretained(delegate.get())));
+  SyncWithAllThreads();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, PlayAndDestroy_DoesNotCallHandler) {
+  EXPECT_CALL(event_handler_, OnStreamStateChanged(_)).Times(0);
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(
+                              [](AudioOutputDelegate::UniquePtr delegate) {
+                                delegate->OnPlayStream();
+                                delegate.reset();
+                              },
+                              base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, Error_CallsHandler) {
+  EXPECT_CALL(event_handler_, OnStreamError(kStreamId)).Times(1);
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&media::AudioOutputController::OnError,
+                                     delegate->controller(), nullptr));
+  SyncWithAllThreads();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, ErrorAndDestroy_DoesNotCallHandler) {
+  EXPECT_CALL(event_handler_, OnStreamError(kStreamId)).Times(0);
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(
+                              [](AudioOutputDelegate::UniquePtr delegate) {
+                                delegate->controller()->OnError(nullptr);
+                                delegate.reset();
+                              },
+                              base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, PlayPause_CallsHandlerCorrectNumberOfTimes) {
+  {
+    InSequence s;
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(true)).Times(1);
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(false)).Times(1);
+  }
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPlayStream,
+                                     base::Unretained(delegate.get())));
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPauseStream,
+                                     base::Unretained(delegate.get())));
+  SyncWithAllThreads();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest,
+       PlayPausePlay_CallsHandlerCorrectNumberOfTimes) {
+  {
+    InSequence s;
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(true)).Times(1);
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(false)).Times(1);
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(true)).Times(1);
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(false)).Times(1);
+  }
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPlayStream,
+                                     base::Unretained(delegate.get())));
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPauseStream,
+                                     base::Unretained(delegate.get())));
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPlayStream,
+                                     base::Unretained(delegate.get())));
+  SyncWithAllThreads();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, PlayPlay_CallsHandlerCorrectNumberOfTimes) {
+  {
+    InSequence s;
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(true)).Times(1);
+    EXPECT_CALL(event_handler_, OnStreamStateChanged(false)).Times(1);
+  }
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPlayStream,
+                                     base::Unretained(delegate.get())));
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPlayStream,
+                                     base::Unretained(delegate.get())));
+  SyncWithAllThreads();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, Pause_DoesNotCallHandler) {

[o1ka, 2016/11/22 13:24:03]

Naming needs clarification

[Max Morin, 2016/11/22 16:04:29]

Done.

+  EXPECT_CALL(event_handler_, OnStreamStateChanged(_)).Times(0);
+
+  AudioOutputDelegate::UniquePtr delegate = CreateDelegate();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&AudioOutputDelegate::OnPauseStream,
+                                     base::Unretained(delegate.get())));
+  SyncWithAllThreads();
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind([](AudioOutputDelegate::UniquePtr delegate) {},
+                 base::Passed(std::move(delegate))));
+  SyncWithAllThreads();
+}
+
+TEST_F(AudioOutputDelegateTest, CreateAndDestroy_DoesNotCallHandler) {
+  EXPECT_CALL(event_handler_, OnStreamCreated(_, _, _)).Times(0);
+
+  AudioOutputDelegate::UniquePtr delegate;
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(
+                              [](AudioOutputDelegateTest* test) {
+                                AudioOutputDelegate::UniquePtr delegate;
+                                test->CreateDelegateOnIO(&delegate);
+                              },
+                              base::Unretained(this)));
+  SyncWithAllThreads();
+}
+}  // namespace content

[o1ka, 2016/11/22 13:24:03]

I'm not sure if various cases of destruction sequence are covered - and
destruction is the most sensitive part of the code here.

[Max Morin, 2016/11/22 16:04:29]

There are cases for various events being in progress (play, pause, create,
error) as well as checks for all the handlers during creation and destruction (I
merged those tests into a test "CreateAndDelete"). There is only one case of the
destruction sequence (disregarding null checks of the delegate and the mirroring
manager, those will never be null anyways).