MediaCodecLoop unit tests

MediaCodecLoop unit tests.

This CL adds unit tests for MediaCodecLoop that compile and run
anywhere.  We do this, so that we don't accidentally take any
dependencies on the android runtime, e.g., BuildInfo.  It does
this with new build targets.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.

Note that building non-unittest targets on non-android platforms will
not include extra code.

BUG=632401
Committed: https://crrev.com/7014d6d056a3a524c123b5886065a6f1e2722c09
Cr-Commit-Position: refs/heads/master@{#411075}

[liberato (no reviews please), 2016-07-07 16:08:40]

Description was changed from

==========
started android anywhere

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.  As we add code that's not directly
tied to android (e.g., AVDA, MCAD, MCL), it's nice to split up some
of it to facilitate proper unit testing.  We also get to run the
tests more often.

The disadvantage is that it's more complicated.  Perhaps we would
do better to just clean up the interfaces in place (android/), and
do proper unit testing there.  not sure that the "anywhere" part
buys us much.

Note that building non-unittest targets on non-android platforms will
not include extra code.

BUG=
==========

to

==========
started android anywhere

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.  As we add code that's not directly
tied to android (e.g., AVDA, MCAD, MCL), it's nice to split up some
of it to facilitate proper unit testing.  We also get to run the
tests more often.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Note that building non-unittest targets on non-android platforms will
not include extra code.

The disadvantage is that it's more complicated.  Perhaps we would
do better to just clean up the interfaces in place (android/), and
do proper unit testing there.  not sure that the "anywhere" part
buys us much.  we might even include checks at commit time to make
sure that we don't include android-specific stuff, similar to DEPS.

BUG=
==========

[liberato (no reviews please), 2016-07-07 16:08:40]

liberato@chromium.org changed reviewers:
+ watk@chromium.org

[liberato (no reviews please), 2016-07-07 16:23:09]

liberato@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[liberato (no reviews please), 2016-07-07 16:23:10]

i tried out the idea of making media_unittests run MCL unit tests even on
desktop.  here's the result, which seems to work fine.  it's kinda nice that
we'd get these tests run all the time, and that it makes a clear,
enforced-at-build-time distinction between core android classes that use JNI to
talk to the platform, vs. things like MCL, that are perfectly happy with android
mocks.

i expect that this could expand to MCAD, AVDA (+strategy, codec image, timer
loop).

it's intended as a prototype, just to show the general structure that might
result.  certainly one could make other choices -- not putting the "runs
anywhere" directory under "android", for example.

i'm not sure if i like the added complexity of the whole approach.  alternate
approach is just to clean up MCL / etc. so that they don't depend on JNI, but
also skip getting them included in desktop builds and keep the existing
directory structure.  that would no longer enforce the distinction during build,
though.  not sure if we could add the distinction back via cl hooks or similar.

going to chat with crouleau@ also, to get his input.

wow, now that i've essentially retyped the commit message, i'm going to stop :)

comments welcome.

thanks
-fl

[DaleCurtis, 2016-07-07 17:32:45]

I like the idea, but would prefer to not introduce some new "anywhere" directory
concept. Can we just list the media/base/android/ test files explicitly in
media_unittests on all platforms? I.e. create a media_anywhere source_set that
media_unittests can depend on?

I also worry that we haven't written enough tests here to know if this is going
to be a good idea, or if it's just something we'll need to remove later in order
to get proper testing.

Does media_unittests_apk run on CQ bots today? I thought it did.

[liberato (no reviews please), 2016-07-07 17:46:23]

On 2016/07/07 17:32:45, DaleCurtis wrote:
> I like the idea, but would prefer to not introduce some new "anywhere"
directory
> concept. Can we just list the media/base/android/ test files explicitly in
> media_unittests on all platforms? I.e. create a media_anywhere source_set that
> media_unittests can depend on?

yeah, we can do that too.  i was a lot less impressed with the current CL once i
wrote it, than how it looked on paper.

the main benefit of tests outside of android is that it forces us to layer our
android code better.  the source set idea gets that, so i tend to like it
better.

> I also worry that we haven't written enough tests here to know if this is
going
> to be a good idea, or if it's just something we'll need to remove later in
order
> to get proper testing.

i don't think that we'd have to remove it -- i'm not suggesting removing the
android-specific tests, nor does this limit what we can test there.  we'd just
be adding a way to test things outside of android.
 
> Does media_unittests_apk run on CQ bots today? I thought it did.

i believe that it does, yes, and would continue to do so.

[liberato (no reviews please), 2016-07-07 18:36:37]

same idea, this time based on dale's suggestion.

-fl

[DaleCurtis, 2016-07-07 20:38:10]

