Remove MockMediaInternals and incestuous usage.

Remove MockMediaInternals and incestuous usage.

While introducing the new AudioLog class, I encountered some WebRTC
and AudioRendererHost unittests abusing MediaInternals for internal
state observation.  https://codereview.chromium.org/68173025/

In the AudioRendererHost case, everything tested is already enforced
by various DCHECKs and the AudioManager (all streams closed, etc).

Similarly, in the WebRTC case, most checks are enforced implicitly or
tested via other means.

To remove the usage in AudioRendererHost I needed to switch over to
a TestBrowserThreadBundle which should have been done anyways.

BUG=260005
TEST=unittests still pass.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=236919

[tommi (sloooow) - chröme, 2013-11-22 11:55:31]

lgtm

https://codereview.chromium.org/81953002/diff/30001/content/browser/renderer_...
File content/browser/renderer_host/media/audio_renderer_host_unittest.cc
(right):

https://codereview.chromium.org/81953002/diff/30001/content/browser/renderer_...
content/browser/renderer_host/media/audio_renderer_host_unittest.cc:55: explicit
MockAudioRendererHost(media::AudioManager* audio_manager,
no need for explicit

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
File content/test/webrtc_audio_device_test.cc (right):

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
content/test/webrtc_audio_device_test.cc:188: has_input_devices_(false),
has_output_devices_(false) {
initialize media_internals_

[acolwell GONE FROM CHROMIUM, 2013-11-22 19:12:47]

lgtm

https://codereview.chromium.org/81953002/diff/30001/content/browser/renderer_...
File content/browser/renderer_host/media/audio_renderer_host_unittest.cc
(right):

https://codereview.chromium.org/81953002/diff/30001/content/browser/renderer_...
content/browser/renderer_host/media/audio_renderer_host_unittest.cc:272:
&AudioRendererHostTest::PostQuitMessageLoop,
Why do you need PostQuitMessageLoop? You should be able to
media::BindToCurrentLoop(run_loop.QuitClosure()).

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
File content/test/webrtc_audio_device_test.cc (right):

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
content/test/webrtc_audio_device_test.cc:322: media_internals_,
media_stream_manager_.get(), channel_.get());
This appears to be the only use of media_internals_. Can't you just use
MediaInternals::GetInstance() here and remove the need for the member variable?

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
File content/test/webrtc_audio_device_test.h (right):

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
content/test/webrtc_audio_device_test.h:14: #include
"content/browser/media/media_internals.h"
nit: forward declare instead.

[DaleCurtis, 2013-11-22 20:33:33]

Thanks for the review, guys!

https://codereview.chromium.org/81953002/diff/30001/content/browser/renderer_...
File content/browser/renderer_host/media/audio_renderer_host_unittest.cc
(right):

https://codereview.chromium.org/81953002/diff/30001/content/browser/renderer_...
content/browser/renderer_host/media/audio_renderer_host_unittest.cc:55: explicit
MockAudioRendererHost(media::AudioManager* audio_manager,
On 2013/11/22 11:55:31, tommi wrote:
> no need for explicit

Done.

https://codereview.chromium.org/81953002/diff/30001/content/browser/renderer_...
content/browser/renderer_host/media/audio_renderer_host_unittest.cc:272:
&AudioRendererHostTest::PostQuitMessageLoop,
On 2013/11/22 19:12:48, acolwell wrote:
> Why do you need PostQuitMessageLoop? You should be able to
> media::BindToCurrentLoop(run_loop.QuitClosure()).

Derp. It was left over from a previous incarnation.

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
File content/test/webrtc_audio_device_test.cc (right):

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
content/test/webrtc_audio_device_test.cc:188: has_input_devices_(false),
has_output_devices_(false) {
On 2013/11/22 11:55:31, tommi wrote:
> initialize media_internals_

No longer necessary.

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
content/test/webrtc_audio_device_test.cc:322: media_internals_,
media_stream_manager_.get(), channel_.get());
On 2013/11/22 19:12:48, acolwell wrote:
> This appears to be the only use of media_internals_. Can't you just use
> MediaInternals::GetInstance() here and remove the need for the member
variable?

Done.

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
File content/test/webrtc_audio_device_test.h (right):

https://codereview.chromium.org/81953002/diff/30001/content/test/webrtc_audio...
content/test/webrtc_audio_device_test.h:14: #include
"content/browser/media/media_internals.h"
On 2013/11/22 19:12:48, acolwell wrote:
> nit: forward declare instead.

Done.

[commit-bot: I haz the power, 2013-11-22 20:35:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/81953002/110001

[commit-bot: I haz the power, 2013-11-22 21:14:11]

Retried try job too often on mac_rel for step(s) content_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-11-22 23:14:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/81953002/330001

[commit-bot: I haz the power, 2013-11-23 01:58:38]

Change committed as 236919