[Android WebView] AwMediaUrlInterceptor to handle media assets.

[Android WebView] AwMediaUrlInterceptor to handle media assets.

This change registers an AwMediaUrlInterceptor to enable apps
to play media assets.

This is a follow-up change to:
https://codereview.chromium.org/411353002/

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

[Ignacio Solla, 2014-07-24 18:07:36]

Is it modifying the DEPS files ok?

benm@chromium.org: Please review changes in 

mnaganov@chromium.org: Please review changes in

[mnaganov (inactive), 2014-07-25 08:28:54]

I think DEPS changes are OK.

Have you also tried doing a component build (adding "component=shared_library"
to GYP_DEFINES)? It usually reveals lacking exports for classes that are used in
unit tests.

https://codereview.chromium.org/415043006/diff/1/android_webview/DEPS
File android_webview/DEPS (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/DEPS#newcode1
android_webview/DEPS:1: # Please include joth@ and (joi@ or erikwright@) on the
review for any changes
benm@: Should we replace joth@ with you here?

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
File android_webview/native/aw_media_url_interceptor.cc (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor.cc:13: bool
AwMediaUrlInterceptor::Intercept(const std::string& url,
'namespace' doesn't increase indentation. This code must not be indented.

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor.cc:15: const std::string
asset_file_prefix("file:///android_asset/");
Please use android_webview/common/url_constants.h for file prefixes.

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
File android_webview/native/aw_media_url_interceptor.h (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor.h:22: int64* size) const;
OVERRIDE?

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
File android_webview/native/aw_media_url_interceptor_unittest.cc (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor_unittest.cc:24: :
fd_(UNSET_VALUE), offset_(UNSET_VALUE), size_(UNSET_VALUE),
It is preferred to initialize / dispose test variables in overridden 'SetUp' and
'TearDown' methods. Otherwise you are potentially adding dependencies between
tests (that is, running one test can affect results of subsequent tests).

[Ignacio Solla, 2014-07-25 11:05:48]

https://codereview.chromium.org/415043006/diff/1/android_webview/DEPS
File android_webview/DEPS (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/DEPS#newcode1
android_webview/DEPS:1: # Please include joth@ and (joi@ or erikwright@) on the
review for any changes
On 2014/07/25 08:28:54, Mikhail Naganov (Cr) wrote:
> benm@: Should we replace joth@ with you here?

I have tentatively replaced them, please benm confirm that this is ok.

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
File android_webview/native/aw_media_url_interceptor.cc (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor.cc:13: bool
AwMediaUrlInterceptor::Intercept(const std::string& url,
On 2014/07/25 08:28:54, Mikhail Naganov (Cr) wrote:
> 'namespace' doesn't increase indentation. This code must not be indented.

Done.

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor.cc:15: const std::string
asset_file_prefix("file:///android_asset/");
On 2014/07/25 08:28:54, Mikhail Naganov (Cr) wrote:
> Please use android_webview/common/url_constants.h for file prefixes.

Done.

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
File android_webview/native/aw_media_url_interceptor.h (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor.h:22: int64* size) const;
On 2014/07/25 08:28:54, Mikhail Naganov (Cr) wrote:
> OVERRIDE?

Done.

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
File android_webview/native/aw_media_url_interceptor_unittest.cc (right):

https://codereview.chromium.org/415043006/diff/1/android_webview/native/aw_me...
android_webview/native/aw_media_url_interceptor_unittest.cc:24: :
fd_(UNSET_VALUE), offset_(UNSET_VALUE), size_(UNSET_VALUE),
On 2014/07/25 08:28:54, Mikhail Naganov (Cr) wrote:
> It is preferred to initialize / dispose test variables in overridden 'SetUp'
and
> 'TearDown' methods. Otherwise you are potentially adding dependencies between
> tests (that is, running one test can affect results of subsequent tests).

The tests are still isolated when using the constructor, and it is actually
preferred in this case:

https://code.google.com/p/googletest/wiki/FAQ#Should_I_use_the_constructor/de...

[mnaganov (inactive), 2014-07-25 11:18:05]

LGTM % comments

Thanks for pointing out the guideline about test class instances! I was unaware
of the fact that they are re-created for each test.

https://codereview.chromium.org/415043006/diff/20001/android_webview/DEPS
File android_webview/DEPS (right):

https://codereview.chromium.org/415043006/diff/20001/android_webview/DEPS#new...
android_webview/DEPS:1: # Please include benm@ and (joi@ or erikwright@) on the
review for any changes
BTW, joi@ had quit Google and stopped working on Chromium, so he should be
removed from this list. Sorry for missing this on the first round.

https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
File android_webview/native/aw_media_url_interceptor.h (right):

https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
android_webview/native/aw_media_url_interceptor.h:20: int* fd,
nit: Are you sure that indentation of arguments is correct here? Looks strange
to me.

[Ignacio Solla, 2014-07-25 13:12:22]

Thanks (btw, junit tests also recreate TestCase objects for every case)

In my previous reply I forgot to mention that I have done a component build and
that it revealed no errors. This is precisely what I've done:
GYP_DEFINES=component=shared_library
android_gyp
ninja -C out/Debug -j200 -l30 android_webview_apk

https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
File android_webview/native/aw_media_url_interceptor.h (right):

https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
android_webview/native/aw_media_url_interceptor.h:20: int* fd,
On 2014/07/25 11:18:05, Mikhail Naganov (Cr) wrote:
> nit: Are you sure that indentation of arguments is correct here? Looks strange
> to me.

Sorry I missed this.

[mnaganov (inactive), 2014-07-25 13:18:41]

On 2014/07/25 13:12:22, Ignacio Solla wrote:
> Thanks (btw, junit tests also recreate TestCase objects for every case)
> 
> In my previous reply I forgot to mention that I have done a component build
and
> that it revealed no errors. This is precisely what I've done:
> GYP_DEFINES=component=shared_library
> android_gyp
> ninja -C out/Debug -j200 -l30 android_webview_apk

Thanks. But you also modify 'android_webview_unittests_apk'. Have you tried
building it?

> 
>
https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
> File android_webview/native/aw_media_url_interceptor.h (right):
> 
>
https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
> android_webview/native/aw_media_url_interceptor.h:20: int* fd,
> On 2014/07/25 11:18:05, Mikhail Naganov (Cr) wrote:
> > nit: Are you sure that indentation of arguments is correct here? Looks
strange
> > to me.
> 
> Sorry I missed this.

[Ignacio Solla, 2014-07-25 13:22:57]

On 2014/07/25 13:18:41, Mikhail Naganov (Cr) wrote:
> On 2014/07/25 13:12:22, Ignacio Solla wrote:
> > Thanks (btw, junit tests also recreate TestCase objects for every case)
> > 
> > In my previous reply I forgot to mention that I have done a component build
> and
> > that it revealed no errors. This is precisely what I've done:
> > GYP_DEFINES=component=shared_library
> > android_gyp
> > ninja -C out/Debug -j200 -l30 android_webview_apk
> 
> Thanks. But you also modify 'android_webview_unittests_apk'. Have you tried
> building it?
> 
> > 
> >
>
https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
> > File android_webview/native/aw_media_url_interceptor.h (right):
> > 
> >
>
https://codereview.chromium.org/415043006/diff/20001/android_webview/native/a...
> > android_webview/native/aw_media_url_interceptor.h:20: int* fd,
> > On 2014/07/25 11:18:05, Mikhail Naganov (Cr) wrote:
> > > nit: Are you sure that indentation of arguments is correct here? Looks
> strange
> > > to me.
> > 
> > Sorry I missed this.

Yes, it builds.

[Ignacio Solla, 2014-07-30 14:03:08]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-07-30 14:03:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/igsolla@chromium.org/415043006/20002

[commit-bot: I haz the power, 2014-07-30 15:18:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/35320)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-07-30 15:20:12]

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

[commit-bot: I haz the power, 2014-07-30 15:20:12]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/35344)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)

[Ignacio Solla, 2014-07-31 11:18:53]

The CQ bit was checked by igsolla@chromium.org

[Ignacio Solla, 2014-07-31 11:19:01]

The CQ bit was unchecked by igsolla@chromium.org

[Ignacio Solla, 2014-07-31 11:25:55]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-07-31 11:28:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/igsolla@chromium.org/415043006/50001

[commit-bot: I haz the power, 2014-07-31 14:56:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-07-31 15:00:38]

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

[commit-bot: I haz the power, 2014-07-31 15:00:40]

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

[Ignacio Solla, 2014-07-31 15:30:54]

I'm not able to land this change due to the following error:

** Presubmit ERRORS **
Missing LGTM from OWNERS of dependencies added to DEPS:
    '+content/browser/media/android',
    '+media/base',

- xhwang: please review the dependency on media/base
- qinmin: please review the dependency on content/browser/media/android

[xhwang, 2014-07-31 16:46:24]

https://codereview.chromium.org/415043006/diff/50001/android_webview/native/a...
File android_webview/native/aw_media_url_interceptor.h (right):

https://codereview.chromium.org/415043006/diff/50001/android_webview/native/a...
android_webview/native/aw_media_url_interceptor.h:12: #include
"media/base/media_export.h"
Is this needed? If not, the dependency can be more specific, e.g.

+media/base/android

[Ignacio Solla, 2014-08-01 09:29:42]

https://codereview.chromium.org/415043006/diff/50001/android_webview/native/a...
File android_webview/native/aw_media_url_interceptor.h (right):

https://codereview.chromium.org/415043006/diff/50001/android_webview/native/a...
android_webview/native/aw_media_url_interceptor.h:12: #include
"media/base/media_export.h"
On 2014/07/31 16:46:24, xhwang wrote:
> Is this needed? If not, the dependency can be more specific, e.g.
> 
> +media/base/android

Good catch, actually it is not needed. Removed.

[qinmin, 2014-08-01 16:34:49]

lgtm

[Ignacio Solla, 2014-08-01 16:35:37]

The CQ bit was checked by igsolla@chromium.org

[Ignacio Solla, 2014-08-01 16:36:00]

The CQ bit was unchecked by igsolla@chromium.org

[xhwang, 2014-08-01 16:38:16]

lgtm

[Ignacio Solla, 2014-08-01 16:39:02]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-08-01 16:42:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/igsolla@chromium.org/415043006/70001

[commit-bot: I haz the power, 2014-08-01 20:36:40]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/36544)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/41580)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-01 20:41:36]

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

[commit-bot: I haz the power, 2014-08-01 20:41:37]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/36592)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[Ignacio Solla, 2014-08-04 09:04:03]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-08-04 09:05:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/igsolla@chromium.org/415043006/90001

[commit-bot: I haz the power, 2014-08-04 10:44:14]

Change committed as 287315