Approach seems clearer. My question about the media unittests running on the CQ
was about whether or not this buys us anything. If that apk is already running
in CQ, what are we gaining here?

[liberato (no reviews please), 2016-07-07 20:57:10]

On 2016/07/07 20:38:10, DaleCurtis wrote:
> Approach seems clearer. My question about the media unittests running on the
CQ
> was about whether or not this buys us anything. If that apk is already running
> in CQ, what are we gaining here?

it lets us decide which part of our code may depend on android, and which may
not.  what this buys us is that the compiler will enforce our decision.

right now, we have a mess of hard-to-test code.  presumably, we'd choose to move
the android-specific bits into small, mockable pieces.  as an example, mocking
out "SurfaceTexture" right now isn't trivial, since it depends on ScopedJavaRef,
which depends on JNI - that's not really a unit test anymore.

we'll fix that either way, and (maybe) use these changes to make sure that it
never slides back the other way.

[DaleCurtis, 2016-07-07 21:01:34]

I worry it might have the opposite affect of making it difficult to add tests. I
don't think it's a bad thing to have some unit tests that are actually
integration tests. It doesn't seem like your idea precludes us from writing
those types of tests too.

[liberato (no reviews please), 2016-07-07 21:13:19]

On 2016/07/07 21:01:34, DaleCurtis wrote:
> I worry it might have the opposite affect of making it difficult to add tests.

yeah, that could happen.  i'm trying to write some MCL tests + MediaCodec mock
on linux to see if it's harder or not.

> don't think it's a bad thing to have some unit tests that are actually
> integration tests

i agree.  the caveat is, of course, that it's harder to tell what we're testing.
 i think that we need both better unit coverage and better integration.

>It doesn't seem like your idea precludes us from writing those types of tests
too.

it doesn't preclude it -- we can always write android-specific tests.

the idea came out of how to properly unit test this stuff without accidentally
depending on android.  for example, how do we change sdk_int() during testing? 
it's really easy to miss those dependencies on android, where they're always
available.  try that on linux, and the compiler gets mad.  it points out exactly
what dependencies exist.

i'm still unsure if i like this whole idea or not.

[DaleCurtis, 2016-07-07 21:16:19]

This all sounds reasonable to me and the changes are just build tweaks, so I
don't have a problem with it. There are a few todo's in there, so I won't
approve for now, but let me know if you decide to land this.

[liberato (no reviews please), 2016-07-08 17:49:46]

just wrote some more tests, and wanted to point out how cool gmock is.  nothing
to review yet, just impressed by gmock and thought that i'd share.

https://codereview.chromium.org/2132653002/diff/40001/media/base/android/medi...
File media/base/android/media_codec_loop_unittest.cc (right):

https://codereview.chromium.org/2132653002/diff/40001/media/base/android/medi...
media/base/android/media_codec_loop_unittest.cc:175: EXPECT_CALL(*client_,
gmock is really amazing.  it lets me set an expectation that insists that
|index| of the provided OutputBuffer matches what we think it should be.

https://codereview.chromium.org/2132653002/diff/40001/media/base/android/medi...
media/base/android/media_codec_loop_unittest.cc:228:
.WillOnce(DoAll(SetArgPointee<1>(input_buffer_index),
this sets the *index parameter of DequeueInputBuffer to |input_buffer_index| as
a side-effect of the expectation.

[DaleCurtis, 2016-07-08 18:57:06]

Looking good! I already feel better about MCL :) Gmock can be very handy, but
it's easy to get into a state where writing the tests is very difficult. See
VideoRendererImpl unittest for an example of where there's so much state and
interaction that each test can take a day to write.

Hopefully this can actually sniff out some new bugs too.

[liberato (no reviews please), 2016-07-27 16:11:13]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 16:11:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-27 16:15:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-27 16:15:13]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[liberato (no reviews please), 2016-07-27 17:16:21]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 17:16:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-27 17:23:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-27 17:23:34]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[liberato (no reviews please), 2016-07-28 16:32:43]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 16:33:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 16:50:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 16:51:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[liberato (no reviews please), 2016-07-28 17:08:51]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 17:09:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2016-07-28 17:21:08]

Description was changed from

==========
started android anywhere

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.  As we add code that's not directly
tied to android (e.g., AVDA, MCAD, MCL), it's nice to split up some
of it to facilitate proper unit testing.  We also get to run the
tests more often.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Note that building non-unittest targets on non-android platforms will
not include extra code.

The disadvantage is that it's more complicated.  Perhaps we would
do better to just clean up the interfaces in place (android/), and
do proper unit testing there.  not sure that the "anywhere" part
buys us much.  we might even include checks at commit time to make
sure that we don't include android-specific stuff, similar to DEPS.

