Ensure asynchronous reply from MojoAudioOutputIPC.

AudioOutputDevice currently expects all responses to be asynchronous,
and this change also simplifies the code a bit.

Bug: 816348
Change-Id: I66d51184f2553043578dda17a5cb7aeebb809a20
Reviewed-on: https://chromium-review.googlesource.com/937721
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539915}

content/renderer/media/mojo_audio_output_ipc.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "media/audio/audio_device_description.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/system/platform_handle.h"
@@ -71,11 +72,9 @@ void MojoAudioOutputIPC::CreateStream(media::AudioOutputIPCDelegate* delegate,
     // No authorization requested yet. Request one for the default device.
     // Since the delegate didn't explicitly request authorization, we shouldn't
     // send a callback to it.
-    if (!DoRequestDeviceAuthorization(
-            0, media::AudioDeviceDescription::kDefaultDeviceId,
-            base::BindOnce(&TrivialAuthorizedCallback))) {
-      return;
-    }
+    DoRequestDeviceAuthorization(
+        0, media::AudioDeviceDescription::kDefaultDeviceId,
+        base::BindOnce(&TrivialAuthorizedCallback));
   }
 
   DCHECK_EQ(delegate_, delegate);
@@ -90,7 +89,7 @@ void MojoAudioOutputIPC::CreateStream(media::AudioOutputIPCDelegate* delegate,
 
   // Don't set a connection error handler. Either an error has already been
   // signaled through the AudioOutputStreamClient interface, or the connection
-  // is broken because because the frame owning |this| was destroyed, in which
+  // is broken because the frame owning |this| was destroyed, in which
   // case |this| will soon be cleaned up anyways.
 }
 
@@ -158,7 +157,7 @@ MojoAudioOutputIPC::MakeProviderRequest() {
   return request;
 }
 
-bool MojoAudioOutputIPC::DoRequestDeviceAuthorization(
+void MojoAudioOutputIPC::DoRequestDeviceAuthorization(
     int session_id,
     const std::string& device_id,
     AuthorizationCB callback) {
@@ -167,20 +166,22 @@ bool MojoAudioOutputIPC::DoRequestDeviceAuthorization(
   if (!factory) {
     LOG(ERROR) << "MojoAudioOutputIPC failed to acquire factory";
 
-    // Resetting the callback ensures consistent behaviour with when the factory
-    // is destroyed before reply, i.e. calling OnDeviceAuthorized with
-    // ERROR_INTERNAL in the normal case. Note that this operation might destroy
-    // |this|. The AudioOutputIPCDelegate will call CloseStream as necessary.
-    callback.Reset();
-    // As |this| may be deleted, no new lines may be added here.
-    return false;
+    // Create a provider request for consistency with the normal case.
+    MakeProviderRequest();
+    // Resetting the callback asynchronously ensures consistent behaviour with
+    // when the factory is destroyed before reply, i.e. calling
+    // OnDeviceAuthorized with ERROR_INTERNAL in the normal case.
+    // The AudioOutputIPCDelegate will call CloseStream as necessary.
+    base::SequencedTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce([](AuthorizationCB cb) {}, std::move(callback)));
+    return;
   }
 
   static_assert(sizeof(int) == sizeof(int32_t),
                 "sizeof(int) == sizeof(int32_t)");
   factory->RequestDeviceAuthorization(MakeProviderRequest(), session_id,
                                       device_id, std::move(callback));
-  return true;
 }
 
 void MojoAudioOutputIPC::ReceivedDeviceAuthorization(

content/renderer/media/mojo_audio_output_ipc.h

@@ -58,10 +58,10 @@ class CONTENT_EXPORT MojoAudioOutputIPC
   bool StreamCreationRequested();
   media::mojom::AudioOutputStreamProviderRequest MakeProviderRequest();
 
-  // Tries to acquire a RendererAudioOutputStreamFactory, returns true on
-  // success. On failure, |this| has been deleted, so returning immediately
-  // is required.
-  bool DoRequestDeviceAuthorization(int session_id,
+  // Tries to acquire a RendererAudioOutputStreamFactory and requests device
+  // authorization. On failure to aquire a factory, |callback| is destructed
+  // asynchronously.
+  void DoRequestDeviceAuthorization(int session_id,
                                     const std::string& device_id,
                                     AuthorizationCB callback);
 

content/renderer/media/mojo_audio_output_ipc_unittest.cc

@@ -207,16 +207,31 @@ TEST(MojoAudioOutputIPC, AuthorizeWithoutFactory_CallsAuthorizedWithError) {
   std::unique_ptr<media::AudioOutputIPC> ipc =
       std::make_unique<MojoAudioOutputIPC>(NullAccessor());
 
+  ipc->RequestDeviceAuthorization(&delegate, kSessionId, kDeviceId, Origin());
+
+  // Don't call OnDeviceAuthorized synchronously, should wait until we run the
+  // RunLoop.
   EXPECT_CALL(delegate,
               OnDeviceAuthorized(media::OUTPUT_DEVICE_STATUS_ERROR_INTERNAL, _,
-                                 std::string()))
-      .WillOnce(Invoke([&](media::OutputDeviceStatus,
-                           const media::AudioParameters&, const std::string&) {
-        ipc->CloseStream();
-        ipc.reset();
-      }));
-  ipc->RequestDeviceAuthorization(&delegate, kSessionId, kDeviceId, Origin());
+                                 std::string()));
+  base::RunLoop().RunUntilIdle();
+  ipc->CloseStream();
+}
+
+TEST(MojoAudioOutputIPC,
+     CreateWithoutAuthorizationWithoutFactory_CallsAuthorizedWithError) {
+  base::MessageLoopForIO message_loop;
+  StrictMock<MockDelegate> delegate;
+
+  std::unique_ptr<media::AudioOutputIPC> ipc =
+      std::make_unique<MojoAudioOutputIPC>(NullAccessor());
+
+  ipc->CreateStream(&delegate, Params());
+
+  // No call to OnDeviceAuthorized since authotization wasn't explicitly
+  // requested.
   base::RunLoop().RunUntilIdle();
+  ipc->CloseStream();
 }
 
 TEST(MojoAudioOutputIPC, DeviceAuthorized_Propagates) {