Implementing AudioOutputDevice authorization timeout by scheduling a …

…delayed cancelable AudioOutputDevice::OnDeviceAuthorized() call.

BUG=615589

Review-Url: https://codereview.chromium.org/2043883005
Cr-Commit-Position: refs/heads/master@{#399685}
(cherry picked from commit 698458b)

Review URL: https://codereview.chromium.org/2090863003 .

Cr-Commit-Position: refs/branch-heads/2743@{#456}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}

content/renderer/media/audio_device_factory.cc

@@ -4,7 +4,10 @@
 
 #include "content/renderer/media/audio_device_factory.h"
 
+#include <algorithm>
+
 #include "base/logging.h"
+#include "content/common/content_constants_internal.h"
 #include "content/renderer/media/audio_input_message_filter.h"
 #include "content/renderer/media/audio_message_filter.h"
 #include "content/renderer/media/audio_renderer_mixer_manager.h"
@@ -20,6 +23,7 @@ namespace content {
 AudioDeviceFactory* AudioDeviceFactory::factory_ = NULL;
 
 namespace {
+const int64_t kMaxAuthorizationTimeoutMs = 4000;
 
 scoped_refptr<media::AudioOutputDevice> NewOutputDevice(
     int render_frame_id,
@@ -29,7 +33,11 @@ scoped_refptr<media::AudioOutputDevice> NewOutputDevice(
   AudioMessageFilter* const filter = AudioMessageFilter::Get();
   scoped_refptr<media::AudioOutputDevice> device(new media::AudioOutputDevice(
       filter->CreateAudioOutputIPC(render_frame_id), filter->io_task_runner(),
-      session_id, device_id, security_origin));
+      session_id, device_id, security_origin,
+      // Set authorization request timeout at 80% of renderer hung timeout, but
+      // no more than kMaxAuthorizationTimeout.
+      base::TimeDelta::FromMilliseconds(std::min(kHungRendererDelayMs * 8 / 10,
+                                                 kMaxAuthorizationTimeoutMs))));
   device->RequestDeviceAuthorization();
   return device;
 }

media/audio/audio_output_device.cc

@@ -12,8 +12,10 @@
 
 #include "base/callback_helpers.h"
 #include "base/macros.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "media/audio/audio_device_description.h"
@@ -52,7 +54,8 @@ AudioOutputDevice::AudioOutputDevice(
     const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
     int session_id,
     const std::string& device_id,
-    const url::Origin& security_origin)
+    const url::Origin& security_origin,
+    base::TimeDelta authorization_timeout)
     : ScopedTaskRunnerObserver(io_task_runner),
       callback_(NULL),
       ipc_(std::move(ipc)),
@@ -64,7 +67,8 @@ AudioOutputDevice::AudioOutputDevice(
       security_origin_(security_origin),
       stopping_hack_(false),
       did_receive_auth_(true, false),
-      device_status_(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL) {
+      device_status_(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL),
+      auth_timeout_(authorization_timeout) {
   CHECK(ipc_);
 
   // The correctness of the code depends on the relative values assigned in the
@@ -155,6 +159,16 @@ void AudioOutputDevice::RequestDeviceAuthorizationOnIOThread() {
   state_ = AUTHORIZING;
   ipc_->RequestDeviceAuthorization(this, session_id_, device_id_,
                                    security_origin_);
+
+  // Create the timer on the thread it's used on. It's guaranteed to be
+  // deleted on the same thread since users must call Stop() before deleting
+  // AudioOutputDevice; see ShutDownOnIOThread().
+  auth_timeout_action_.reset(new base::OneShotTimer());
+  auth_timeout_action_->Start(
+      FROM_HERE, auth_timeout_,
+      base::Bind(&AudioOutputDevice::OnDeviceAuthorized, this,
+                 OUTPUT_DEVICE_STATUS_ERROR_TIMED_OUT, media::AudioParameters(),
+                 std::string()));
 }
 
 void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
@@ -228,6 +242,9 @@ void AudioOutputDevice::ShutDownOnIOThread() {
   }
   start_on_authorized_ = false;
 
+  // Destoy the timer on the thread it's used on.
+  auth_timeout_action_.reset();
+
   // We can run into an issue where ShutDownOnIOThread is called right after
   // OnStreamCreated is called in cases where Start/Stop are called before we
   // get the OnStreamCreated callback.  To handle that corner case, we call
@@ -284,6 +301,16 @@ void AudioOutputDevice::OnDeviceAuthorized(
     const media::AudioParameters& output_params,
     const std::string& matched_device_id) {
   DCHECK(task_runner()->BelongsToCurrentThread());
+
+  auth_timeout_action_.reset();
+
+  // Do nothing if late authorization is received after timeout.
+  if (state_ == IPC_CLOSED)
+    return;
+
+  UMA_HISTOGRAM_BOOLEAN("Media.Audio.Render.OutputDeviceAuthorizationTimedOut",
+                        device_status == OUTPUT_DEVICE_STATUS_ERROR_TIMED_OUT);
+
   DCHECK_EQ(state_, AUTHORIZING);
 
   // It may happen that a second authorization is received as a result to a

media/audio/audio_output_device.h

@@ -77,6 +77,10 @@
 #include "media/base/media_export.h"
 #include "media/base/output_device_info.h"
 
+namespace base {
+class OneShotTimer;
+}
+
 namespace media {
 
 class MEDIA_EXPORT AudioOutputDevice
@@ -90,7 +94,8 @@ class MEDIA_EXPORT AudioOutputDevice
       const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
       int session_id,
       const std::string& device_id,
-      const url::Origin& security_origin);
+      const url::Origin& security_origin,
+      base::TimeDelta authorization_timeout);
 
   // Request authorization to use the device specified in the constructor.
   void RequestDeviceAuthorization();
@@ -145,6 +150,13 @@ class MEDIA_EXPORT AudioOutputDevice
   void ShutDownOnIOThread();
   void SetVolumeOnIOThread(double volume);
 
+  // Process device authorization result on the IO thread.
+  void ProcessDeviceAuthorizationOnIOThread(
+      OutputDeviceStatus device_status,
+      const media::AudioParameters& output_params,
+      const std::string& matched_device_id,
+      bool timed_out);
+
   // base::MessageLoop::DestructionObserver implementation for the IO loop.
   // If the IO loop dies before we do, we shut down the audio thread from here.
   void WillDestroyCurrentMessageLoop() override;
@@ -201,6 +213,9 @@ class MEDIA_EXPORT AudioOutputDevice
   AudioParameters output_params_;
   OutputDeviceStatus device_status_;
 
+  const base::TimeDelta auth_timeout_;
+  std::unique_ptr<base::OneShotTimer> auth_timeout_action_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioOutputDevice);
 };
 

media/audio/audio_output_device_unittest.cc

@@ -17,9 +17,11 @@
 #include "base/sync_socket.h"
 #include "base/task_runner.h"
 #include "base/test/test_timeouts.h"
+#include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "media/audio/audio_output_device.h"
 #include "media/audio/sample_rates.h"
+#include "media/base/test_helpers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -42,6 +44,7 @@ namespace {
 const char kDefaultDeviceId[] = "";
 const char kNonDefaultDeviceId[] = "valid-nondefault-device-id";
 const char kUnauthorizedDeviceId[] = "unauthorized-device-id";
+const int kAuthTimeoutForTestingMs = 500;
 
 class MockRenderCallback : public AudioRendererSink::RenderCallback {
  public:
@@ -99,7 +102,9 @@ class AudioOutputDeviceTest
   void ExpectRenderCallback();
   void WaitUntilRenderCallback();
   void StopAudioDevice();
+  void CreateDevice(const std::string& device_id);
   void SetDevice(const std::string& device_id);
+  void CheckDeviceStatus(OutputDeviceStatus device_status);
 
  protected:
   // Used to clean up TLS pointers that the test(s) will initialize.
@@ -139,11 +144,16 @@ AudioOutputDeviceTest::~AudioOutputDeviceTest() {
   audio_device_ = NULL;
 }
 
-void AudioOutputDeviceTest::SetDevice(const std::string& device_id) {
+void AudioOutputDeviceTest::CreateDevice(const std::string& device_id) {
   audio_output_ipc_ = new MockAudioOutputIPC();
-  audio_device_ = new AudioOutputDevice(base::WrapUnique(audio_output_ipc_),
-                                        io_loop_.task_runner(), 0, device_id,
-                                        url::Origin());
+  audio_device_ = new AudioOutputDevice(
+      base::WrapUnique(audio_output_ipc_), io_loop_.task_runner(), 0, device_id,
+      url::Origin(),
+      base::TimeDelta::FromMilliseconds(kAuthTimeoutForTestingMs));
+}
+
+void AudioOutputDeviceTest::SetDevice(const std::string& device_id) {
+  CreateDevice(device_id);
   EXPECT_CALL(*audio_output_ipc_,
               RequestDeviceAuthorization(audio_device_.get(), 0, device_id, _));
   audio_device_->RequestDeviceAuthorization();
@@ -160,6 +170,11 @@ void AudioOutputDeviceTest::SetDevice(const std::string& device_id) {
                             &callback_);
 }
 
+void AudioOutputDeviceTest::CheckDeviceStatus(OutputDeviceStatus status) {
+  DCHECK(!io_loop_.task_runner()->BelongsToCurrentThread());
+  EXPECT_EQ(status, audio_device_->GetOutputDeviceInfo().device_status());
+}
+
 void AudioOutputDeviceTest::ReceiveAuthorization(OutputDeviceStatus status) {
   device_status_ = status;
   if (device_status_ != OUTPUT_DEVICE_STATUS_OK)
@@ -318,6 +333,37 @@ TEST_P(AudioOutputDeviceTest, UnauthorizedDevice) {
   StopAudioDevice();
 }
 
+TEST_P(AudioOutputDeviceTest, AuthorizationTimedOut) {
+  base::Thread thread("DeviceInfo");
+  thread.Start();
+
+  CreateDevice(kNonDefaultDeviceId);
+  EXPECT_CALL(*audio_output_ipc_,
+              RequestDeviceAuthorization(audio_device_.get(), 0,
+                                         kNonDefaultDeviceId, _));
+  EXPECT_CALL(*audio_output_ipc_, CloseStream());
+
+  // Request authorization; no reply from the browser.
+  audio_device_->RequestDeviceAuthorization();
+
+  media::WaitableMessageLoopEvent event;
+
+  // Request device info on another thread.
+  thread.task_runner()->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&AudioOutputDeviceTest::CheckDeviceStatus,
+                 base::Unretained(this), OUTPUT_DEVICE_STATUS_ERROR_TIMED_OUT),
+      event.GetClosure());
+
+  io_loop_.RunUntilIdle();
+
+  // Runs the loop and waits for |thread| to call event's closure.
+  event.RunAndWait();
+
+  audio_device_->Stop();
+  io_loop_.RunUntilIdle();
+}
+
 INSTANTIATE_TEST_CASE_P(Render, AudioOutputDeviceTest, Values(false));
 
 }  // namespace media.

media/base/output_device_info.h

@@ -18,6 +18,7 @@ enum OutputDeviceStatus {
   OUTPUT_DEVICE_STATUS_OK = 0,
   OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND,
   OUTPUT_DEVICE_STATUS_ERROR_NOT_AUTHORIZED,
+  OUTPUT_DEVICE_STATUS_ERROR_TIMED_OUT,
   OUTPUT_DEVICE_STATUS_ERROR_INTERNAL,
   OUTPUT_DEVICE_STATUS_LAST = OUTPUT_DEVICE_STATUS_ERROR_INTERNAL,
 };

tools/metrics/histograms/histograms.xml

@@ -20513,6 +20513,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Media.Audio.Render.OutputDeviceAuthorizationTimedOut"
+    enum="BooleanTimedOut">
+  <owner>[email protected]</owner>
+  <summary>
+    Whether audio output device timed out waiting for authorization reply from
+    the browser side.
+  </summary>
+</histogram>
+
 <histogram name="Media.Audio.RenderFailsWhenBufferSizeChangesMac"
     enum="BooleanChanged">
   <owner>[email protected]</owner>