BUG=
==========

to

==========
MediaCodecLoop unit tests.

This CL adds unit tests for MediaCodecLoop that compile and run
anywhere.  We do this, so that we don't accidentally take any
dependencies on the android runtime, e.g., BuildInfo.  It does
this with new build targets.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.

Note that building non-unittest targets on non-android platforms will
not include extra code.

BUG=632401
==========

[liberato (no reviews please), 2016-07-28 17:22:05]

i decided to decouple this from the BuildInfo mock, since there some issues
there that might delay it.  for now, we just pass sdk_int into MCL during
construction.

i'm not very happy about this approach; i'd rather pass in a buildinfo mock.
since there is only one or two consumers of MCL, and since it'll be easy to
retrofit once BuildInfo mocks are available, i think that it's worth it to
unblock unit testing.

[commit-bot: I haz the power, 2016-07-28 17:24:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 17:24:11]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[liberato (no reviews please), 2016-07-28 19:59:41]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 20:00:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 20:15:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 20:15:55]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[liberato (no reviews please), 2016-07-28 21:55:19]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 21:55:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2016-07-28 22:00:34]

Patchset #12 (id:220001) has been deleted

[commit-bot: I haz the power, 2016-07-28 22:40:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 22:40:52]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[liberato (no reviews please), 2016-07-29 17:32:56]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 17:33:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2016-07-29 18:13:01]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 18:13:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 19:08:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 19:08:10]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[liberato (no reviews please), 2016-08-01 16:12:44]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 16:12:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-01 16:55:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-01 16:55:10]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[DaleCurtis, 2016-08-01 18:23:17]

Still not in love with the anywhere name, so please at least give it a pretty
thorough comment in the .GN file. Overall lgtm though with some minor changes.

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/med...
File media/base/android/media_codec_loop.cc (right):

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/med...
media/base/android/media_codec_loop.cc:312: base::TimeTicks now =
(test_tick_clock_ ? test_tick_clock_->NowTicks()
Instead of doing this, always create tick_clock_. See media::VideoRendererImpl
for details.

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/med...
File media/base/android/media_codec_loop.h (right):

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/med...
media/base/android/media_codec_loop.h:317: base::TickClock* test_tick_clock_ =
nullptr;
If you're going to use in header initialization do it for everything possible
and group those parameters. Otherwise do this in the constructor.

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/med...
File media/base/android/media_codec_loop_unittest.cc (right):

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/med...
media/base/android/media_codec_loop_unittest.cc:146: struct OutputBuffer {
Move above or below all functions.

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/med...
media/base/android/media_codec_loop_unittest.cc:188: // TODO: how is the
lifecycle of |clock_| handled?  |task_runner_| can outlive
See VideoRendererImpl tests.

[DaleCurtis, 2016-08-01 18:24:00]

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/BUI...
File media/base/android/BUILD.gn (right):

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/BUI...
media/base/android/BUILD.gn:5: import("//build/config/android/config.gni")
Is this accessible on non-android checkouts?

https://codereview.chromium.org/2132653002/diff/300001/media/base/android/BUI...
media/base/android/BUILD.gn:10: #assert(is_android)
Delete now.

[liberato (no reviews please), 2016-08-02 15:26:39]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 15:26:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 16:06:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 16:07:01]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[liberato (no reviews please), 2016-08-02 16:44:26]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 16:44:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2016-08-02 17:46:14]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 17:46:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 18:45:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 18:45:18]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[liberato (no reviews please), 2016-08-02 21:39:29]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 21:40:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[liberato (no reviews please), 2016-08-02 23:16:54]

The CQ bit was checked by liberato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 23:17:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 01:17:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-03 01:17:57]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[liberato (no reviews please), 2016-08-08 23:54:02]

turns out that MCL tests on windows fails due to what looks like a compiler bug
with constexpr global variables.  i didn't see any other instances in chrome,
luckily, except for MCL and AVDA.

anyway, i converted from constexpr variables to constexpr functions, and it
works everywhere.

[DaleCurtis, 2016-08-08 23:56:32]

On 2016/08/08 at 23:54:02, liberato wrote:
> turns out that MCL tests on windows fails due to what looks like a compiler
bug with constexpr global variables.  i didn't see any other instances in
chrome, luckily, except for MCL and AVDA.
> 
> anyway, i converted from constexpr variables to constexpr functions, and it
works everywhere.

Can you give an example? Or minimized test case? We should file it as a bug if
it is one.

[liberato (no reviews please), 2016-08-09 00:14:41]

