Enhance WebMediaPlayer::buffered() to account for evicted ranges.

Enhance WebMediaPlayer::buffered() to account for evicted ranges.

The BufferedResourceLoader now reports the earliest available byte
via its ProgressCB.  BufferedDataSource recieves this update and
notifies the BufferedDataSourceHost to remove all ranges prior to
the eviction point.

Additionally modifies WebMediaPlayerImpl() to ignore demuxed ranges
when reporting buffered() for streaming sources. Notably, this will
cause missing ranges in the controls for streaming resources.  I
think this is acceptable since users can't seek to these positions.

At present, this CL doesn't really do anything other than affect the
controls.  A followup change will change would change seekable() to
use buffered() when a streaming resource w/ a duration is present.

BUG=412562
TEST=lots, tbd.

[DaleCurtis, 2014-09-17 01:30:04]

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

[DaleCurtis, 2014-09-17 01:43:20]

Posting for feedback only right now, if we agree this is a reasonable
improvement for 200-type resources, I'll go ahead with the rest of the plan. 
Here's the outline of my plan from the previous CL:

https://code.google.com/p/chromium/issues/detail?id=412562#c9

Lets keep any larger design discussion on that bug.

+scherkus instead of acolwell since he'll be leaving chromium/ shortly.

[scherkus (not reviewing), 2014-09-17 16:25:28]

At a high level I have concerns that we're hot fixing a lot of areas of the code
base (see: this change, the [0, epsilon] change, the cache lock change, the
subsequent perf regression as http://crbug.com/408765) -- but what's the long
term plan?

Perhaps we should sit down today and discuss before we proceed. For example in
this case we're adding more code to an area that should be nuked.

https://codereview.chromium.org/574253002/diff/1/media/blink/buffered_data_so...
File media/blink/buffered_data_source.h (left):

https://codereview.chromium.org/574253002/diff/1/media/blink/buffered_data_so...
media/blink/buffered_data_source.h:33: // pull-based.  http://crbug.com/131444
while http://crbug.com/131444 has been closed as WontFix the general idea still
lives on -- BufferedDataSourceHost should really go away and we should move to
purely time-based demuxing

I'm having trouble finding the relevant bug though :\

[DaleCurtis, 2014-09-17 18:02:39]

Sure lets chat today. The gist of my approach is that regardless of any larger
plan (which is in progress), that's not a valid reason to hold off on targeted
fixes. Especially those likely to survive in some form or another in any "new
world," for which this change qualifies.

I think despite your statement, for reasons outlined on the bug and mentioned by
philipj about Opera's previous implementation, it's unlikely we can ever switch
to a time-only based system. Some collusion with the caching/buffering layer
will be necessary to cover the spec. I'd love to hear your reasoning though as
you've worked with this code far longer than I.

You've seen the rough draft of the plan, but for public reference, we're still
in the data collection phase and I filed http://crbug.com/413451 several days
ago to track collection of the necessary metrics to determine the value of any
investment in rewriting the cache/buffering layer.

[scherkus (not reviewing), 2014-09-17 18:17:48]

On 2014/09/17 18:02:39, DaleCurtis wrote:
> Sure lets chat today. The gist of my approach is that regardless of any larger
> plan (which is in progress), that's not a valid reason to hold off on targeted
> fixes. Especially those likely to survive in some form or another in any "new
> world," for which this change qualifies.
> 
> I think despite your statement, for reasons outlined on the bug and mentioned
by
> philipj about Opera's previous implementation, it's unlikely we can ever
switch
> to a time-only based system. Some collusion with the caching/buffering layer
> will be necessary to cover the spec. I'd love to hear your reasoning though as
> you've worked with this code far longer than I.
> 
> You've seen the rough draft of the plan, but for public reference, we're still
> in the data collection phase and I filed http://crbug.com/413451 several days
> ago to track collection of the necessary metrics to determine the value of any
> investment in rewriting the cache/buffering layer.

I get that there'll be collusion, but the collusion we're currently committing
to is extremely subtle such as inserting PostTask()'s and this CL which clears
ranges. I want to get a sense on how we'll do better in the future.

[philipj_slow, 2014-09-17 18:34:48]

So for my education, why is it that one can't simply look at the buffered byte
ranges in cache and map those to time ranges? Is it difficult to inspect for
fundamental reasons, like being out-of-process and volatile? Impossible with
existing APIs because the API design on things on principle that the cache
should be transparent? Something else?

[philipj_slow, 2014-09-17 19:00:11]

On 2014/09/17 18:34:48, philipj wrote:
> Impossible with existing APIs because the API design on things on principle
> that the cache should be transparent?

Let's try that again: Impossible with existing APIs because as a matter of
principle the cache should be transparent?

[DaleCurtis, 2014-09-17 19:31:55]

On 2014/09/17 18:17:48, scherkus wrote:
> On 2014/09/17 18:02:39, DaleCurtis wrote:
> > Sure lets chat today. The gist of my approach is that regardless of any
larger
> > plan (which is in progress), that's not a valid reason to hold off on
targeted
> > fixes. Especially those likely to survive in some form or another in any
"new
> > world," for which this change qualifies.
> > 
> > I think despite your statement, for reasons outlined on the bug and
mentioned
> by
> > philipj about Opera's previous implementation, it's unlikely we can ever
> switch
> > to a time-only based system. Some collusion with the caching/buffering layer
> > will be necessary to cover the spec. I'd love to hear your reasoning though
as
> > you've worked with this code far longer than I.
> > 
> > You've seen the rough draft of the plan, but for public reference, we're
still
> > in the data collection phase and I filed http://crbug.com/413451 several
days
> > ago to track collection of the necessary metrics to determine the value of
any
> > investment in rewriting the cache/buffering layer.
> 
> I get that there'll be collusion, but the collusion we're currently committing
> to is extremely subtle such as inserting PostTask()'s and this CL which clears
> ranges. I want to get a sense on how we'll do better in the future.

Lets keep the discussion of the PostTask out of this change. As mentioned in the
discussion on http://crbug.com/408765, that's not the fix we want to land.

[DaleCurtis, 2014-09-17 19:41:12]

On 2014/09/17 19:00:11, philipj wrote:
> On 2014/09/17 18:34:48, philipj wrote:
> > Impossible with existing APIs because the API design on things on principle
> > that the cache should be transparent?
> 
> Let's try that again: Impossible with existing APIs because as a matter of
> principle the cache should be transparent?

Well, that's kind of what this CL is doing. Today there are two levels of cache,
the HttpCache which lives in net/ and the SeekableBuffer inside the
BufferedResourceLoader. You've correctly realized that the HttpCache APIs are
designed with the idea that its completely transparent. What I'm adding in this
CL is a mechanism for keeping the "monitor" BufferedDataSourceHostImpl in sync
with what we actually know about the cache. The WebMediaPlayer than uses the
demuxed time-range knowledge plus the data from the "monitor" class to report
the correct buffered ranges.

[DaleCurtis, 2014-09-18 00:45:10]

Per offline discussion with scherkus@ I'm going to table this discussion until
we can determine if a new cache architecture with a focus on mobile would be a
better investment even for desktop.

philipj, happy to discuss anything about the current architecture if you're
still curious though :)

