Factor out AudioOutputDelegate from AudioRendererHost.

content/browser/renderer_host/media/audio_output_delegate.h

+// 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.
+
+#ifndef CONTENT_BROWSER_RENDERER_HOST_MEDIA_AUDIO_OUTPUT_DELEGATE_H_
+#define CONTENT_BROWSER_RENDERER_HOST_MEDIA_AUDIO_OUTPUT_DELEGATE_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/shared_memory.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sync_socket.h"

[DaleCurtis, 2016/11/29 20:31:23]

Forward declare. This and almost all the others.
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

+#include "content/browser/media/capture/audio_mirroring_manager.h"
+#include "content/browser/renderer_host/media/audio_sync_reader.h"
+#include "content/public/browser/media_observer.h"
+#include "media/audio/audio_logging.h"
+#include "media/audio/audio_output_controller.h"
+#include "media/base/audio_parameters.h"
+
+namespace content {
+
+// This class is operated on the IO thread.
+class CONTENT_EXPORT AudioOutputDelegate
+    : private media::AudioOutputController::EventHandler {

[DaleCurtis, 2016/11/29 20:31:23]

Google C++ only allows public inheritance:
https://google.github.io/styleguide/cppguide.html#Inheritance

+ public:
+  class EventHandler {

[DaleCurtis, 2016/11/29 20:31:23]

This must have a virtual destructor:
https://google.github.io/styleguide/cppguide.html#Interfaces

+   public:
+    // All these methods are called on the IO thread.
+
+    // Called when the state changes between playing and not playing.
+    virtual void OnStreamStateChanged(bool playing) = 0;
+
+    // Called when construction is finished and the stream is ready for
+    // playout.
+    virtual void OnStreamCreated(int stream_id,
+                                 base::SharedMemory* shared_memory,
+                                 base::CancelableSyncSocket* socket) = 0;
+
+    // Called if stream encounters an error and has become unusable.
+    virtual void OnStreamError(int stream_id) = 0;
+  };
+
+  class Deleter {
+   public:
+    Deleter() = default;
+    explicit Deleter(AudioMirroringManager* mirroring_manager)
+        : mirroring_manager_(mirroring_manager) {}
+    void operator()(AudioOutputDelegate* delegate);
+
+   private:
+    AudioMirroringManager* mirroring_manager_;
+  };
+
+  using UniquePtr = std::unique_ptr<AudioOutputDelegate, Deleter>;

[DaleCurtis, 2016/11/29 20:31:23]

Needs a more "unique" name, though since this is only used in a handful of
places, it seems fine to always specify it.

[Max Morin, 2016/12/01 16:08:37]

How about AudioOutputDelegate::ScopedPointer, similar to
ScopedAudioManagerPointer? I think std::unique_ptr<AudioOutputDelegate,
AudioOutputDelegate::Deleter> is too long.

[DaleCurtis, 2016/12/01 19:33:06]

Hmm, Scoped shouldn't be used anymore for this. I was going to suggest
AudioOutputDelegateUniquePtr, but I guess that's not any better than
AudioOutputDelegate::UniquePtr except it's always qualified. Up to you.

+
+  // |handler| must be valid until the moment that the UniquePtr destructor

[DaleCurtis, 2016/11/29 20:31:23]

Needs more comments.

+  // finishes.
+  static UniquePtr Create(EventHandler* handler,

[DaleCurtis, 2016/11/29 20:31:23]

It's sad there are so many parameters here. I don't have any obvious ideas for
removal, but it's worth thinking about.

+                          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);
+  ~AudioOutputDelegate() override;
+
+  // TODO(maxmorin): Remove this when crbug.com/647185 is closed.
+  // This function is used to provide control of the audio stream to
+  // WebrtcAudioPrivateGetActiveSinkFunction and others in the webrtc extension
+  // API. Since the controller is shared, this means that it might outlive the
+  // AudioOutputDelegate. In this case, it is still safe to call functions on
+  // the controller, but it will not do anything. The controller is also shared
+  // with AudioStreamMonitor.
+  const scoped_refptr<media::AudioOutputController>& controller() const {

[DaleCurtis, 2016/11/29 20:31:23]

Shouldn't this be returned by value? or a raw ptr if it's not expected to own.

[miu, 2016/11/29 21:36:27]

Hmm...I don't like the shared-ownership model here, but it seems we'll be
correcting that soon. Based on style guidelines, #2 applies:

1. Raw pointer if caller is not expected to take ownership.

2. scoped_refptr<...> (no const, not by ref) if caller takes ownership.

[Max Morin, 2016/12/01 16:08:37]

Right, no const and no & (not changing to raw pointer since the dependency needs
scoped_refptr). Not having AudioOutputController as reference counted at all
would be nice, but depends on the bug mentioned. Ownership is also shared with
AudioStreamMonitor (bound in callback), but I think that can be fixed if we want
to.

+    return controller_;
+  }
+
+  int stream_id() const { return stream_id_; }
+
+  // Stream control:
+  void OnPlayStream();
+  void OnPauseStream();
+  void OnSetVolume(double volume);
+
+ private:
+  AudioOutputDelegate(EventHandler* handler,
+                      media::AudioManager* audio_manager,
+                      std::unique_ptr<media::AudioLog> audio_log,
+                      int stream_id,
+                      int render_frame_id,
+                      int render_process_id,
+                      const media::AudioParameters& params,
+                      const std::string& output_device_id);
+
+  // AudioOutputController::EventHandler implementation.
+  void OnControllerCreated() override;
+  void OnControllerPlaying() override;
+  void OnControllerPaused() override;
+  void OnControllerError() override;
+
+  void UpdatePlayingState(bool playing);
+
+  // |handler_| is null if we are in the process of destruction. In this case,
+  // we will ignore events from |controller_|.
+  EventHandler* handler_;
+  std::unique_ptr<media::AudioLog> const audio_log_;
+  std::unique_ptr<AudioSyncReader> reader_;
+  scoped_refptr<media::AudioOutputController> controller_;
+  const int stream_id_;
+  const int render_frame_id_;
+  const int render_process_id_;
+
+  // This flag ensures that we only send OnStreamStateChanged notifications
+  // and (de)register with the stream monitor when the state actually changes.
+  bool playing_ = false;
+  base::WeakPtrFactory<AudioOutputDelegate> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(AudioOutputDelegate);
+};
+}  // namespace content

[DaleCurtis, 2016/11/29 20:31:23]

Add line above.

+
+#endif  // CONTENT_BROWSER_RENDERER_HOST_MEDIA_AUDIO_OUTPUT_DELEGATE_H_