On 2016/08/08 23:56:32, DaleCurtis wrote:
> On 2016/08/08 at 23:54:02, liberato wrote:
> > turns out that MCL tests on windows fails due to what looks like a compiler
> bug with constexpr global variables.  i didn't see any other instances in
> chrome, luckily, except for MCL and AVDA.
> > 
> > anyway, i converted from constexpr variables to constexpr functions, and it
> works everywhere.
> 
> Can you give an example? Or minimized test case? We should file it as a bug if
> it is one.

i don't have a minimized test case yet.

the effect was that 'constexpr base::TimeDelta foo =
base::TimeDelta::From{AnythingButMicroseconds}(10);' ends up initializing foo to
0 microseconds.  moving that same thing into function scope, or removing
constexpr, causes it to work.  ToMicroseconds is special since it just does the
assignment, rather than converting to microseconds.  adding 'static' or moving
to anonymous namespace (different init order, i think), did nothing.

i'll see if i can make something standalone.  my windows dev environment is
still not set up very well.

[DaleCurtis, 2016-08-09 00:19:54]

cc:jyasskin for FYI on the constexpr base::Time issues (see previous message
from liberato@).

[Jeffrey Yasskin, 2016-08-09 00:53:16]

On 2016/08/09 00:14:41, liberato wrote:
> On 2016/08/08 23:56:32, DaleCurtis wrote:
> > On 2016/08/08 at 23:54:02, liberato wrote:
> > > turns out that MCL tests on windows fails due to what looks like a
compiler
> > bug with constexpr global variables.  i didn't see any other instances in
> > chrome, luckily, except for MCL and AVDA.
> > > 
> > > anyway, i converted from constexpr variables to constexpr functions, and
it
> > works everywhere.
> > 
> > Can you give an example? Or minimized test case? We should file it as a bug
if
> > it is one.
> 
> i don't have a minimized test case yet.
> 
> the effect was that 'constexpr base::TimeDelta foo =
> base::TimeDelta::From{AnythingButMicroseconds}(10);' ends up initializing foo
to
> 0 microseconds.  moving that same thing into function scope, or removing
> constexpr, causes it to work.  ToMicroseconds is special since it just does
the
> assignment, rather than converting to microseconds.  adding 'static' or moving
> to anonymous namespace (different init order, i think), did nothing.
> 
> i'll see if i can make something standalone.  my windows dev environment is
> still not set up very well.

I don't recognize the description of the problem, but it's possible that MSVC
constexpr is broken. We should loop in brucedawson@ if we can find a simple test
case.

[liberato (no reviews please), 2016-08-09 01:21:22]

On 2016/08/09 00:53:16, Jeffrey Yasskin wrote:
> On 2016/08/09 00:14:41, liberato wrote:
> > On 2016/08/08 23:56:32, DaleCurtis wrote:
> > > On 2016/08/08 at 23:54:02, liberato wrote:
> > > > turns out that MCL tests on windows fails due to what looks like a
> compiler
> > > bug with constexpr global variables.  i didn't see any other instances in
> > > chrome, luckily, except for MCL and AVDA.
> > > > 
> > > > anyway, i converted from constexpr variables to constexpr functions, and
> it
> > > works everywhere.
> > > 
> > > Can you give an example? Or minimized test case? We should file it as a
bug
> if
> > > it is one.
> > 
> > i don't have a minimized test case yet.
> > 
> > the effect was that 'constexpr base::TimeDelta foo =
> > base::TimeDelta::From{AnythingButMicroseconds}(10);' ends up initializing
foo
> to
> > 0 microseconds.  moving that same thing into function scope, or removing
> > constexpr, causes it to work.  ToMicroseconds is special since it just does
> the
> > assignment, rather than converting to microseconds.  adding 'static' or
moving
> > to anonymous namespace (different init order, i think), did nothing.
> > 
> > i'll see if i can make something standalone.  my windows dev environment is
> > still not set up very well.
> 
> I don't recognize the description of the problem, but it's possible that MSVC
> constexpr is broken. We should loop in brucedawson@ if we can find a simple
test
> case.

i moved base::TimeDelta into media/base/android as base1::TimeDelta, and it
still fails (yay!).  So, it's not related to different libraries.  will keep
simplifying.

[liberato (no reviews please), 2016-08-09 01:33:00]