[philipj_slow, 2014-09-18 19:47:46]

There's plenty I'd like to learn about the Chromium side of <video> and friends,
if you're all going to BlinkOn we should talk!

Does BufferedResourceLoader exist only because stuff in HttpCache can't be
pinned, or are there some other benefits to the two levels of cache? Presto of
course also had *some* data in memory, but that didn't have anything to do with
buffered.

This stuff kind of gets me excited, but I have so much to do in Blink that I
really shouldn't volunteer to look into it :)

[DaleCurtis, 2014-09-18 20:24:14]

On 2014/09/18 19:47:46, philipj wrote:
> There's plenty I'd like to learn about the Chromium side of <video> and
friends,
> if you're all going to BlinkOn we should talk!

I hadn't planned to, but it's probably a good idea with acolwell@ moving on. I
believe scherkus@ will be there, but I'll see about going as well.

> 
> Does BufferedResourceLoader exist only because stuff in HttpCache can't be
> pinned, or are there some other benefits to the two levels of cache? Presto of
> course also had *some* data in memory, but that didn't have anything to do
with
> buffered.

From my discussions with scherkus@ yesterday, this seems to be correct: We've
got
both levels of the cache because the transparency prevents any kind of
assumptions
about what's in it and nothing like pinning exists.

> 
> This stuff kind of gets me excited, but I have so much to do in Blink that I
> really shouldn't volunteer to look into it :)

I'll make sure any plans I come up with for the cache are publicly available so
you (and the rest of Opera) can provide some input! Thanks!

[philipj_slow, 2014-09-18 21:02:35]

On 2014/09/18 20:24:14, DaleCurtis wrote:
> On 2014/09/18 19:47:46, philipj wrote:
> > There's plenty I'd like to learn about the Chromium side of <video> and
> friends,
> > if you're all going to BlinkOn we should talk!
> 
> I hadn't planned to, but it's probably a good idea with acolwell@ moving on. I
> believe scherkus@ will be there, but I'll see about going as well.

Cool! No pressure, but I hope you can make it!