Expose a seekable range at 0 for streaming sources with duration.

Expose a seekable range at 0 for streaming sources with duration.

Allows looping to work with 200-type sources.

BUG=412562
TEST=forthcoming layout tests.

Committed: https://crrev.com/dfe0527f2c241795d4574503360a531a8221bfa0
Cr-Commit-Position: refs/heads/master@{#298565}

[DaleCurtis, 2014-10-01 00:08:25]

dalecurtis@chromium.org changed reviewers:
+ philipj@opera.com

[philipj_slow, 2014-10-01 09:37:50]

Non-owner LGTM with nits.

https://codereview.chromium.org/621573002/diff/1/content/renderer/media/andro...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/621573002/diff/1/content/renderer/media/andro...
content/renderer/media/android/webmediaplayer_android.cc:531: if
(!base::IsFinite(seekable_end))
With this change I believe you can remove the readyState check above, because
duration() already returns NaN for that case.

https://codereview.chromium.org/621573002/diff/1/media/blink/webmediaplayer_i...
File media/blink/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/621573002/diff/1/media/blink/webmediaplayer_i...
media/blink/webmediaplayer_impl.cc:499: if (data_source_ &&
data_source_->IsStreaming()) {
Can you DCHECK(base::IsFinite(seekable_end) && seekable_end > 0.0) in an else
branch, or does that not always hold true? The previous code clearly never
returned a [0,0] range, but now it's not clear if it's possible for the
non-streaming case or not.

[DaleCurtis, 2014-10-06 23:54:54]

Patchset #2 (id:20001) has been deleted

[DaleCurtis, 2014-10-06 23:55:42]

PTAL.  Simplified the code using your suggestion.

https://codereview.chromium.org/621573002/diff/1/content/renderer/media/andro...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/621573002/diff/1/content/renderer/media/andro...
content/renderer/media/android/webmediaplayer_android.cc:531: if
(!base::IsFinite(seekable_end))
On 2014/10/01 09:37:50, philipj wrote:
> With this change I believe you can remove the readyState check above, because
> duration() already returns NaN for that case.

Done.

https://codereview.chromium.org/621573002/diff/1/media/blink/webmediaplayer_i...
File media/blink/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/621573002/diff/1/media/blink/webmediaplayer_i...
media/blink/webmediaplayer_impl.cc:499: if (data_source_ &&
data_source_->IsStreaming()) {
On 2014/10/01 09:37:50, philipj wrote:
> Can you DCHECK(base::IsFinite(seekable_end) && seekable_end > 0.0) in an else
> branch, or does that not always hold true? The previous code clearly never
> returned a [0,0] range, but now it's not clear if it's possible for the
> non-streaming case or not.

I've reworked this to look more like the android code. !IsFinite(duration) will
always return an empty range now.  Streaming media will return 0,0.

[philipj_slow, 2014-10-07 08:43:55]

LGTM

[philipj_slow, 2014-10-07 08:44:40]

philipj@opera.com changed reviewers:
+ scherkus@chromium.org

[philipj_slow, 2014-10-07 08:44:40]

Adding scherkus for OWNERS review.

[DaleCurtis, 2014-10-07 17:11:02]

Thanks for review! I forgot you're not a Chrome committer, so scherkus@'s review
will be necessary. Normally since I'm an OWNER any committers approval is
enough.

[scherkus (not reviewing), 2014-10-07 18:18:13]

lgtm

[DaleCurtis, 2014-10-07 18:19:27]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-10-07 18:20:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/621573002/40001

[commit-bot: I haz the power, 2014-10-07 20:52:13]

Committed patchset #2 (id:40001) as f1c3583d398ccb52fb0812e949ca882a3747c71f

[commit-bot: I haz the power, 2014-10-07 20:52:52]

Patchset 2 (id:??) landed as
https://crrev.com/dfe0527f2c241795d4574503360a531a8221bfa0
Cr-Commit-Position: refs/heads/master@{#298565}