On 2016/08/09 01:21:22, liberato wrote:
> On 2016/08/09 00:53:16, Jeffrey Yasskin wrote:
> > On 2016/08/09 00:14:41, liberato wrote:
> > > On 2016/08/08 23:56:32, DaleCurtis wrote:
> > > > On 2016/08/08 at 23:54:02, liberato wrote:
> > > > > turns out that MCL tests on windows fails due to what looks like a
> > compiler
> > > > bug with constexpr global variables.  i didn't see any other instances
in
> > > > chrome, luckily, except for MCL and AVDA.
> > > > > 
> > > > > anyway, i converted from constexpr variables to constexpr functions,
and
> > it
> > > > works everywhere.
> > > > 
> > > > Can you give an example? Or minimized test case? We should file it as a
> bug
> > if
> > > > it is one.
> > > 
> > > i don't have a minimized test case yet.
> > > 
> > > the effect was that 'constexpr base::TimeDelta foo =
> > > base::TimeDelta::From{AnythingButMicroseconds}(10);' ends up initializing
> foo
> > to
> > > 0 microseconds.  moving that same thing into function scope, or removing
> > > constexpr, causes it to work.  ToMicroseconds is special since it just
does
> > the
> > > assignment, rather than converting to microseconds.  adding 'static' or
> moving
> > > to anonymous namespace (different init order, i think), did nothing.
> > > 
> > > i'll see if i can make something standalone.  my windows dev environment
is
> > > still not set up very well.
> > 
> > I don't recognize the description of the problem, but it's possible that
MSVC
> > constexpr is broken. We should loop in brucedawson@ if we can find a simple
> test
> > case.
> 
> i moved base::TimeDelta into media/base/android as base1::TimeDelta, and it
> still fails (yay!).  So, it's not related to different libraries.  will keep
> simplifying.

it has something to do with the ternary conditional in TimeDelta::FromProduct. 
When i make it say "value > 5 ? TimeDelta(1234) : TimeDelta(5678)", i get zero
milliseconds.  if i just return TimeDelta(1234), however, then it ends up
correctly as 1234.

i'll try moving the conditional higher in the FromMilliseconds call tree (remove
calls to FromProduct), to see if the call pattern is important, or if just a
conditional is enough.

[liberato (no reviews please), 2016-08-09 16:20:46]

On 2016/08/09 01:33:00, liberato wrote:
> On 2016/08/09 01:21:22, liberato wrote:
> > On 2016/08/09 00:53:16, Jeffrey Yasskin wrote:
> > > On 2016/08/09 00:14:41, liberato wrote:
> > > > On 2016/08/08 23:56:32, DaleCurtis wrote:
> > > > > On 2016/08/08 at 23:54:02, liberato wrote:
> > > > > > turns out that MCL tests on windows fails due to what looks like a
> > > compiler
> > > > > bug with constexpr global variables.  i didn't see any other instances
> in
> > > > > chrome, luckily, except for MCL and AVDA.
> > > > > > 
> > > > > > anyway, i converted from constexpr variables to constexpr functions,
> and
> > > it
> > > > > works everywhere.
> > > > > 
> > > > > Can you give an example? Or minimized test case? We should file it as
a
> > bug
> > > if
> > > > > it is one.
> > > > 
> > > > i don't have a minimized test case yet.
> > > > 
> > > > the effect was that 'constexpr base::TimeDelta foo =
> > > > base::TimeDelta::From{AnythingButMicroseconds}(10);' ends up
initializing
> > foo
> > > to
> > > > 0 microseconds.  moving that same thing into function scope, or removing
> > > > constexpr, causes it to work.  ToMicroseconds is special since it just
> does
> > > the
> > > > assignment, rather than converting to microseconds.  adding 'static' or
> > moving
> > > > to anonymous namespace (different init order, i think), did nothing.
> > > > 
> > > > i'll see if i can make something standalone.  my windows dev environment
> is
> > > > still not set up very well.
> > > 
> > > I don't recognize the description of the problem, but it's possible that
> MSVC
> > > constexpr is broken. We should loop in brucedawson@ if we can find a
simple
> > test
> > > case.
> > 
> > i moved base::TimeDelta into media/base/android as base1::TimeDelta, and it
> > still fails (yay!).  So, it's not related to different libraries.  will keep
> > simplifying.
> 
> it has something to do with the ternary conditional in TimeDelta::FromProduct.

> When i make it say "value > 5 ? TimeDelta(1234) : TimeDelta(5678)", i get zero
> milliseconds.  if i just return TimeDelta(1234), however, then it ends up
> correctly as 1234.
> 
> i'll try moving the conditional higher in the FromMilliseconds call tree
(remove
> calls to FromProduct), to see if the call pattern is important, or if just a
> conditional is enough.

The following reproduces the bug.  I compile it as part of chromium, and call
try_it() from MCL constructor (for convenience).  I didn't fiddle with trying
to build it standalone, and/or changing compiler flags.  the gn flags for the
build contained "use_goma = true", and that's it.

