Media: Delete Pause-To-Buffer.

Media: Delete Pause-To-Buffer.

This code was broken in M52, but nobody has complained, so remove it.

Pause-To-Buffer is a consistent source of surprise, and mistakes leading
to accidental bandwidth consumption. The new download button is a much
better interface (at least in cases where it appears).

Review-Url: https://codereview.chromium.org/2689323002
Cr-Commit-Position: refs/heads/master@{#460563}
Committed: https://chromium.googlesource.com/chromium/src/+/2f5bb6155b40ff1533e4310f18e294ab34b55f14

[sandersd (OOO until July 31), 2017-02-13 23:41:35]

sandersd@chromium.org changed reviewers:
+ dalecurtis@chromium.org, hubbe@chromium.org, mlamouri@chromium.org

[sandersd (OOO until July 31), 2017-02-13 23:41:35]

hubbe@: Please check the multibuffer unittest in particular, I wasn't sure
exactly what was being tested.

mlamouri@: Please review changes in Blink.

[DaleCurtis, 2017-02-13 23:45:39]

lgtm!

[hubbe, 2017-02-14 00:02:54]

LGTM

BUT, it might be premature to delete this.
It seems the cache was working on M54, and also the broken cache behavior only
affect files with etags.

Is there a replacement plan yet?

[DaleCurtis, 2017-02-14 00:06:44]

Ah I didn't realize it was a conditional breakage. Though conditional in this
case includes every video served from s3, no? Do we have a sense of what
percentage of videos that is?

[DaleCurtis, 2017-02-14 00:07:16]

(I'm still in favor of removing either way though, this is obscure and with the
download button users can instead download their media)

[DaleCurtis, 2017-02-14 00:21:21]

Actually, looks like this behavior is busted even without e-tags. You can test
by using media/tools/constrained_network_server/

sudo python cns.py --interface=lo --port=8777 --www-root /var/www

Put some video in your /var/www directory. I used buck2.webm, navigate to:

http://localhost:8777/ServeConstrained?f=buck2.mp4&bandwidth=2000

(You can also add latency=<ms> (in ms), and loss=% if you want to test other
configurations)

You can see that pause and buffer stops after a few seconds and does not
continue. There are no e-tags in this request.

[mlamouri (slow - plz ping), 2017-02-14 16:42:16]

I'm a bit worried that this might be used a lot in emerging markets. The
download button might not be visible in cases where the pause-to-buffer
behaviour would work. It might be good to get a thumbs up from dahlke@ and maybe
have metrics that verify that it's indeed broken and useless.

[DaleCurtis, 2017-02-14 17:50:13]

dalecurtis@chromium.org changed reviewers:
+ dahlke@chromium.org

[DaleCurtis, 2017-02-14 17:50:13]

I mean, it's definitely not being used a lot right now since it's broken and has
been since ~July 2016, for better or worse. Last time we broke this we heard
about it immediately, that we haven't heard anything in ~7 months tells me this
isn't likely missed. +dahlke for commentary.

[chromium-reviews, 2017-02-14 18:23:16]

How do you know it's been broken since July?

On Tue, Feb 14, 2017 at 9:50 AM, <dalecurtis@chromium.org> wrote:

> I mean, it's definitely not being used a lot right now since it's broken
> and has
> been since ~July 2016, for better or worse. Last time we broke this we
> heard
> about it immediately, that we haven't heard anything in ~7 months tells me
> this
> isn't likely missed. +dahlke for commentary.
>
> https://codereview.chromium.org/2689323002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[blink-reviews, 2017-02-14 18:23:17]

How do you know it's been broken since July?

On Tue, Feb 14, 2017 at 9:50 AM, <dalecurtis@chromium.org> wrote:

> I mean, it's definitely not being used a lot right now since it's broken
> and has
> been since ~July 2016, for better or worse. Last time we broke this we
> heard
> about it immediately, that we haven't heard anything in ~7 months tells me
> this
> isn't likely missed. +dahlke for commentary.
>
> https://codereview.chromium.org/2689323002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[DaleCurtis, 2017-02-14 18:30:31]

I don't know for sure that it was broken that early, but I suspect something in
the Spitzer/Multibuffer launch broke it intermittently, and the if-match issue
broke it entirely for content served with etags. It's definitely broken since
M56.

I'll test a build of M52 later today and verify the date.

[DaleCurtis, 2017-02-14 19:29:03]

On 2017/02/14 at 18:30:31, DaleCurtis wrote:
> I don't know for sure that it was broken that early, but I suspect something
in the Spitzer/Multibuffer launch broke it intermittently, and the if-match
issue broke it entirely for content served with etags. It's definitely broken
since M56.
> 
> I'll test a build of M52 later today and verify the date.

Confirmed busted in M52 via the mechanism above; it's also busted in M51, so the
breakage goes back even further than I thought. Still narrowing down the range.

[blink-reviews, 2017-02-14 20:25:40]

+Husain

On Tue, Feb 14, 2017 at 11:29 AM, <dalecurtis@chromium.org> wrote:

> On 2017/02/14 at 18:30:31, DaleCurtis wrote:
> > I don't know for sure that it was broken that early, but I suspect
> something
> in the Spitzer/Multibuffer launch broke it intermittently, and the if-match
> issue broke it entirely for content served with etags. It's definitely
> broken
> since M56.
> >
> > I'll test a build of M52 later today and verify the date.
>
> Confirmed busted in M52 via the mechanism above; it's also busted in M51,
> so the
> breakage goes back even further than I thought. Still narrowing down the
> range.
>
> https://codereview.chromium.org/2689323002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2017-02-14 20:25:40]

+Husain

On Tue, Feb 14, 2017 at 11:29 AM, <dalecurtis@chromium.org> wrote:

> On 2017/02/14 at 18:30:31, DaleCurtis wrote:
> > I don't know for sure that it was broken that early, but I suspect
> something
> in the Spitzer/Multibuffer launch broke it intermittently, and the if-match
> issue broke it entirely for content served with etags. It's definitely
> broken
> since M56.
> >
> > I'll test a build of M52 later today and verify the date.
>
> Confirmed busted in M52 via the mechanism above; it's also busted in M51,
> so the
> breakage goes back even further than I thought. Still narrowing down the
> range.
>
> https://codereview.chromium.org/2689323002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[hbengali, 2017-03-25 00:20:41]

Dan just pinged me about this. I am comfortable removing it - it's not obvious
that pausing will buffer the content, and it may use data unintentionally and in
a way that can't be easily stopped (would require closing the page or skipping
to the end of the video).

