Parsing filesystem url before giving it to media player

Parsing filesystem url before giving it to media player

Android media player directly takes a url to playback a video and it cannot handle filesystem urls.
To solve the problem, we need to translate a filesystem url to a platform path before calling setDataSource().
CookieGetter is renamed to MediaResourceGetter to handle such translations.

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

[qinmin, 2013-03-08 02:27:58]

PTAL

[kinuko, 2013-03-08 02:43:20]

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
File content/browser/android/media_resource_getter_impl.cc (right):

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
content/browser/android/media_resource_getter_impl.cc:163: &platform_path);
If you can implement this with async API, can you instead use GetMetadata?  It
also returns platform path (if available) but in an async way.

[qinmin, 2013-03-08 03:09:27]

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
File content/browser/android/media_resource_getter_impl.cc (right):

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
content/browser/android/media_resource_getter_impl.cc:163: &platform_path);
For GetMetadata(), does it has to be on the IO thread? or can it be invoked on
any thread?

On 2013/03/08 02:43:21, kinuko wrote:
> If you can implement this with async API, can you instead use GetMetadata?  It
> also returns platform path (if available) but in an async way.

[kinuko, 2013-03-08 03:14:32]

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
File content/browser/android/media_resource_getter_impl.cc (right):

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
content/browser/android/media_resource_getter_impl.cc:163: &platform_path);
On 2013/03/08 03:09:28, qinmin wrote:
> For GetMetadata(), does it has to be on the IO thread? or can it be invoked on
> any thread?
> 
> On 2013/03/08 02:43:21, kinuko wrote:
> > If you can implement this with async API, can you instead use GetMetadata? 
It
> > also returns platform path (if available) but in an async way.
> 