// Copyright 2016 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 <stdio.h>

class Foo {
public:
  constexpr Foo(int x) : x_(x) {}

  static constexpr Foo Create(int x) {
    // Inlining ReallyCreate here avoids the bug.
    return ReallyCreate(x);
  }

  static constexpr Foo ReallyCreate(int x) {
    // Removing ?: avoids the bug.
    return x > 5 ? Foo(1) : Foo(2);
  }

  int x_;
};

// Removing constexpr avoids the bug.
// Calling ReallyCreate directly avoids the bug.
constexpr Foo foo = Foo::Create(1);

void try_it() {
  // Moving foo decl here from global scope avoids the bug.

  // Prints zero, but should print 2.
  fprintf(stderr, "foo.x_ == %d\n", foo.x_);
}

[liberato (no reviews please), 2016-08-09 17:44:22]

On 2016/08/09 16:20:46, liberato wrote:
> On 2016/08/09 01:33:00, liberato wrote:
> > On 2016/08/09 01:21:22, liberato wrote:
> > > On 2016/08/09 00:53:16, Jeffrey Yasskin wrote:
> > > > On 2016/08/09 00:14:41, liberato wrote:
> > > > > On 2016/08/08 23:56:32, DaleCurtis wrote:
> > > > > > On 2016/08/08 at 23:54:02, liberato wrote:
> > > > > > > turns out that MCL tests on windows fails due to what looks like a
> > > > compiler
> > > > > > bug with constexpr global variables.  i didn't see any other
instances
> > in
> > > > > > chrome, luckily, except for MCL and AVDA.
> > > > > > > 
> > > > > > > anyway, i converted from constexpr variables to constexpr
functions,
> > and
> > > > it
> > > > > > works everywhere.
> > > > > > 
> > > > > > Can you give an example? Or minimized test case? We should file it
as
> a
> > > bug
> > > > if
> > > > > > it is one.
> > > > > 
> > > > > i don't have a minimized test case yet.
> > > > > 
> > > > > the effect was that 'constexpr base::TimeDelta foo =
> > > > > base::TimeDelta::From{AnythingButMicroseconds}(10);' ends up
> initializing
> > > foo
> > > > to
> > > > > 0 microseconds.  moving that same thing into function scope, or
removing
> > > > > constexpr, causes it to work.  ToMicroseconds is special since it just
> > does
> > > > the
> > > > > assignment, rather than converting to microseconds.  adding 'static'
or
> > > moving
> > > > > to anonymous namespace (different init order, i think), did nothing.
> > > > > 
> > > > > i'll see if i can make something standalone.  my windows dev
environment
> > is
> > > > > still not set up very well.
> > > > 
> > > > I don't recognize the description of the problem, but it's possible that
> > MSVC
> > > > constexpr is broken. We should loop in brucedawson@ if we can find a
> simple
> > > test
> > > > case.
> > > 
> > > i moved base::TimeDelta into media/base/android as base1::TimeDelta, and
it
> > > still fails (yay!).  So, it's not related to different libraries.  will
keep
> > > simplifying.
> > 
> > it has something to do with the ternary conditional in
TimeDelta::FromProduct.
> 
> > When i make it say "value > 5 ? TimeDelta(1234) : TimeDelta(5678)", i get
zero
> > milliseconds.  if i just return TimeDelta(1234), however, then it ends up
> > correctly as 1234.
> > 
> > i'll try moving the conditional higher in the FromMilliseconds call tree
> (remove
> > calls to FromProduct), to see if the call pattern is important, or if just a
> > conditional is enough.
> 
> The following reproduces the bug.  I compile it as part of chromium, and call
> try_it() from MCL constructor (for convenience).  I didn't fiddle with trying
> to build it standalone, and/or changing compiler flags.  the gn flags for the
> build contained "use_goma = true", and that's it.
> 
> // Copyright 2016 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 <stdio.h>
> 
> class Foo {
> public:
>   constexpr Foo(int x) : x_(x) {}
> 
>   static constexpr Foo Create(int x) {
>     // Inlining ReallyCreate here avoids the bug.
>     return ReallyCreate(x);
>   }
> 
>   static constexpr Foo ReallyCreate(int x) {
>     // Removing ?: avoids the bug.
>     return x > 5 ? Foo(1) : Foo(2);
>   }
> 
>   int x_;
> };
> 
> // Removing constexpr avoids the bug.
> // Calling ReallyCreate directly avoids the bug.
> constexpr Foo foo = Foo::Create(1);
> 
> void try_it() {
>   // Moving foo decl here from global scope avoids the bug.
> 
>   // Prints zero, but should print 2.
>   fprintf(stderr, "foo.x_ == %d\n", foo.x_);
> }

