Factor out AudioOutputDelegate from AudioRendererHost.

content/browser/renderer_host/media/audio_output_delegate.cc

+// 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,
+    media::AudioParameters params,
+    const std::string& output_device_id)
+    : handler_(handler),

[o1ka, 2016/11/03 13:39:12]

Either we need to DCHECK(handler_) or we need to have "if" before l.62 in the
deleter.

[Max Morin, 2016/11/18 16:03:42]

Done.

+      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(audio_log_);
+  DCHECK(controller_);
+  if (media_observer)
+    media_observer->OnCreatingAudioStream(render_process_id_, render_frame_id_);
+  if (mirroring_manager_)
+    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_);

[o1ka, 2016/11/03 13:39:12]

DCHECK(!handler_) probably?

[Max Morin, 2016/11/18 16:03:42]

Done.

+  AudioStreamMonitor::StopMonitoringStream(render_process_id_, render_frame_id_,

[o1ka, 2016/11/03 13:39:12]

I suspect we do not monitor at this point? Is it fine to stop twice?

[Max Morin, 2016/11/18 16:03:42]

Yes, it's fine to start monitoring while already monitoring (in this case, the
old monitoring is overwritten with the new one) and it's also fine to stop a
nonexistent monitoring (it's a thread hop followed by a no-op). It does not seem
like controller pauses the stream while closing it, so if it's not paused before
closing, it will have monitoring. To simplify it a bit, I moved the start/stop
monitoring into the same function as the one notifying handler. This also means
that monitoring is stopped when the deleter is called, so we do not need to stop
monitoring here.

+                                           stream_id_);
+
+  mirroring_manager_->RemoveDiverter(controller_.get());

[o1ka, 2016/11/03 13:39:12]

Please restore the comment on why diverter is removed here.

[Max Morin, 2016/11/18 16:03:42]

Done.

+}
+
+void AudioOutputDelegate::Deleter::operator()(AudioOutputDelegate* delegate) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  delegate->UpdatePlayingStateForHandler(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

[o1ka, 2016/11/03 13:39:12]

nit: add a line before

[Max Morin, 2016/11/18 16:03:42]

Done.

+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,
+    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);
+  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;
+
+  UpdatePlayingStateForHandler(true);
+
+  AudioStreamMonitor::StartMonitoringStream(

[o1ka, 2016/11/03 13:39:12]

We want to do it only if we switched from pause to play, right?

[Max Morin, 2016/11/18 16:03:42]

Done.

+      render_process_id_, render_frame_id_, stream_id_,
+      base::Bind(&media::AudioOutputController::ReadCurrentPowerAndClip,
+                 controller_));

[o1ka, 2016/11/03 13:39:12]

I would add an explicit comment that AudioStreamMonitor will hold a ref ptr to
the controller (So, basically, we never know when the controller is deleted:
note that "StopMonitoring..." is async - which probably also makes sense to
mention in the destructor).

[Max Morin, 2016/11/18 16:03:42]

Good point. We don't have to worry too much since the controller doesn't send
notifications when closed, but it's good to keep its lifetime in mind.

+}
+
+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;
+
+  UpdatePlayingStateForHandler(false);
+
+  AudioStreamMonitor::StopMonitoringStream(render_process_id_, render_frame_id_,

[o1ka, 2016/11/03 13:39:12]

same here: if we do not monitor it, do we want to stop?

[Max Morin, 2016/11/18 16:03:42]

Done.

+                                           stream_id_);
+}
+
+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::UpdatePlayingStateForHandler(bool playing) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(handler_);
+  if (playing != playing_) {
+    playing_ = playing;
+    handler_->OnStreamStateChanged(playing_);
+  }
+}
+
+}  // namespace content