Mojo interfaces needed for switching audio rendering stream creation and closing from IPC to Mojo

content/browser/media/audio_output_impl.h

Index: content/browser/media/audio_output_impl.h
diff --git a/content/browser/media/audio_output_impl.h b/content/browser/media/audio_output_impl.h
new file mode 100644
index 0000000000000000000000000000000000000000..3eae5bce95f39b671f76656fc2825a0c0a861433
--- /dev/null
+++ b/content/browser/media/audio_output_impl.h
@@ -0,0 +1,87 @@
+// Copyright 2015 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_MEDIA_AUDIO_OUTPUT_IMPL_H_
+#define CONTENT_BROWSER_MEDIA_AUDIO_OUTPUT_IMPL_H_
+
+#include "base/callback.h"
+#include "base/compiler_specific.h"
+#include "base/id_map.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "content/browser/renderer_host/media/audio_renderer_host.h"
+#include "content/common/content_export.h"
+#include "media/mojo/interfaces/audio_output.mojom.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+
+namespace base {
+class FilePath;
+}
+
+namespace content {
+
+class CONTENT_EXPORT AudioOutputStreamImpl : public mojom::AudioOutputStream {
+ public:
+  explicit AudioOutputStreamImpl(
+      mojo::InterfaceRequest<mojom::AudioOutputStream> request,
+      int stream_id,
+      AudioRendererHost::AudioEntry* entry,
+      AudioRendererHost* audio_renderer_host);
+  void Close() override;
+  ~AudioOutputStreamImpl() override;
+
+ private:
+  mojo::Binding<mojom::AudioOutputStream> binding_;
+  int stream_id_;
+  AudioRendererHost::AudioEntry* entry_;

[Henrik Grunell, 2016/04/19 15:36:08]

What's the lifetime of AudioOutputStreamImpl? I.e. is the AudioEntry and 
AudioRendererHost guaranteed to outlive it?

[rchtara, 2016/04/21 09:10:17]

|audio_renderer_host_| is scoped_refptr in AudioOutputImpl. And because all
AudioOutputStreamImpl instances are going to be owned by AudioOutputImpl,
it's safe to access |audio_renderer_host| from AudioOutputStreamImpl. The
only issue that could happen is during AudioOutputStreamImpl destructor.
|audio_renderer_host| cannot access from there.
enry is removed'

[Henrik Grunell, 2016/04/22 08:23:03]

OK, but what if the reference is released before AudioOutputStreamImpl when
AudioOutputImpl is deleted, and it's the last reference. Then AudioRendererHost
is deleted before AudioOutputStreamImpl and we could have a use after free.

+  AudioRendererHost* audio_renderer_host_;
+  DISALLOW_COPY_AND_ASSIGN(AudioOutputStreamImpl);
+};
+
+// This class must always be accessed from the creation thread.
+class CONTENT_EXPORT AudioOutputImpl
+    : NON_EXPORTED_BASE(public mojom::AudioOutput) {
+ public:
+  AudioOutputImpl(mojom::AudioOutputRequest request);
+
+  ~AudioOutputImpl() override;
+
+  static void CreateService(
+      scoped_refptr<AudioRendererHost> audio_renderer_host,
+      mojom::AudioOutputRequest request) {
+    auto service = new AudioOutputImpl(std::move(request));
+    service->audio_renderer_host_ = audio_renderer_host;
+    audio_renderer_host->audio_output_impl_ = service;
+  }
+
+  void InitMojo(mojo::InterfaceRequest<mojom::AudioOutput> request);
+
+  mojom::AudioOutputStreamPtr* StreamFactory(

[Henrik Grunell, 2016/04/19 15:36:08]

Who calls this function? Comments on the functions would be good.

[rchtara, 2016/04/21 09:10:17]

Done.

[Henrik Grunell, 2016/04/22 08:23:03]

Don't specify the function that calls it, that can change any time.

+      int stream_id,
+      AudioRendererHost::AudioEntry* entry,
+      AudioRendererHost* audio_renderer_host);
+
+ private:
+  void CreateStream(int stream_id,
+                    int render_frame_id,
+                    mojom::AudioOutputStreamParametersPtr params,
+                    const CreateStreamCallback& callback) override;
+  void DoCloseStream(int32_t id);
+
+  // Mojo connection error callback.
+  void OnDisconnect(AudioOutputImpl* impl);
+
+  scoped_refptr<AudioRendererHost> audio_renderer_host_;
+  base::ThreadChecker thread_checker_;
+  mojo::StrongBinding<AudioOutput> binding_;
+  std::map<int, scoped_ptr<AudioOutputStreamImpl>> stream_impls;

[Henrik Grunell, 2016/04/19 15:36:08]

Add trailing _ to member variable.

[rchtara, 2016/04/21 09:10:17]

Done.

+  DISALLOW_COPY_AND_ASSIGN(AudioOutputImpl);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_MEDIA_AUDIO_OUTPUT_IMPL_H_