filed the compiler bug as crbug.com/635974 .

[liberato (no reviews please), 2016-08-09 17:44:26]

On 2016/08/09 16:20:46, liberato wrote:
> On 2016/08/09 01:33:00, liberato wrote:
> > On 2016/08/09 01:21:22, liberato wrote:
> > > On 2016/08/09 00:53:16, Jeffrey Yasskin wrote:
> > > > On 2016/08/09 00:14:41, liberato wrote:
> > > > > On 2016/08/08 23:56:32, DaleCurtis wrote:
> > > > > > On 2016/08/08 at 23:54:02, liberato wrote:
> > > > > > > turns out that MCL tests on windows fails due to what looks like a
> > > > compiler
> > > > > > bug with constexpr global variables.  i didn't see any other
instances
> > in
> > > > > > chrome, luckily, except for MCL and AVDA.
> > > > > > > 
> > > > > > > anyway, i converted from constexpr variables to constexpr
functions,
> > and
> > > > it
> > > > > > works everywhere.
> > > > > > 
> > > > > > Can you give an example? Or minimized test case? We should file it
as
> a
> > > bug
> > > > if
> > > > > > it is one.
> > > > > 
> > > > > i don't have a minimized test case yet.
> > > > > 
> > > > > the effect was that 'constexpr base::TimeDelta foo =
> > > > > base::TimeDelta::From{AnythingButMicroseconds}(10);' ends up
> initializing
> > > foo
> > > > to
> > > > > 0 microseconds.  moving that same thing into function scope, or
removing
> > > > > constexpr, causes it to work.  ToMicroseconds is special since it just
> > does
> > > > the
> > > > > assignment, rather than converting to microseconds.  adding 'static'
or
> > > moving
> > > > > to anonymous namespace (different init order, i think), did nothing.
> > > > > 
> > > > > i'll see if i can make something standalone.  my windows dev
environment
> > is
> > > > > still not set up very well.
> > > > 
> > > > I don't recognize the description of the problem, but it's possible that
> > MSVC
> > > > constexpr is broken. We should loop in brucedawson@ if we can find a
> simple
> > > test
> > > > case.
> > > 
> > > i moved base::TimeDelta into media/base/android as base1::TimeDelta, and
it
> > > still fails (yay!).  So, it's not related to different libraries.  will
keep
> > > simplifying.
> > 
> > it has something to do with the ternary conditional in
TimeDelta::FromProduct.
> 
> > When i make it say "value > 5 ? TimeDelta(1234) : TimeDelta(5678)", i get
zero
> > milliseconds.  if i just return TimeDelta(1234), however, then it ends up
> > correctly as 1234.
> > 
> > i'll try moving the conditional higher in the FromMilliseconds call tree
> (remove
> > calls to FromProduct), to see if the call pattern is important, or if just a
> > conditional is enough.
> 
> The following reproduces the bug.  I compile it as part of chromium, and call
> try_it() from MCL constructor (for convenience).  I didn't fiddle with trying
> to build it standalone, and/or changing compiler flags.  the gn flags for the
> build contained "use_goma = true", and that's it.
> 
> // Copyright 2016 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 <stdio.h>
> 
> class Foo {
> public:
>   constexpr Foo(int x) : x_(x) {}
> 
>   static constexpr Foo Create(int x) {
>     // Inlining ReallyCreate here avoids the bug.
>     return ReallyCreate(x);
>   }
> 
>   static constexpr Foo ReallyCreate(int x) {
>     // Removing ?: avoids the bug.
>     return x > 5 ? Foo(1) : Foo(2);
>   }
> 
>   int x_;
> };
> 
> // Removing constexpr avoids the bug.
> // Calling ReallyCreate directly avoids the bug.
> constexpr Foo foo = Foo::Create(1);
> 
> void try_it() {
>   // Moving foo decl here from global scope avoids the bug.
> 
>   // Prints zero, but should print 2.
>   fprintf(stderr, "foo.x_ == %d\n", foo.x_);
> }

filed the compiler bug as crbug.com/635974 .

[watk, 2016-08-09 18:32:56]

