Move media/audio files into media namespace

Move media/audio files into media namespace

BUG=115187
TEST=compiles and runs without breaking audio tag; media_unittests, content_unittests

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

[vrk (LEFT CHROMIUM), 2012-03-21 00:40:05]

jam: content OWNERS review
scherkus: everything! (not as scary as it looks)

enal/tommi/crogers: FYI


This CL is based off of http://codereview.chromium.org/9655018/

[vrk (LEFT CHROMIUM), 2012-03-21 00:45:46]

Oh, and there was one unit test I couldn't move into the media namespace. When
it's in the media namespace, I hit some weird Gtest asserts. When it's in the
global scope, it's fine. See comment!

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
File media/audio/audio_output_proxy_unittest.cc (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
media/audio/audio_output_proxy_unittest.cc:18: // XXX(vrk): This unit test runs
into GTest asserts when I try to put it in
Wanted to highlight this weirdness. I have no idea what's going on with this
test :/ any ideas?

[jam, 2012-03-21 04:33:03]

thanks a lot for doing this, lgtm for content files outside of the media subdirs

as for the gtest, I'm not sure what the problem is

[scherkus (not reviewing), 2012-03-21 08:50:27]

LGTM w/ nits + suggestion for gtest stuff

I think we have multiple definitions of certain mocks that should be in anon
namespaces

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/browser/rend...
File content/browser/renderer_host/media/audio_renderer_host_unittest.cc
(right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/browser/rend...
content/browser/renderer_host/media/audio_renderer_host_unittest.cc:207:
media::AudioParameters::kAudioCDSampleRate, 16,
indent here

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/renderer/med...
File content/renderer/media/renderer_webaudiodevice_impl.h (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/renderer/med...
content/renderer/media/renderer_webaudiodevice_impl.h:19: class
RendererWebAudioDeviceImpl : public WebKit::WebAudioDevice,
nit: want to fix this to be initializer list style?

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
File media/audio/audio_output_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
media/audio/audio_output_dispatcher.h:77: // XXX(vrk): See comment in
AudioOutputProxyTest.
???

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
File media/audio/audio_output_proxy_unittest.cc (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
media/audio/audio_output_proxy_unittest.cc:18: // XXX(vrk): This unit test runs
into GTest asserts when I try to put it in
On 2012/03/21 00:45:46, Victoria Kirst wrote:
> Wanted to highlight this weirdness. I have no idea what's going on with this
> test :/ any ideas?

it's possible that we're running into duplicate definitions of mock classes
(i.e., one inside of media namespace, one outside of namespace) -- for example I
notice many different definitions of MockAudioManager

try moving the mock classes below into an anon namespace

[scherkus (not reviewing), 2012-03-21 13:05:44]

also check if valgrind suppressions need updating

[vrk (LEFT CHROMIUM), 2012-03-21 20:17:48]

Thanks scherkus and jam!

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/browser/rend...
File content/browser/renderer_host/media/audio_renderer_host_unittest.cc
(right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/browser/rend...
content/browser/renderer_host/media/audio_renderer_host_unittest.cc:207:
media::AudioParameters::kAudioCDSampleRate, 16,
On 2012/03/21 08:50:27, scherkus wrote:
> indent here

Done.

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/renderer/med...
File content/renderer/media/renderer_webaudiodevice_impl.h (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/content/renderer/med...
content/renderer/media/renderer_webaudiodevice_impl.h:19: class
RendererWebAudioDeviceImpl : public WebKit::WebAudioDevice,
On 2012/03/21 08:50:27, scherkus wrote:
> nit: want to fix this to be initializer list style?

Done.

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
File media/audio/audio_output_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
media/audio/audio_output_dispatcher.h:77: // XXX(vrk): See comment in
AudioOutputProxyTest.
On 2012/03/21 08:50:27, scherkus wrote:
> ???

Oh, I was just trying to explain why AudioOutputProxyTest was in the global
scope! Deleted now anyway!

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
File media/audio/audio_output_proxy_unittest.cc (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
media/audio/audio_output_proxy_unittest.cc:18: // XXX(vrk): This unit test runs
into GTest asserts when I try to put it in
On 2012/03/21 08:50:27, scherkus wrote:
> On 2012/03/21 00:45:46, Victoria Kirst wrote:
> > Wanted to highlight this weirdness. I have no idea what's going on with this
> > test :/ any ideas?
> 
> it's possible that we're running into duplicate definitions of mock classes
> (i.e., one inside of media namespace, one outside of namespace) -- for example
I
> notice many different definitions of MockAudioManager
> 
> try moving the mock classes below into an anon namespace

Yay that works, and seems like the right thing to do anyway to avoid name
collisions in unit tests in the future! Thanks!

...but still a bit confused about why there was a problem originally!

The other definitions of "MockAudioManager" are in
media/audio/audio_low_latency_input_output_unittest.cc and
/content/browser/speech/speech_recognizer_impl_unittest.cc. Only the first is
linked into media_unittests, and both of these definitions are in unnamed
namespaces regardless. Doesn't that mean they're unique/private to the
translation unit? So how could there be a problem, I guess gmock is doing
something funky with these classes in the macro expansion?

[scherkus (not reviewing), 2012-03-22 09:43:21]

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
File media/audio/audio_output_proxy_unittest.cc (right):

https://chromiumcodereview.appspot.com/9805001/diff/6002/media/audio/audio_ou...
media/audio/audio_output_proxy_unittest.cc:18: // XXX(vrk): This unit test runs
into GTest asserts when I try to put it in
On 2012/03/21 20:17:48, Victoria Kirst wrote:
> On 2012/03/21 08:50:27, scherkus wrote:
> > On 2012/03/21 00:45:46, Victoria Kirst wrote:
> > > Wanted to highlight this weirdness. I have no idea what's going on with
this
> > > test :/ any ideas?
> > 
> > it's possible that we're running into duplicate definitions of mock classes
> > (i.e., one inside of media namespace, one outside of namespace) -- for
example
> I
> > notice many different definitions of MockAudioManager
> > 
> > try moving the mock classes below into an anon namespace
> 
> Yay that works, and seems like the right thing to do anyway to avoid name
> collisions in unit tests in the future! Thanks!
> 
> ...but still a bit confused about why there was a problem originally!
> 
> The other definitions of "MockAudioManager" are in
> media/audio/audio_low_latency_input_output_unittest.cc and
> /content/browser/speech/speech_recognizer_impl_unittest.cc. Only the first is
> linked into media_unittests, and both of these definitions are in unnamed
> namespaces regardless. Doesn't that mean they're unique/private to the
> translation unit? So how could there be a problem, I guess gmock is doing
> something funky with these classes in the macro expansion?

Actually it looks like it wasn't MockAudioManager but perhaps
MockAudioSourceCallback! None of them appear to be in anon namespaces.

Want to see if it's possible to de-dupe the various mocks in a follow up CL? I'm
not sure how much the duplicate definitions diverge.

[commit-bot: I haz the power, 2012-04-02 16:50:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vrk@chromium.org/9805001/21001

[commit-bot: I haz the power, 2012-04-02 18:29:13]

Can't apply patch for file media/tools/player_wtl/movie.cc.
While running patch -p1 --forward --force;
patching file media/tools/player_wtl/movie.cc
Hunk #1 FAILED at 20.
1 out of 1 hunk FAILED -- saving rejects to file
media/tools/player_wtl/movie.cc.rej