Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, MediaMetadataRetriever can get the metadata of the local video file which is selected with the file input element.

BUG=253465
Committed: https://crrev.com/0a348eea7c984eca84abc6a6e96779d7d03f3ad5
Cr-Commit-Position: refs/heads/master@{#371786}

[horo, 2016-01-25 11:04:39]

Description was changed from

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

BUG=253465
==========

to

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, we can play the local video file which is selected with the
file input element.

BUG=253465
==========

[horo, 2016-01-25 11:09:51]

horo@chromium.org changed reviewers:
+ tedchoc@chromium.org

[horo, 2016-01-25 11:09:52]

tedchoc@
Could you please review this?

[Ted C, 2016-01-25 18:16:58]

tedchoc@chromium.org changed reviewers:
+ qinmin@chromium.org

[Ted C, 2016-01-25 18:16:58]

+qinmin -- can you take a look at this?  I know nothing of this file.  Happy to
provide owners approval, but I'd like someone more familiar with the class to
say this is something we want to support.

[qinmin, 2016-01-25 18:58:56]

lgtm % nit

https://codereview.chromium.org/1626283002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java
(right):

https://codereview.chromium.org/1626283002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java:237:
Log.e(TAG, "Error configuring data source: %s", e.getMessage());
nit: you can simply do Log.e(TAG, "Error configuring data source: %s", e);

[horo, 2016-01-25 22:58:41]

Thank you!

https://codereview.chromium.org/1626283002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java
(right):

https://codereview.chromium.org/1626283002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java:237:
Log.e(TAG, "Error configuring data source: %s", e.getMessage());
On 2016/01/25 18:58:56, qinmin wrote:
> nit: you can simply do Log.e(TAG, "Error configuring data source: %s", e);

Done.

[horo, 2016-01-25 22:58:58]

The CQ bit was checked by horo@chromium.org

[horo, 2016-01-25 22:58:58]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/1626283002/#ps20001
(title: "remove ".getMessage()"")

[commit-bot: I haz the power, 2016-01-25 23:04:30]

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

[horo, 2016-01-26 03:10:13]

Description was changed from

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, we can play the local video file which is selected with the
file input element.

BUG=253465
==========

to

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, MediaMetadataRetriever can get the metadata of the local video
file which is selected with the file input element.

BUG=253465
==========

[horo, 2016-01-26 03:10:13]

Description was changed from

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, we can play the local video file which is selected with the
file input element.

BUG=253465
==========

to

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, MediaMetadataRetriever can get the metadata of the local video
file which is selected with the file input element.

BUG=253465
==========

[horo, 2016-01-26 18:18:49]

tedchoc@

Ah, I need OWNER's l-g-t-m.

[Ted C, 2016-01-26 21:49:02]

https://codereview.chromium.org/1626283002/diff/40001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java
(right):

https://codereview.chromium.org/1626283002/diff/40001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java:410:
void configure(Context context, Uri uri) {
this should be private since it is not used outside of this class.  also, it
seems the others are @VisibleForTesting as they are covered by tests of some
sort.  Should you add a test for this new path?

[horo, 2016-01-27 06:46:57]

https://codereview.chromium.org/1626283002/diff/40001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java
(right):

https://codereview.chromium.org/1626283002/diff/40001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java:410:
void configure(Context context, Uri uri) {
On 2016/01/26 21:49:02, Ted C wrote:
> this should be private since it is not used outside of this class.  also, it
> seems the others are @VisibleForTesting as they are covered by tests of some
> sort.  Should you add a test for this new path?

Done.
Added unit tests.

[Ted C, 2016-01-27 10:54:45]

lgtm

[horo, 2016-01-27 14:50:29]

The CQ bit was checked by horo@chromium.org

[horo, 2016-01-27 14:50:29]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/1626283002/#ps60001
(title: "add unit tests")

[commit-bot: I haz the power, 2016-01-27 14:50:49]

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

[commit-bot: I haz the power, 2016-01-27 14:55:17]

Description was changed from

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, MediaMetadataRetriever can get the metadata of the local video
file which is selected with the file input element.

BUG=253465
==========

to

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, MediaMetadataRetriever can get the metadata of the local video
file which is selected with the file input element.

BUG=253465
==========

[commit-bot: I haz the power, 2016-01-27 14:55:18]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-27 14:56:31]

Description was changed from

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, MediaMetadataRetriever can get the metadata of the local video
file which is selected with the file input element.

BUG=253465
==========

to

==========
Use MediaMetadataRetriever.setDataSource(Context, Uri) for "content" scheme.

This cl will fix the bug reported at https://crbug.com/253465#c26.
With this patch, MediaMetadataRetriever can get the metadata of the local video
file which is selected with the file input element.

BUG=253465

Committed: https://crrev.com/0a348eea7c984eca84abc6a6e96779d7d03f3ad5
Cr-Commit-Position: refs/heads/master@{#371786}
==========

[commit-bot: I haz the power, 2016-01-27 14:56:32]

Patchset 4 (id:??) landed as
https://crrev.com/0a348eea7c984eca84abc6a6e96779d7d03f3ad5
Cr-Commit-Position: refs/heads/master@{#371786}