Fix race in AudioRendererHost around render frame ID validation.

content/browser/renderer_host/media/audio_renderer_host_unittest.cc

 // Copyright (c) 2012 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_renderer_host.h"
 
 #include <stdint.h>
 
 #include <memory>
 
@@ skipping 11 matching lines @@
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "ipc/ipc_message_utils.h"
 #include "media/audio/audio_manager.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/media_switches.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::Assign;
+using ::testing::AtLeast;
 using ::testing::DoAll;
 using ::testing::NotNull;
 
 namespace content {
 
 namespace {
 const int kRenderProcessId = 1;
 const int kRenderFrameId = 5;
 const int kStreamId = 50;
 const char kSecurityOrigin[] = "http://localhost";
 const char kDefaultDeviceId[] = "";
 const char kBadDeviceId[] =
     "badbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbad1";
 const char kInvalidDeviceId[] = "invalid-device-id";
 
-#if DCHECK_IS_ON()
 void ValidateRenderFrameId(int render_process_id,
                            int render_frame_id,
                            const base::Callback<void(bool)>& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   const bool frame_exists = (render_process_id == kRenderProcessId &&
                              render_frame_id == kRenderFrameId);
   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                           base::Bind(callback, frame_exists));
 }
-#endif  // DCHECK_IS_ON()
 
 }  // namespace
 
 class MockAudioMirroringManager : public AudioMirroringManager {
  public:
   MockAudioMirroringManager() {}
   virtual ~MockAudioMirroringManager() {}
 
   MOCK_METHOD3(AddDiverter,
                void(int render_process_id,
@@ skipping 12 matching lines @@
                         MediaInternals* media_internals,
                         MediaStreamManager* media_stream_manager,
                         const std::string& salt)
       : AudioRendererHost(kRenderProcessId,
                           audio_manager,
                           mirroring_manager,
                           media_internals,
                           media_stream_manager,
                           salt),
         shared_memory_length_(0) {
-#if DCHECK_IS_ON()
     set_render_frame_id_validate_function_for_testing(&ValidateRenderFrameId);
-#endif  // DCHECK_IS_ON()
   }
 
   // A list of mock methods.
   MOCK_METHOD4(OnDeviceAuthorized,
                void(int stream_id,
                     media::OutputDeviceStatus device_status,
                     const media::AudioParameters& output_params,
                     const std::string& matched_device_id));
   MOCK_METHOD2(OnStreamCreated, void(int stream_id, int length));
   MOCK_METHOD1(OnStreamPlaying, void(int stream_id));
@@ skipping 174 matching lines @@
 
       // At some point in the future, a corresponding RemoveDiverter() call must
       // be made.
       EXPECT_CALL(mirroring_manager_, RemoveDiverter(NotNull()))
           .RetiresOnSaturation();
     }
     SyncWithAudioThread();
   }
 
   void CreateWithInvalidRenderFrameId() {
-    // Because the render frame is invalid, the host should only reply with a
-    // stream error message.
+    // When creating a stream with an invalid render frame ID, the host will
+    // reply with a stream error message.
     EXPECT_CALL(*host_, OnStreamError(kStreamId));
 
-    // When DCHECKs are on, provide a seemingly-valid render frame ID; and it
-    // should be rejected when AudioRendererHost calls
-    // ValidateRenderFrameId(). When DCHECKs are off, AudioRendererHost won't
-    // call the validation function, but can still reject a render frame ID when
-    // it is obviously bogus.
-#if DCHECK_IS_ON()
+    // However, validation does not block stream creation, so these method calls
+    // might be made:
+    EXPECT_CALL(*host_, OnStreamCreated(kStreamId, _)).Times(AtLeast(0));
+    EXPECT_CALL(mirroring_manager_, AddDiverter(_, _, _)).Times(AtLeast(0));
+    EXPECT_CALL(mirroring_manager_, RemoveDiverter(_)).Times(AtLeast(0));
+
+    // Provide a seemingly-valid render frame ID; and it should be rejected when
+    // AudioRendererHost calls ValidateRenderFrameId().
     const int kInvalidRenderFrameId = kRenderFrameId + 1;
-#else
-    const int kInvalidRenderFrameId = MSG_ROUTING_NONE;
-#endif  // DCHECK_IS_ON()
-
     const media::AudioParameters params(
         media::AudioParameters::AUDIO_FAKE, media::CHANNEL_LAYOUT_STEREO,
         media::AudioParameters::kAudioCDSampleRate, 16,
         media::AudioParameters::kAudioCDSampleRate / 10);
     host_->OnCreateStream(kStreamId, kInvalidRenderFrameId, params);
+    base::RunLoop().RunUntilIdle();
   }
 
   void Close() {
     // Send a message to AudioRendererHost to tell it we want to close the
     // stream.
     host_->OnCloseStream(kStreamId);
     SyncWithAudioThread();
   }
 
   void Play() {
@@ skipping 129 matching lines @@
 }
 
 TEST_F(AudioRendererHostTest, CreateFailsForInvalidRenderFrame) {
   CreateWithInvalidRenderFrameId();
   Close();
 }
 
 // TODO(hclam): Add tests for data conversation in low latency mode.
 
 }  // namespace content