Factor out AudioOutputDelegate from AudioRendererHost.

content/browser/renderer_host/media/audio_output_delegate.cc

Index: content/browser/renderer_host/media/audio_output_delegate.cc
diff --git a/content/browser/renderer_host/media/audio_output_delegate.cc b/content/browser/renderer_host/media/audio_output_delegate.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b88748655788abd78918f6040bdda4ba6375d458
--- /dev/null
+++ b/content/browser/renderer_host/media/audio_output_delegate.cc
@@ -0,0 +1,192 @@
+// Copyright 2016 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 <utility>
+
+#include "base/bind.h"
+#include "content/browser/media/audio_stream_monitor.h"
+#include "content/browser/media/media_internals.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace content {
+
+AudioOutputDelegate::AudioOutputDelegate(
+    EventHandler* handler,
+    media::AudioManager* audio_manager,
+    std::unique_ptr<media::AudioLog> audio_log,
+    AudioMirroringManager* mirroring_manager,
+    MediaObserver* media_observer,
+    int stream_id,
+    int render_frame_id,
+    int render_process_id,
+    const media::AudioParameters& params,
+    const std::string& output_device_id)
+    : handler_(handler),
+      audio_log_(std::move(audio_log)),
+      mirroring_manager_(mirroring_manager),
+      reader_(AudioSyncReader::Create(params)),
+      controller_(media::AudioOutputController::Create(audio_manager,
+                                                       this,
+                                                       params,
+                                                       output_device_id,
+                                                       reader_.get())),
+      stream_id_(stream_id),
+      render_frame_id_(render_frame_id),
+      render_process_id_(render_process_id),
+      weak_factory_(this) {
+  DCHECK(handler_);
+  DCHECK(audio_log_);
+  DCHECK(controller_);
+  if (media_observer)

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

Do it in Create() instead?

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

Done.

+    media_observer->OnCreatingAudioStream(render_process_id_, render_frame_id_);
+  if (mirroring_manager_)

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

Instead of keeping pointer in AudioOutputDelegate, you can probably add diverter
in Create() and remove it in Deleter? Looks like we keep the pointer in AOD to
only remove diverter in the destructor. Then making Deleter to manage that seems
to be a better separation of concerns.

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

Very well. Seems like it just makes the code more complicated though?

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

To me it looks fine. Maybe replacing Owned and lambda with Passed and a helper
function which takes ownership of diverter will simplify reading a bit. Up to
you.

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

This is fine. Another possibility: ScopedDiverterRegistration, which adds a
diverter to the mirroring manager on construction and removes it on destruction.
Sounds like over-engineering though :)

+    mirroring_manager_->AddDiverter(render_process_id_, render_frame_id_,
+                                    controller_.get());
+
+  audio_log_->OnCreated(stream_id, params, output_device_id);
+}
+
+AudioOutputDelegate::~AudioOutputDelegate() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(!playing_);
+  DCHECK(!handler_);
+
+  // De-register the controller from the AudioMirroringManager now that the
+  // controller has closed the AudioOutputStream and shut itself down.  This
+  // ensures that calling RemoveDiverter() here won't trigger the controller to
+  // re-start the default AudioOutputStream and cause a brief audio blip to come
+  // out the user's speakers.  http://crbug.com/474432
+  mirroring_manager_->RemoveDiverter(controller_.get());
+}
+
+void AudioOutputDelegate::Deleter::operator()(AudioOutputDelegate* delegate) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  delegate->UpdatePlayingState(false);
+  delegate->handler_ = nullptr;
+  delegate->audio_log_->OnClosed(delegate->stream_id_);
+  // |controller_| will call the closure (on IO thread) when it's done closing,
+  // and it is only after that call that we can delete |delegate|. By giving the
+  // closure ownership of |delegate|, we keep delegate alive until |controller_|
+  // is closed.
+  delegate->controller_->Close(
+      base::Bind([](AudioOutputDelegate*) {}, base::Owned(delegate)));
+}
+
+// static
+AudioOutputDelegate::UniquePtr AudioOutputDelegate::Create(
+    EventHandler* handler,
+    media::AudioManager* audio_manager,
+    std::unique_ptr<media::AudioLog> audio_log,
+    AudioMirroringManager* mirroring_manager,
+    MediaObserver* media_observer,
+    int stream_id,
+    int render_frame_id,
+    int render_process_id,
+    const media::AudioParameters& params,
+    const std::string& output_device_id) {
+  return UniquePtr(new AudioOutputDelegate(
+      handler, audio_manager, std::move(audio_log), mirroring_manager,
+      media_observer, stream_id, render_frame_id, render_process_id, params,
+      output_device_id));
+}
+
+void AudioOutputDelegate::OnPlayStream() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  controller_->Play();
+  audio_log_->OnStarted(stream_id_);
+}
+
+void AudioOutputDelegate::OnPauseStream() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  controller_->Pause();
+  audio_log_->OnStopped(stream_id_);
+}
+
+void AudioOutputDelegate::OnSetVolume(double volume) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  controller_->SetVolume(volume);

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

We may want to sanitize volume here
(https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/audi...)
to avoid doing it for IPC/Mojo separately.

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

Makes more sense to sanitize in the IPC layer. Ideally, this class doesn't
know/care that it's near an IPC edge. I added a DCHECK.

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

Acknowledged.

+  audio_log_->OnSetVolume(stream_id_, volume);
+}
+
+void AudioOutputDelegate::OnControllerCreated() {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&AudioOutputDelegate::OnControllerCreated,
+                   weak_factory_.GetWeakPtr()));
+    return;
+  }
+  if (!handler_)
+    return;
+
+  handler_->OnStreamCreated(stream_id_, reader_->shared_memory(),
+                            reader_->foreign_socket());
+}
+
+void AudioOutputDelegate::OnControllerPlaying() {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&AudioOutputDelegate::OnControllerPlaying,
+                   weak_factory_.GetWeakPtr()));
+    return;
+  }
+  if (!handler_)
+    return;
+
+  UpdatePlayingState(true);
+}
+
+void AudioOutputDelegate::OnControllerPaused() {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&AudioOutputDelegate::OnControllerPaused,
+                                       weak_factory_.GetWeakPtr()));
+    return;
+  }
+  if (!handler_)
+    return;
+
+  UpdatePlayingState(false);
+}
+
+void AudioOutputDelegate::OnControllerError() {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&AudioOutputDelegate::OnControllerError,
+                                       weak_factory_.GetWeakPtr()));
+    return;
+  }
+  audio_log_->OnError(stream_id_);
+
+  if (!handler_)
+    return;
+
+  handler_->OnStreamError(stream_id_);
+}
+
+void AudioOutputDelegate::UpdatePlayingState(bool playing) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(handler_);
+  if (playing != playing_) {
+    playing_ = playing;
+    handler_->OnStreamStateChanged(playing_);
+    if (playing) {
+      // Note that this takes a reference to |controller_|, and

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

(just in case you are fixing nits I pointed to: extra line before comment is not
needed in such cases)

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

Acknowledged.

+      // (Start|Stop)MonitoringStream calls are async, so we don't have a
+      // guarantee for when the controller is destroyed.
+      AudioStreamMonitor::StartMonitoringStream(
+          render_process_id_, render_frame_id_, stream_id_,
+          base::Bind(&media::AudioOutputController::ReadCurrentPowerAndClip,
+                     controller_));
+    } else {
+      AudioStreamMonitor::StopMonitoringStream(render_process_id_,
+                                               render_frame_id_, stream_id_);
+    }
+  }
+}
+
+}  // namespace content