[sandersd (OOO until July 31), 2017-03-29 00:25:36]

sandersd@chromium.org changed reviewers:
+ dcheng@chromium.org

[sandersd (OOO until July 31), 2017-03-29 00:25:36]

dcheng@chromium.org: Please review changes in third_party/WebKit

[dcheng, 2017-03-29 17:30:30]

lgtm

[sandersd (OOO until July 31), 2017-03-29 18:22:05]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 18:22:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 20:39:00]

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

[commit-bot: I haz the power, 2017-03-29 20:39:01]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[sandersd (OOO until July 31), 2017-03-29 20:50:24]

The CQ bit was checked by sandersd@chromium.org

[sandersd (OOO until July 31), 2017-03-29 20:50:25]

The patchset sent to the CQ was uploaded after l-g-t-m from hubbe@chromium.org,
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2689323002/#ps20001
(title: "Rebase.")

[commit-bot: I haz the power, 2017-03-29 20:51:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 22:57:57]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490820624942030,
"parent_rev": "9e468760955a427460f9d8ebb604f0e6a24a6c35", "commit_rev":
"2f5bb6155b40ff1533e4310f18e294ab34b55f14"}

[commit-bot: I haz the power, 2017-03-29 22:58:50]

Description was changed from

==========
Media: Delete Pause-To-Buffer.

This code was broken in M52, but nobody has complained, so remove it.

Pause-To-Buffer is a consistent source of surprise, and mistakes leading
to accidental bandwidth consumption. The new download button is a much
better interface (at least in cases where it appears).
==========

to

==========
Media: Delete Pause-To-Buffer.

This code was broken in M52, but nobody has complained, so remove it.

Pause-To-Buffer is a consistent source of surprise, and mistakes leading
to accidental bandwidth consumption. The new download button is a much
better interface (at least in cases where it appears).

Review-Url: https://codereview.chromium.org/2689323002
Cr-Commit-Position: refs/heads/master@{#460563}
Committed:
https://chromium.googlesource.com/chromium/src/+/2f5bb6155b40ff1533e4310f18e2...
==========

[commit-bot: I haz the power, 2017-03-29 22:58:52]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2f5bb6155b40ff1533e4310f18e2...