Flip proprietary codecs to false for Android chromium builds.

Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.android:android_chromium_gn_rel

Committed: https://crrev.com/8dd1542a877b87424e764ee495bd1ae6ade30492
Cr-Commit-Position: refs/heads/master@{#380839}

[DaleCurtis, 2016-01-09 00:02:59]

Patchset #1 (id:1) has been deleted

[DaleCurtis, 2016-01-09 00:03:32]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs like mp3, aac, mp4, etc.

TODO: Where is webview configured? Do we need to force proprietary
codecs on for them?

TODO: Enhance webview tests to include .webm vp8 versions such that
they can still run without proprietary codecs.

BUG=570762
TEST=webview tests pass.
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs like mp3, aac, mp4, etc.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

TODO: Where is webview configured? Do we need to force proprietary
codecs on for them?

BUG=570762
TEST=webview tests pass.
==========

[DaleCurtis, 2016-03-07 23:04:43]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs like mp3, aac, mp4, etc.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

TODO: Where is webview configured? Do we need to force proprietary
codecs on for them?

BUG=570762
TEST=webview tests pass.
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs like mp3, aac, mp4, etc.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land once the launch bug is
updated.

BUG=570762
TEST=webview tests pass.
==========

[DaleCurtis, 2016-03-07 23:04:44]

dalecurtis@chromium.org changed reviewers:
+ boliu@chromium.org, ddorwin@chromium.org, dpranke@chromium.org

[DaleCurtis, 2016-03-07 23:05:31]

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

[commit-bot: I haz the power, 2016-03-07 23:06:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/60001

[commit-bot: I haz the power, 2016-03-08 00:36:00]

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

[commit-bot: I haz the power, 2016-03-08 00:36:01]

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

[DaleCurtis, 2016-03-09 00:06:12]

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

[DaleCurtis, 2016-03-09 22:02:47]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs like mp3, aac, mp4, etc.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land once the launch bug is
updated.

BUG=570762
TEST=webview tests pass.
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs like mp3, aac, mp4, etc.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
==========

[DaleCurtis, 2016-03-09 22:03:24]

Now that https://codereview.chromium.org/1773883002 is in the CQ, this is the
next step in making Android like every other Chromium platform.

[DaleCurtis, 2016-03-09 22:16:56]

For easier reviewing:
boliu@ please review android_webview/*,
dpranke@ build/*
ddorwin@ */media/*
nyquist@, chrome/android/javatests/*, */dom_distiller/*

[boliu, 2016-03-09 22:23:33]

https://codereview.chromium.org/1577433004/diff/120001/android_webview/androi...
File android_webview/android_webview_tests.gypi (right):

https://codereview.chromium.org/1577433004/diff/120001/android_webview/androi...
android_webview/android_webview_tests.gypi:37: '<(asset_location)/video.mp4',
Should the mp4 one just be removed? Is there value in testing specific codecs at
this level?

https://codereview.chromium.org/1577433004/diff/120001/android_webview/javate...
File
android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java
(right):

https://codereview.chromium.org/1577433004/diff/120001/android_webview/javate...
android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java:118:
// Android key systems only support proprietary codecs prior to Lollipop.
This comment seems backwards. Code says widevine *is supported* on or after
lollipop?

[nyquist, 2016-03-09 22:44:05]

chrome/android/javatests/*, */dom_distiller/* lgtm

[Dirk Pranke, 2016-03-09 22:58:15]

//build lgtm.

[DaleCurtis, 2016-03-09 23:58:54]

https://codereview.chromium.org/1577433004/diff/120001/android_webview/androi...
File android_webview/android_webview_tests.gypi (right):

https://codereview.chromium.org/1577433004/diff/120001/android_webview/androi...
android_webview/android_webview_tests.gypi:37: '<(asset_location)/video.mp4',
On 2016/03/09 at 22:23:33, boliu wrote:
> Should the mp4 one just be removed? Is there value in testing specific codecs
at this level?

Good point. Done.

https://codereview.chromium.org/1577433004/diff/120001/android_webview/javate...
File
android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java
(right):

https://codereview.chromium.org/1577433004/diff/120001/android_webview/javate...
android_webview/javatests/src/org/chromium/android_webview/test/KeySystemTest.java:118:
// Android key systems only support proprietary codecs prior to Lollipop.
On 2016/03/09 at 22:23:33, boliu wrote:
> This comment seems backwards. Code says widevine *is supported* on or after
lollipop?

Key systems (widevine or test) with non-proprietary codecs are only supported in
Lollipop or higher. I've inverted the statement for better readability: "Android
key systems only support non-proprietary codecs on Lollipop+."

[boliu, 2016-03-10 00:28:02]

lgtm

So cq covers K only. There are some android bots with other android versions
here towards the right: https://build.chromium.org/p/chromium.android/console

Oh it looks so sad right now :( But should still make sure they don't get worse
afterwards..

[ddorwin, 2016-03-10 01:46:39]

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
File android_webview/test/shell/assets/full_screen_video_inside_div_test.html
(right):

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
android_webview/test/shell/assets/full_screen_video_inside_div_test.html:12:
<source src="video.webm" type "video/webm">
Dropped the '=' after "type".

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
File android_webview/test/shell/assets/full_screen_video_test.html (right):

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
android_webview/test/shell/assets/full_screen_video_test.html:12: <source
src="video.webm" type "video/webm">
ditto

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
File android_webview/test/shell/assets/multiple_videos_test.html (right):

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
android_webview/test/shell/assets/multiple_videos_test.html:21: <source
src="video.webm" type "video/webm">
ditto here and below.

https://codereview.chromium.org/1577433004/diff/140001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1577433004/diff/140001/build/common.gypi#newc...
build/common.gypi:836: # Enables proprietary codecs and demuxers; e.g. H264,
MOV, AAC, and MP3.
nit: MOV as in the container? Use MP4 instead? I also suggest keeping codecs
together.

https://codereview.chromium.org/1577433004/diff/140001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1577433004/diff/140001/build/config/features....
build/config/features.gni:49: # Enables proprietary codecs and demuxers; e.g.
H264, MOV, AAC, and MP3.
Oh, now I see the source. Suggest cleaning it up here too.

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

https://codereview.chromium.org/1577433004/diff/140001/media/base/android/BUI...
media/base/android/BUILD.gn:96: sources += [ "media_codec_player_unittest.cc" ]
Comment, bug, or TODO?
Should it be rewritten to use or support VP8 or even VP9?

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp#newcod...
media/media.gyp:1330: 'test/pipeline_integration_test_base.cc',
Update media/BUILD.gn too.

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp#newcod...
media/media.gyp:1364: 'base/android/media_codec_player_unittest.cc',
ditto

[DaleCurtis, 2016-03-10 02:00:03]

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
File android_webview/test/shell/assets/full_screen_video_inside_div_test.html
(right):

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
android_webview/test/shell/assets/full_screen_video_inside_div_test.html:12:
<source src="video.webm" type "video/webm">
On 2016/03/10 at 01:46:38, ddorwin wrote:
> Dropped the '=' after "type".

Thanks, done.

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
File android_webview/test/shell/assets/full_screen_video_test.html (right):

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
android_webview/test/shell/assets/full_screen_video_test.html:12: <source
src="video.webm" type "video/webm">
On 2016/03/10 at 01:46:38, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
File android_webview/test/shell/assets/multiple_videos_test.html (right):

https://codereview.chromium.org/1577433004/diff/140001/android_webview/test/s...
android_webview/test/shell/assets/multiple_videos_test.html:21: <source
src="video.webm" type "video/webm">
On 2016/03/10 at 01:46:38, ddorwin wrote:
> ditto here and below.

Done.

https://codereview.chromium.org/1577433004/diff/140001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1577433004/diff/140001/build/common.gypi#newc...
build/common.gypi:836: # Enables proprietary codecs and demuxers; e.g. H264,
MOV, AAC, and MP3.
On 2016/03/10 at 01:46:38, ddorwin wrote:
> nit: MOV as in the container? Use MP4 instead? I also suggest keeping codecs
together.

Done.

https://codereview.chromium.org/1577433004/diff/140001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1577433004/diff/140001/build/config/features....
build/config/features.gni:49: # Enables proprietary codecs and demuxers; e.g.
H264, MOV, AAC, and MP3.
On 2016/03/10 at 01:46:38, ddorwin wrote:
> Oh, now I see the source. Suggest cleaning it up here too.

Done.

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

https://codereview.chromium.org/1577433004/diff/140001/media/base/android/BUI...
media/base/android/BUILD.gn:96: sources += [ "media_codec_player_unittest.cc" ]
On 2016/03/10 at 01:46:39, ddorwin wrote:
> Comment, bug, or TODO?
> Should it be rewritten to use or support VP8 or even VP9?

No, this will be deleted long term.

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp#newcod...
media/media.gyp:1330: 'test/pipeline_integration_test_base.cc',
On 2016/03/10 at 01:46:39, ddorwin wrote:
> Update media/BUILD.gn too.

Whoops, not sure how that got dropped.

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp#newcod...
media/media.gyp:1364: 'base/android/media_codec_player_unittest.cc',
On 2016/03/10 at 01:46:39, ddorwin wrote:
> ditto

This is already done above in media/base/android/BUILD.gn

[ddorwin, 2016-03-10 02:03:36]

lgtm

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1577433004/diff/140001/media/media.gyp#newcod...
media/media.gyp:1364: 'base/android/media_codec_player_unittest.cc',
On 2016/03/10 02:00:02, DaleCurtis wrote:
> On 2016/03/10 at 01:46:39, ddorwin wrote:
> > ditto
> 
> This is already done above in media/base/android/BUILD.gn

I meant ditto for a comment/bug/TODO. :) That wasn't clear, though. You've
already addressed that.

[DaleCurtis, 2016-03-10 02:06:41]

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

[commit-bot: I haz the power, 2016-03-10 02:07:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/160001

[DaleCurtis, 2016-03-10 02:12:56]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs like mp3, aac, mp4, etc.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
==========

[commit-bot: I haz the power, 2016-03-10 02:40:00]

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

[commit-bot: I haz the power, 2016-03-10 02:40:01]

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

[DaleCurtis, 2016-03-10 02:47:32]

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

[commit-bot: I haz the power, 2016-03-10 02:48:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/180001

[commit-bot: I haz the power, 2016-03-10 03:30:30]

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

[commit-bot: I haz the power, 2016-03-10 03:30:31]

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

[DaleCurtis, 2016-03-10 18:53:38]

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

[commit-bot: I haz the power, 2016-03-10 18:54:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/200001

[commit-bot: I haz the power, 2016-03-10 20:26:10]

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

[commit-bot: I haz the power, 2016-03-10 20:26:12]

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

[DaleCurtis, 2016-03-10 21:01:00]

Hmm, the areProprietaryCodecs() check is failing on the bots now even though
we've switched to building with proprietary codecs. I'm not able to reproduce
this locally with either a gyp (which the android bots are surprisingly still
using) or gn build.

boliu@ is there anything weird about the android_webview tests on the bots that
comes to mind?

[boliu, 2016-03-10 21:02:41]

On 2016/03/10 21:01:00, DaleCurtis wrote:
> Hmm, the areProprietaryCodecs() check is failing on the bots now even though
> we've switched to building with proprietary codecs. I'm not able to reproduce
> this locally with either a gyp (which the android bots are surprisingly still
> using) or gn build.
> 
> boliu@ is there anything weird about the android_webview tests on the bots
that
> comes to mind?

GYP_DEFINES the bot is using:
GYP_DEFINES: OS=android component=static_library dcheck_always_on=1 fastbuild=1
ffmpeg_branding=Chrome
gomadir=/b/build/slave/android/build/src/build/goma/client proprietary_codecs=1
target_arch=arm test_isolation_mode=prepare use_goma=1

Is that ffmpeg_branding=Chrome bit wrong?

[DaleCurtis, 2016-03-10 21:06:45]

On 2016/03/10 at 21:02:41, boliu wrote:
> On 2016/03/10 21:01:00, DaleCurtis wrote:
> > Hmm, the areProprietaryCodecs() check is failing on the bots now even though
> > we've switched to building with proprietary codecs. I'm not able to
reproduce
> > this locally with either a gyp (which the android bots are surprisingly
still
> > using) or gn build.
> > 
> > boliu@ is there anything weird about the android_webview tests on the bots
that
> > comes to mind?
> 
> GYP_DEFINES the bot is using:
> GYP_DEFINES: OS=android component=static_library dcheck_always_on=1
fastbuild=1 ffmpeg_branding=Chrome
gomadir=/b/build/slave/android/build/src/build/goma/client proprietary_codecs=1
target_arch=arm test_isolation_mode=prepare use_goma=1
> 
> Is that ffmpeg_branding=Chrome bit wrong?

Yeah I built locally with those defines and can't reproduce on a KitKat N4 or
Marshmallow N5. Those defines are correct now. I've flipped all the android bots
to set ffmpeg_branding+proprietary_codecs. I'll poke at some of the other bot
timeouts which seemed like flake, but might be related after all.

[boliu, 2016-03-10 22:29:48]

On 2016/03/10 21:06:45, DaleCurtis wrote:
> On 2016/03/10 at 21:02:41, boliu wrote:
> > On 2016/03/10 21:01:00, DaleCurtis wrote:
> > > Hmm, the areProprietaryCodecs() check is failing on the bots now even
though
> > > we've switched to building with proprietary codecs. I'm not able to
> reproduce
> > > this locally with either a gyp (which the android bots are surprisingly
> still
> > > using) or gn build.
> > > 
> > > boliu@ is there anything weird about the android_webview tests on the bots
> that
> > > comes to mind?
> > 
> > GYP_DEFINES the bot is using:
> > GYP_DEFINES: OS=android component=static_library dcheck_always_on=1
> fastbuild=1 ffmpeg_branding=Chrome
> gomadir=/b/build/slave/android/build/src/build/goma/client
proprietary_codecs=1
> target_arch=arm test_isolation_mode=prepare use_goma=1
> > 
> > Is that ffmpeg_branding=Chrome bit wrong?
> 
> Yeah I built locally with those defines and can't reproduce on a KitKat N4 or
> Marshmallow N5. Those defines are correct now. I've flipped all the android
bots
> to set ffmpeg_branding+proprietary_codecs. I'll poke at some of the other bot
> timeouts which seemed like flake, but might be related after all.

Then I've got nothing..

Only other thing worth mentioning is bot is using "hammerhead KTU84P", ie a
nexus 5 on that build of K. Can't imagine that would matter though?

[DaleCurtis, 2016-03-10 22:55:59]

Whoops, I was testing with 4.4.0 which doesn't support MediaDrm, so my results
were invalid. I think the real error was in how I was returning values from the
JavaScript code. Latest patch set tries to fix this. Will see how the bots do,
not sure why it doesn't repro locally though.

[DaleCurtis, 2016-03-10 22:56:35]

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

[commit-bot: I haz the power, 2016-03-10 22:57:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/220001

[commit-bot: I haz the power, 2016-03-10 23:47:08]

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

[commit-bot: I haz the power, 2016-03-10 23:47:09]

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

[DaleCurtis, 2016-03-10 23:51:56]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-03-10 23:51:57]

Android bot seems happy now. Other ones appear to be infrastructure related
timeouts, sending to CQ. Thanks!

[DaleCurtis, 2016-03-10 23:51:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, nyquist@chromium.org, boliu@chromium.org,
ddorwin@chromium.org
Link to the patchset: https://codereview.chromium.org/1577433004/#ps220001
(title: "Fix KeySystem test.")

[commit-bot: I haz the power, 2016-03-10 23:53:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/220001

[commit-bot: I haz the power, 2016-03-11 00:36:03]

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

[commit-bot: I haz the power, 2016-03-11 00:36:05]

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

[DaleCurtis, 2016-03-11 00:44:33]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2016-03-11 00:45:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/220001

[commit-bot: I haz the power, 2016-03-11 01:18:31]

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

[commit-bot: I haz the power, 2016-03-11 01:18:33]

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

[DaleCurtis, 2016-03-11 16:26:18]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2016-03-11 16:26:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/220001

[commit-bot: I haz the power, 2016-03-11 16:32:35]

Committed patchset #11 (id:220001)

[commit-bot: I haz the power, 2016-03-11 16:34:12]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.

Committed: https://crrev.com/43631fe1435e3c79467550b2cf797383d4cba8e6
Cr-Commit-Position: refs/heads/master@{#380649}
==========

[commit-bot: I haz the power, 2016-03-11 16:34:13]

Patchset 11 (id:??) landed as
https://crrev.com/43631fe1435e3c79467550b2cf797383d4cba8e6
Cr-Commit-Position: refs/heads/master@{#380649}

[boliu, 2016-03-11 20:19:01]

No CQ coverage for the gn bot either :/

https://build.chromium.org/p/chromium.linux/builders/Android%20GN/builds/33306

It uses a different version of K compared to the other bots. KOT49H (4.4.2) vs
KTU84P (4.4.4)

[DaleCurtis, 2016-03-11 20:43:52]

On 2016/03/11 at 20:19:01, boliu wrote:
> No CQ coverage for the gn bot either :/
> 
> https://build.chromium.org/p/chromium.linux/builders/Android%20GN/builds/33306
> 
> It uses a different version of K compared to the other bots. KOT49H (4.4.2) vs
KTU84P (4.4.4)

Doh, I'm away from my desk for the next hour, so please revert if this is
blocking anything. Otherwise it's likely just another s/mp4/webm change.

[pkotwicz, 2016-03-11 21:13:23]

A revert of this CL (patchset #11 id:220001) has been created in
https://codereview.chromium.org/1785173005/ by pkotwicz@chromium.org.

The reason for reverting is: Broke AndroidWebViewTest. See
https://build.chromium.org/p/chromium.linux/builders/Android%20GN.

[DaleCurtis, 2016-03-11 21:57:15]

Hmm, it's failing in GN build because video.webm isn't being copied into the
assets; so it 404s... not sure why that is happening. I can repro locally so
debugging now.

[boliu, 2016-03-11 22:12:47]

On 2016/03/11 21:57:15, DaleCurtis wrote:
> Hmm, it's failing in GN build because video.webm isn't being copied into the
> assets; so it 404s... not sure why that is happening. I can repro locally so
> debugging now.

Hmm, those are loaded using special file://android_asset, which webview
intercepts and replace with reading directly from assets:
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/na...

I don't see how gn or different android version could affect this :/

[DaleCurtis, 2016-03-11 23:15:30]

On 2016/03/11 at 22:12:47, boliu wrote:
> On 2016/03/11 21:57:15, DaleCurtis wrote:
> > Hmm, it's failing in GN build because video.webm isn't being copied into the
> > assets; so it 404s... not sure why that is happening. I can repro locally so
> > debugging now.
> 
> Hmm, those are loaded using special file://android_asset, which webview
intercepts and replace with reading directly from assets:
>
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/na...
> 
> I don't see how gn or different android version could affect this :/

Well I thought the actual file wasn't making it into the assets directory, but
after unzipping the APK I can confirm it's in there. So not sure why it can't
find it now.

[ddorwin, 2016-03-11 23:30:45]

On 2016/03/11 23:15:30, DaleCurtis wrote:
> On 2016/03/11 at 22:12:47, boliu wrote:
> > On 2016/03/11 21:57:15, DaleCurtis wrote:
> > > Hmm, it's failing in GN build because video.webm isn't being copied into
the
> > > assets; so it 404s... not sure why that is happening. I can repro locally
so
> > > debugging now.
> > 
> > Hmm, those are loaded using special file://android_asset, which webview
> intercepts and replace with reading directly from assets:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/na...
> > 
> > I don't see how gn or different android version could affect this :/
> 
> Well I thought the actual file wasn't making it into the assets directory, but
> after unzipping the APK I can confirm it's in there. So not sure why it can't
> find it now.

Could it be an isolate issue or the file not being copied to the devices? I
didn't see anything missed in a quick search.

[DaleCurtis, 2016-03-11 23:30:51]

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

[DaleCurtis, 2016-03-11 23:30:52]

adb log revealed the answer "ApkAssets: Error while loading asset
assets/video.webm: java.io.FileNotFoundException: This file can not be opened as
a file descriptor; it is probably compressed"

The fix is adding .webm to the no compress list in apkbuilder.py. dpranke@ can
you vouch for that change? I've summoned jbudorick@ as well for android OWNERS.

I'm not sure why the .gyp build is not hitting the same issue. I guess it
doesn't use build/**gyp**/apkbuilder.py? :)

[Dirk Pranke, 2016-03-11 23:48:41]

On 2016/03/11 23:30:52, DaleCurtis wrote:
> adb log revealed the answer "ApkAssets: Error while loading asset
> assets/video.webm: java.io.FileNotFoundException: This file can not be opened
as
> a file descriptor; it is probably compressed"
> 
> The fix is adding .webm to the no compress list in apkbuilder.py. dpranke@ can
> you vouch for that change? I've summoned jbudorick@ as well for android
OWNERS.

I have no idea about that; hopefully John does ...

[boliu, 2016-03-11 23:55:37]

On 2016/03/11 23:30:52, DaleCurtis wrote:
> adb log revealed the answer "ApkAssets: Error while loading asset
> assets/video.webm: java.io.FileNotFoundException: This file can not be opened
as
> a file descriptor; it is probably compressed"
> 
> The fix is adding .webm to the no compress list in apkbuilder.py. dpranke@ can
> you vouch for that change? I've summoned jbudorick@ as well for android
OWNERS.
> 
> I'm not sure why the .gyp build is not hitting the same issue. I guess it
> doesn't use build/**gyp**/apkbuilder.py? :)

Having a o_O moment. But googling around, looks like this is a known android
limitation for directly mapping assets from the apk, not something specificly to
webview (as I initially thought)

So I do think this is a legit fix. I *think* gyp doesn't use apkbuilder.py, so
issue didn't hit gyp.

[boliu, 2016-03-11 23:57:43]

Added android_chromium_gn_rel trybot. Looks like that one runs tests

[DaleCurtis, 2016-03-12 00:06:48]

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

[DaleCurtis, 2016-03-12 00:07:01]

Thanks for adding the gn one!

[commit-bot: I haz the power, 2016-03-12 00:07:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/240001

[DaleCurtis, 2016-03-12 00:22:14]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.

Committed: https://crrev.com/43631fe1435e3c79467550b2cf797383d4cba8e6
Cr-Commit-Position: refs/heads/master@{#380649}
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
==========

[jbudorick, 2016-03-12 00:46:10]

//build/android lgtm w/ nit

https://codereview.chromium.org/1577433004/diff/240001/build/android/gyp/apkb...
File build/android/gyp/apkbuilder.py (right):

https://codereview.chromium.org/1577433004/diff/240001/build/android/gyp/apkb...
build/android/gyp/apkbuilder.py:19: # Taken mostly from aapt's Package.cpp.
Additionally includes ".webm"
nit: undo this change

[DaleCurtis, 2016-03-12 00:49:16]

Will wait for GN bot to finish before hitting CQ. Thanks!

https://codereview.chromium.org/1577433004/diff/240001/build/android/gyp/apkb...
File build/android/gyp/apkbuilder.py (right):

https://codereview.chromium.org/1577433004/diff/240001/build/android/gyp/apkb...
build/android/gyp/apkbuilder.py:19: # Taken mostly from aapt's Package.cpp.
Additionally includes ".webm"
On 2016/03/12 at 00:46:10, jbudorick wrote:
> nit: undo this change

Done.

[DaleCurtis, 2016-03-12 00:49:21]

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

[DaleCurtis, 2016-03-12 00:49:37]

The CQ bit was unchecked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2016-03-12 00:49:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/260001

[DaleCurtis, 2016-03-12 00:50:42]

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

[commit-bot: I haz the power, 2016-03-12 00:51:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/280001

[Dirk Pranke, 2016-03-12 00:57:22]

On 2016/03/12 00:49:16, DaleCurtis wrote:
> Will wait for GN bot to finish before hitting CQ. Thanks!

If you add CQ_INCLUDE_TRYBOTS=tryserver.chromium.android:android_chromium_gn_rel
to your CL description,
the bot will be added to the CQ tryrun and the patch won't land if it doesn't
pass ...

[DaleCurtis, 2016-03-12 01:05:02]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.android:android_chromium_gn_rel
==========

[DaleCurtis, 2016-03-12 01:05:27]

Ah thanks! I forgot about that feature.

[DaleCurtis, 2016-03-12 01:05:40]

The CQ bit was unchecked by dalecurtis@chromium.org

[DaleCurtis, 2016-03-12 01:05:44]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-03-12 01:05:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, ddorwin@chromium.org, boliu@chromium.org,
nyquist@chromium.org, jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/1577433004/#ps280001
(title: "Undo comment change.")

[commit-bot: I haz the power, 2016-03-12 01:06:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577433004/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577433004/280001

[commit-bot: I haz the power, 2016-03-12 04:39:44]

Committed patchset #14 (id:280001)

[commit-bot: I haz the power, 2016-03-12 04:40:43]

Description was changed from

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.android:android_chromium_gn_rel
==========

to

==========
Flip proprietary codecs to false for Android chromium builds.

Official builds still using branding=Chrome so they will continue
to support codecs and containers like h264, mp3, aac, and mp4.

Adds support for video/webm videos using vp8,vorbis to the webview
tests so they will continue to work w/o proprietary codecs.

We have approval to do this now. Summary will be posted to the
src= launch bug: http://crbug.com/533190#c17

This will require some changes to the build flags for the official
WebView AOSP builders, which I'll land shortly after this.

BUG=570762
TEST=webview tests pass.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.android:android_chromium_gn_rel

Committed: https://crrev.com/8dd1542a877b87424e764ee495bd1ae6ade30492
Cr-Commit-Position: refs/heads/master@{#380839}
==========

[commit-bot: I haz the power, 2016-03-12 04:40:44]

Patchset 14 (id:??) landed as
https://crrev.com/8dd1542a877b87424e764ee495bd1ae6ade30492
Cr-Commit-Position: refs/heads/master@{#380839}