lgtm

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/BUI...
File media/base/android/BUILD.gn (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/BUI...
media/base/android/BUILD.gn:10: #assert(is_android)
delete

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_direction.h (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_direction.h:9: enum MediaCodecDirection {
Should this be in a namespace?

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_loop.cc (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_loop.cc:357: return sdk_int_ < 18;
Should call the MediaCodecUtil one

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_loop.h (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_loop.h:287: bool
DoesCodecFlushRequireDestruction() const;
MediaCodecUtil calls this CodecNeedsFlushWorkaround(). I think it would be nice
if they were consistent.

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_loop_unittest.cc (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_loop_unittest.cc:98: // We might want to
WaitUntilIdle here.
Not sure what you mean by this

[liberato (no reviews please), 2016-08-09 21:20:48]

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/BUI...
File media/base/android/BUILD.gn (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/BUI...
media/base/android/BUILD.gn:10: #assert(is_android)
On 2016/08/09 18:32:55, watk wrote:
> delete

Done.

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_direction.h (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_direction.h:9: enum MediaCodecDirection {
On 2016/08/09 18:32:55, watk wrote:
> Should this be in a namespace?

good catch.  done.

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_loop.cc (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_loop.cc:357: return sdk_int_ < 18;
On 2016/08/09 18:32:55, watk wrote:
> Should call the MediaCodecUtil one

MediaCodecUtil doesn't build anywhere yet, and i don't want to pull in
MediaCodecLoop for MediaCodecUtil to use.

i'll add a TODO.  i agree that they should be merged.

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_loop.h (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_loop.h:287: bool
DoesCodecFlushRequireDestruction() const;
On 2016/08/09 18:32:55, watk wrote:
> MediaCodecUtil calls this CodecNeedsFlushWorkaround(). I think it would be
nice
> if they were consistent.

Done.

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
File media/base/android/media_codec_loop_unittest.cc (right):

https://codereview.chromium.org/2132653002/diff/420001/media/base/android/med...
media/base/android/media_codec_loop_unittest.cc:98: // We might want to
WaitUntilIdle here.
On 2016/08/09 18:32:55, watk wrote:
> Not sure what you mean by this

doesn't mean much anymore.  removed.

[liberato (no reviews please), 2016-08-09 21:20:56]

The CQ bit was checked by liberato@chromium.org

[liberato (no reviews please), 2016-08-09 21:20:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, watk@chromium.org
Link to the patchset: https://codereview.chromium.org/2132653002/#ps440001
(title: "cl feedback")

[commit-bot: I haz the power, 2016-08-09 21:21:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 01:28:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 01:28:51]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[liberato (no reviews please), 2016-08-10 15:49:36]

The CQ bit was checked by liberato@chromium.org

[commit-bot: I haz the power, 2016-08-10 15:49:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 17:46:03]

Description was changed from

==========
MediaCodecLoop unit tests.

This CL adds unit tests for MediaCodecLoop that compile and run
anywhere.  We do this, so that we don't accidentally take any
dependencies on the android runtime, e.g., BuildInfo.  It does
this with new build targets.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.

Note that building non-unittest targets on non-android platforms will
not include extra code.

BUG=632401
==========

to

==========
MediaCodecLoop unit tests.

This CL adds unit tests for MediaCodecLoop that compile and run
anywhere.  We do this, so that we don't accidentally take any
dependencies on the android runtime, e.g., BuildInfo.  It does
this with new build targets.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.

Note that building non-unittest targets on non-android platforms will
not include extra code.

BUG=632401
==========

[commit-bot: I haz the power, 2016-08-10 17:46:05]

Committed patchset #22 (id:440001)

[commit-bot: I haz the power, 2016-08-10 17:49:59]

Description was changed from

==========
MediaCodecLoop unit tests.

This CL adds unit tests for MediaCodecLoop that compile and run
anywhere.  We do this, so that we don't accidentally take any
dependencies on the android runtime, e.g., BuildInfo.  It does
this with new build targets.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.

Note that building non-unittest targets on non-android platforms will
not include extra code.

BUG=632401
==========

to

==========
MediaCodecLoop unit tests.

This CL adds unit tests for MediaCodecLoop that compile and run
anywhere.  We do this, so that we don't accidentally take any
dependencies on the android runtime, e.g., BuildInfo.  It does
this with new build targets.

For example, we know that media_codec_loop.cc does not depend on
JNI to run, and doesn't care if (for example) MediaCodec is broken
today on the particular device on which the tests run.

Moved MediaCodecBridge and MediaCodecLoop to android/anywhere, which
builds anywhere.  For example, building media_unittests on linux will
include tests for MediaCodecLoop.

Note that building non-unittest targets on non-android platforms will
not include extra code.

BUG=632401

Committed: https://crrev.com/7014d6d056a3a524c123b5886065a6f1e2722c09
Cr-Commit-Position: refs/heads/master@{#411075}
==========

[commit-bot: I haz the power, 2016-08-10 17:50:02]

Patchset 22 (id:??) landed as
https://crrev.com/7014d6d056a3a524c123b5886065a6f1e2722c09
Cr-Commit-Position: refs/heads/master@{#411075}