Ah- I see you're already on File thread, sorry I missed the part.  Then this
looks ok to me. (I'll take another brief look at other files)

[scherkus (not reviewing), 2013-03-08 21:26:27]

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
File content/browser/android/media_resource_getter_impl.cc (right):

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
content/browser/android/media_resource_getter_impl.cc:123: : public
base::RefCountedThreadSafe<PlatformPathGetterTask> {
should be 4 space indent

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_play...
File media/base/android/media_player_bridge.h (right):

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.h:78: const std::string& url,
hmm... it'd be nice to use a GURL instead of a string for URL

is it a big change to get this updated? that way you can use GURL::SchemeIs()
and avoid your own string parsing code

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.h:209: // Cookies for |url_|
nit: this should end w/ period

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_reso...
File media/base/android/media_resource_getter.h (right):

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_reso...
media/base/android/media_resource_getter.h:31: const std::string& url,
ditto for GURL if it's possible

[qinmin, 2013-03-08 22:36:12]

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
File content/browser/android/media_resource_getter_impl.cc (right):

https://codereview.chromium.org/12595005/diff/1/content/browser/android/media...
content/browser/android/media_resource_getter_impl.cc:123: : public
base::RefCountedThreadSafe<PlatformPathGetterTask> {
On 2013/03/08 21:26:27, scherkus wrote:
> should be 4 space indent

Done.

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_play...
File media/base/android/media_player_bridge.h (right):

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.h:78: const std::string& url,
Fixed. 
I remember I tried to use GURL in the past. But somehow media/ does not have
dependencies on googleurl. Even today, GURL is not used anywhere in media/. 
Just checked media.gyp, looks like the dependency is fixed. But the wierd thing
is that we don't use it anywhere.

On 2013/03/08 21:26:27, scherkus wrote:
> hmm... it'd be nice to use a GURL instead of a string for URL
> 
> is it a big change to get this updated? that way you can use GURL::SchemeIs()
> and avoid your own string parsing code

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.h:209: // Cookies for |url_|
On 2013/03/08 21:26:27, scherkus wrote:
> nit: this should end w/ period

Done.

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_reso...
File media/base/android/media_resource_getter.h (right):

https://codereview.chromium.org/12595005/diff/1/media/base/android/media_reso...
media/base/android/media_resource_getter.h:31: const std::string& url,
On 2013/03/08 21:26:27, scherkus wrote:
> ditto for GURL if it's possible

Done.

[scherkus (not reviewing), 2013-03-11 20:28:33]

lgtm w/ nits

https://codereview.chromium.org/12595005/diff/9001/media/base/android/media_p...
File media/base/android/media_player_bridge.h (right):

https://codereview.chromium.org/12595005/diff/9001/media/base/android/media_p...
media/base/android/media_player_bridge.h:143: // Callback function passed to
|cookies_retriever_|.
fix comment

https://codereview.chromium.org/12595005/diff/9001/media/base/android/media_p...
media/base/android/media_player_bridge.h:147: void SetDataSource(const
std::string& url);
sanity check: does this (and other methods) need to be public?

[qinmin, 2013-03-11 21:12:16]

https://codereview.chromium.org/12595005/diff/9001/media/base/android/media_p...
File media/base/android/media_player_bridge.h (right):

https://codereview.chromium.org/12595005/diff/9001/media/base/android/media_p...
media/base/android/media_player_bridge.h:143: // Callback function passed to
|cookies_retriever_|.
On 2013/03/11 20:28:33, scherkus wrote:
> fix comment

Done.

https://codereview.chromium.org/12595005/diff/9001/media/base/android/media_p...
media/base/android/media_player_bridge.h:147: void SetDataSource(const
std::string& url);
Moved to private

On 2013/03/11 20:28:33, scherkus wrote:
> sanity check: does this (and other methods) need to be public?

[ericu, 2013-03-11 21:13:10]

I can't help but think that it's weird to have people calling directly into a
message filter, rather than coming in via a message.  However, there's a fair
amount of intelligence in this layer, so you can't just call into the underlying
code without exposing more than I'd like.  So the file API part LGTM unless
anyone has a better idea.

I haven't read the rest.

[kinuko, 2013-03-12 07:59:43]

https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
content/browser/fileapi/fileapi_message_filter.cc:58: bool
CheckFilePermissionsForProcess(
We're going to add a ChildProcessSecurityPolicy method for permission check.  In
that case you won't need to call into a method of FileAPIMessageFilter?
As Eric noted it looks a bit weird to call into FileAPIMessageFilter not for
message filtering.

[qinmin, 2013-03-12 16:03:31]

https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
content/browser/fileapi/fileapi_message_filter.cc:58: bool
CheckFilePermissionsForProcess(
If that method is provided, it is probably ok to remove this function, but then
I still need a method to translate filesystem url to a platform file path, on
the FILE thread.
Maybe adding another util class that implements a static
OnGetPlatformPathFromFileSystemURL()? 

On 2013/03/12 07:59:43, kinuko wrote:
> We're going to add a ChildProcessSecurityPolicy method for permission check. 
In
> that case you won't need to call into a method of FileAPIMessageFilter?
> As Eric noted it looks a bit weird to call into FileAPIMessageFilter not for
> message filtering.

[qinmin, 2013-03-13 18:34:49]

hi, kinuko, are you ok with the current change? or do you want me to move
"static void FileapiMessageFilter::SyncGetPlatformPath()" into another util
class? In that case, the util class can call methods in
ChildProcessSecurityPolicy if that is added，
Thanks

On 2013/03/12 16:03:31, qinmin wrote:
>
https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
> File content/browser/fileapi/fileapi_message_filter.cc (right):
> 
>
https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
> content/browser/fileapi/fileapi_message_filter.cc:58: bool
> CheckFilePermissionsForProcess(
> If that method is provided, it is probably ok to remove this function, but
then
> I still need a method to translate filesystem url to a platform file path, on
> the FILE thread.
> Maybe adding another util class that implements a static
> OnGetPlatformPathFromFileSystemURL()? 
> 
> On 2013/03/12 07:59:43, kinuko wrote:
> > We're going to add a ChildProcessSecurityPolicy method for permission check.

> In
> > that case you won't need to call into a method of FileAPIMessageFilter?
> > As Eric noted it looks a bit weird to call into FileAPIMessageFilter not for
> > message filtering.

[kinuko, 2013-03-14 00:21:47]

Hi, sorry for my late reply. I was hoping that the ChildProcessSecurityPolicy
can be made anytime soon (it's under review) but it looks to take more time. 
For now could we move both the HasPermissionsForFile and the
SyncGetPlatformPath() helper method into
content/browser/fileapi/browser_file_system_helper.h and call it from your code
and from FileAPIMessage?  Later when we actually integrate the permission check
method we can simply remove HasPermissionsForFile method while keeping the
SyncGetPlatformPath.  WDYT?

THanks,

On 2013/03/13 18:34:49, qinmin wrote:
> hi, kinuko, are you ok with the current change? or do you want me to move
> "static void FileapiMessageFilter::SyncGetPlatformPath()" into another util
> class? In that case, the util class can call methods in
> ChildProcessSecurityPolicy if that is added，
> Thanks
> 
> On 2013/03/12 16:03:31, qinmin wrote:
> >
>
https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
> > File content/browser/fileapi/fileapi_message_filter.cc (right):
> > 
> >
>
https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
> > content/browser/fileapi/fileapi_message_filter.cc:58: bool
> > CheckFilePermissionsForProcess(
> > If that method is provided, it is probably ok to remove this function, but
> then
> > I still need a method to translate filesystem url to a platform file path,
on
> > the FILE thread.
> > Maybe adding another util class that implements a static
> > OnGetPlatformPathFromFileSystemURL()? 
> > 
> > On 2013/03/12 07:59:43, kinuko wrote:
> > > We're going to add a ChildProcessSecurityPolicy method for permission
check.
> 
> > In
> > > that case you won't need to call into a method of FileAPIMessageFilter?
> > > As Eric noted it looks a bit weird to call into FileAPIMessageFilter not
for
> > > message filtering.

[qinmin, 2013-03-14 02:36:13]

Hi, Kinuko. Just made the patch to move all the calls from FileAPIMessageFilter
to BrowserFileSystemHelper. PTAL.

On 2013/03/14 00:21:47, kinuko wrote:
> Hi, sorry for my late reply. I was hoping that the ChildProcessSecurityPolicy
> can be made anytime soon (it's under review) but it looks to take more time. 
> For now could we move both the HasPermissionsForFile and the
> SyncGetPlatformPath() helper method into
> content/browser/fileapi/browser_file_system_helper.h and call it from your
code
> and from FileAPIMessage?  Later when we actually integrate the permission
check
> method we can simply remove HasPermissionsForFile method while keeping the
> SyncGetPlatformPath.  WDYT?
> 
> THanks,
> 
> On 2013/03/13 18:34:49, qinmin wrote:
> > hi, kinuko, are you ok with the current change? or do you want me to move
> > "static void FileapiMessageFilter::SyncGetPlatformPath()" into another util
> > class? In that case, the util class can call methods in
> > ChildProcessSecurityPolicy if that is added，
> > Thanks
> > 
> > On 2013/03/12 16:03:31, qinmin wrote:
> > >
> >
>
https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
> > > File content/browser/fileapi/fileapi_message_filter.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/12595005/diff/13001/content/browser/fileapi/f...
> > > content/browser/fileapi/fileapi_message_filter.cc:58: bool
> > > CheckFilePermissionsForProcess(
> > > If that method is provided, it is probably ok to remove this function, but
> > then
> > > I still need a method to translate filesystem url to a platform file path,
> on
> > > the FILE thread.
> > > Maybe adding another util class that implements a static
> > > OnGetPlatformPathFromFileSystemURL()? 
> > > 
> > > On 2013/03/12 07:59:43, kinuko wrote:
> > > > We're going to add a ChildProcessSecurityPolicy method for permission
> check.
> > 
> > > In
> > > > that case you won't need to call into a method of FileAPIMessageFilter?
> > > > As Eric noted it looks a bit weird to call into FileAPIMessageFilter not
> for
> > > > message filtering.

[kinuko, 2013-03-14 03:11:44]

Cool, fileapi changes lgtm (+nits for naming)

Thanks!

https://codereview.chromium.org/12595005/diff/28001/content/browser/fileapi/b...
File content/browser/fileapi/browser_file_system_helper.h (right):

https://codereview.chromium.org/12595005/diff/28001/content/browser/fileapi/b...
content/browser/fileapi/browser_file_system_helper.h:35: CONTENT_EXPORT bool
CheckFilePermissionsForProcess(
Can rename this to CheckFileSystemPermissionsForProcess or
CheckFileAPIPermissionsForProcess so that it becomes apparent it's for
FileSystem/FileAPI?

[qinmin, 2013-03-14 19:24:03]

jam, would you please take a look at this? need OWNERS stamp on content/

https://codereview.chromium.org/12595005/diff/28001/content/browser/fileapi/b...
File content/browser/fileapi/browser_file_system_helper.h (right):

https://codereview.chromium.org/12595005/diff/28001/content/browser/fileapi/b...
content/browser/fileapi/browser_file_system_helper.h:35: CONTENT_EXPORT bool
CheckFilePermissionsForProcess(
On 2013/03/14 03:11:44, kinuko wrote:
> Can rename this to CheckFileSystemPermissionsForProcess or
> CheckFileAPIPermissionsForProcess so that it becomes apparent it's for
> FileSystem/FileAPI?

Done.

[ericu, 2013-03-14 21:45:18]

That's much better, thanks.

https://codereview.chromium.org/12595005/diff/33001/content/browser/fileapi/f...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/12595005/diff/33001/content/browser/fileapi/f...
content/browser/fileapi/fileapi_message_filter.cc:510:
SyncGetPlatformPath(context_, process_id_,
Unwrap line?  It looks like it would fit.

[qinmin, 2013-03-14 21:47:37]

https://codereview.chromium.org/12595005/diff/33001/content/browser/fileapi/f...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/12595005/diff/33001/content/browser/fileapi/f...
content/browser/fileapi/fileapi_message_filter.cc:510:
SyncGetPlatformPath(context_, process_id_,
On 2013/03/14 21:45:18, ericu wrote:
> Unwrap line?  It looks like it would fit.

Done.

[qinmin, 2013-03-15 17:57:47]

jam, would you please give some OWNER's blessing?
Media and fileapi related changes are reviewed. But i still need OWNER stamp for
content/content_browser.gypi.

On 2013/03/14 21:47:37, qinmin wrote:
>
https://codereview.chromium.org/12595005/diff/33001/content/browser/fileapi/f...
> File content/browser/fileapi/fileapi_message_filter.cc (right):
> 
>
https://codereview.chromium.org/12595005/diff/33001/content/browser/fileapi/f...
> content/browser/fileapi/fileapi_message_filter.cc:510:
> SyncGetPlatformPath(context_, process_id_,
> On 2013/03/14 21:45:18, ericu wrote:
> > Unwrap line?  It looks like it would fit.
> 
> Done.

[qinmin, 2013-03-15 23:51:37]

+yfriedman for content/browser/android

[jam, 2013-03-18 17:05:52]

On 2013/03/15 17:57:47, qinmin wrote:
> jam, would you please give some OWNER's blessing?
> Media and fileapi related changes are reviewed. But i still need OWNER stamp
for
> content/content_browser.gypi.

lgtm

[commit-bot: I haz the power, 2013-03-18 17:17:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qinmin@chromium.org/12595005/39001

[commit-bot: I haz the power, 2013-03-18 17:17:58]

Presubmit check for 12595005-39001 failed and returned exit status 1.

INFO:root:Found 20 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    content/common/media/media_player_messages.h

Presubmit checks took 5.2s to calculate.

[qinmin, 2013-03-18 17:30:26]

+palmer for IPC

[palmer, 2013-03-18 19:54:33]

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/an...
File content/browser/android/media_resource_getter_impl.cc (right):

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/an...
content/browser/android/media_resource_getter_impl.cc:161:
SyncGetPlatformPath(file_system_context_,
At some point, possibly here or inside SyncGetPlatformPath, we need to impose
some restrictions on the resulting pathname. We should refuse to open paths that
are outside the /sdcard directory (or equivalent). Talk to nileshagrawal.

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/an...
content/browser/android/media_resource_getter_impl.cc:182: const GURL&
first_party_for_cookies,
I'm new to this area of code; please forgive my ignorance.  Is there some chance
that we'll include cookies in requests for URLs that resolve to a local file
path? That wouldn't make any sense.

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/fi...
File content/browser/fileapi/browser_file_system_helper.cc (right):

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/fi...
content/browser/fileapi/browser_file_system_helper.cc:72: bool
CheckFileSystemPermissionsForProcess(
Yes, I think it is in here that we would/should enforce a pathname restriction. 
I.e. that pathnames must point to a node below /scard or equivalent). Ping me
privately for more info.

https://chromiumcodereview.appspot.com/12595005/diff/39001/media/base/android...
File media/base/android/media_player_bridge.h (right):

https://chromiumcodereview.appspot.com/12595005/diff/39001/media/base/android...
media/base/android/media_player_bridge.h:16: #include "base/string_util.h"
Do you need this?

https://chromiumcodereview.appspot.com/12595005/diff/39001/media/base/android...
media/base/android/media_player_bridge.h:79: const GURL& url,
Thanks for changing from string to GURL, btw.

[qinmin, 2013-03-18 22:27:58]

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/an...
File content/browser/android/media_resource_getter_impl.cc (right):

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/an...
content/browser/android/media_resource_getter_impl.cc:161:
SyncGetPlatformPath(file_system_context_,
For filesystem url, the data are actually stored under app data directory, not
external storage. Added a check to see if the platform_path is a child of app
data dir.

On 2013/03/18 19:54:34, Chris P. wrote:
> At some point, possibly here or inside SyncGetPlatformPath, we need to impose
> some restrictions on the resulting pathname. We should refuse to open paths
that
> are outside the /sdcard directory (or equivalent). Talk to nileshagrawal.

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/an...
content/browser/android/media_resource_getter_impl.cc:182: const GURL&
first_party_for_cookies,
added a line in MediaPlayerBridge ctor to set has_cookies_ to true for
FileSystem URLs so that we will by pass GetCookies call here.

On 2013/03/18 19:54:34, Chris P. wrote:
> I'm new to this area of code; please forgive my ignorance.  Is there some
chance
> that we'll include cookies in requests for URLs that resolve to a local file
> path? That wouldn't make any sense.

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/fi...
File content/browser/fileapi/browser_file_system_helper.cc (right):

https://chromiumcodereview.appspot.com/12595005/diff/39001/content/browser/fi...
content/browser/fileapi/browser_file_system_helper.cc:72: bool
CheckFileSystemPermissionsForProcess(
added the check in MediaResourceGetterImpl.

On 2013/03/18 19:54:34, Chris P. wrote:
> Yes, I think it is in here that we would/should enforce a pathname
restriction. 
> I.e. that pathnames must point to a node below /scard or equivalent). Ping me
> privately for more info.

https://chromiumcodereview.appspot.com/12595005/diff/39001/media/base/android...
File media/base/android/media_player_bridge.h (right):

https://chromiumcodereview.appspot.com/12595005/diff/39001/media/base/android...
media/base/android/media_player_bridge.h:16: #include "base/string_util.h"
removed.
On 2013/03/18 19:54:34, Chris P. wrote:
> Do you need this?

[Yaron, 2013-03-18 23:05:44]

lgtm

[palmer, 2013-03-18 23:42:36]

lgtm

https://chromiumcodereview.appspot.com/12595005/diff/50002/content/browser/an...
File content/browser/android/media_resource_getter_impl.cc (right):

https://chromiumcodereview.appspot.com/12595005/diff/50002/content/browser/an...
content/browser/android/media_resource_getter_impl.cc:167:
PathService::Get(base::DIR_ANDROID_APP_DATA, &external_storage_path);
Great, thanks.

[commit-bot: I haz the power, 2013-03-18 23:43:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qinmin@chromium.org/12595005/50002

[commit-bot: I haz the power, 2013-03-19 01:26:30]

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

[commit-bot: I haz the power, 2013-03-19 02:44:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qinmin@chromium.org/12595005/50002

[commit-bot: I haz the power, 2013-03-19 02:44:29]

Failed to apply patch for content/content_browser.gypi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/content_browser.gypi
  Hunk #1 succeeded at 228 (offset 2 lines).
  Hunk #2 FAILED at 237.
  1 out of 2 hunks FAILED -- saving rejects to file
content/content_browser.gypi.rej

Patch:       content/content_browser.gypi
Index: content/content_browser.gypi
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index
b82d96e25d209460ed227b4a6721e4e2d34b8884..c5111a03bb0af6fcee4fa3bc80ced53d3181fd47
100644
--- a/content/content_browser.gypi
+++ b/content/content_browser.gypi
@@ -226,8 +226,6 @@
     'browser/android/content_view_render_view.h',
     'browser/android/content_view_statics.cc',
     'browser/android/content_view_statics.h',
-    'browser/android/cookie_getter_impl.cc',
-    'browser/android/cookie_getter_impl.h',
     'browser/android/date_time_chooser_android.cc',
     'browser/android/date_time_chooser_android.h',
     'browser/android/download_controller_android_impl.cc',
@@ -239,6 +237,8 @@
     'browser/android/load_url_params.h',
     'browser/android/media_player_manager_android.cc',
     'browser/android/media_player_manager_android.h',
+    'browser/android/media_resource_getter_impl.cc',
+    'browser/android/media_resource_getter_impl.h',
     'browser/android/sandboxed_process_launcher.cc',
     'browser/android/sandboxed_process_launcher.h',
     'browser/android/surface_texture_peer_browser_impl.cc',

[commit-bot: I haz the power, 2013-03-19 16:59:31]

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

[commit-bot: I haz the power, 2013-03-19 22:04:21]

Change committed as 189126