Defer media loading while on cellular networks.

Defer media loading while on cellular networks.

We only populate networkStateNotifier()'s connectionType() field on
a couple platforms, see http://crbug.com/368358 for details, but it
does work on Android at least.

This prevents us from wasting bandwidth on videos when the device is
on a cellular connection.

BUG=516766
TEST=Load deferred when forced to true, TBD: actual device test.

[DaleCurtis, 2015-08-07 02:55:03]

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

[DaleCurtis, 2015-08-07 02:55:04]

I don't think this is likely the right way to do this, but sending out for
discussion.  Instead we may want something on the WebMediaPlayer interface like
alwaysDeferUntilPlay(); each impl could then do whatever checks it wants to
determine this.

If we want to go that route we'll likely need to create and expose
WebNetworkStateNotifier::connectionType(), since there doesn't seem to be any
piece of Chrome side code currently tracking this data.

WDYT?

[philipj_slow, 2015-08-10 10:09:56]

On 2015/08/07 02:55:04, DaleCurtis wrote:
> I don't think this is likely the right way to do this, but sending out for
> discussion.  Instead we may want something on the WebMediaPlayer interface
like
> alwaysDeferUntilPlay(); each impl could then do whatever checks it wants to
> determine this.
> 
> If we want to go that route we'll likely need to create and expose
> WebNetworkStateNotifier::connectionType(), since there doesn't seem to be any
> piece of Chrome side code currently tracking this data.
> 
> WDYT?

It would be helpful to talk about this in terms of preload I think. Is it that
the default preload state should be none while on cellular networks? If the
network connection changes, should media elements that were previously deferred
start loading?

We have an HTMLMediaElement::effectivePreloadType() which could probably be used
here.

The web compat risk of a change like this seems pretty high, shouldn't we expect
that sites simply create a video and wait for the loadeddata event or similar
before showing it?

[DaleCurtis, 2015-08-10 16:15:03]

Ooh, effectivePreloadType() seems like a reasonable place.  Yes, the least-hacky
way to think about this is that preload defaults to none on "expensive" networks
like cellular -- though my current CL ignores preload entirely. Do you know if
we have any statistics on the preload setting breakdown?

Is there any web-compat risk if we only change the default to none? I'm sure
some sites have some baked expectations, but the spec says "auto": "Hints to the
user agent that the user agent can put the user's needs first without risk to
the server, up to and including optimistically downloading the entire resource."
Letting preload=auto mean preload=none for cellular networks seems to fit inside
the "put the user's needs first."

Note that this is already the shipping behavior on Android -- though I believe
they lie and send an fake metadata event (use 0,0 for size, etc) -- it's just
done in the bowels of the Android code:

https://code.google.com/p/chromium/codesearch#chromium/src/content/public/and...

I wasn't planning to restart the connections once the user moved over to say
WiFi, since we don't do that today and I'd guess that's a small percentage of
users.

[philipj_slow, 2015-08-11 09:51:36]

On 2015/08/10 16:15:03, DaleCurtis wrote:
> Ooh, effectivePreloadType() seems like a reasonable place.  Yes, the
least-hacky
> way to think about this is that preload defaults to none on "expensive"
networks
> like cellular -- though my current CL ignores preload entirely. Do you know if
> we have any statistics on the preload setting breakdown?

I added some but they haven't reached the stable channel:
https://www.chromestatus.com/metrics/feature/timeline/popularity/892
https://www.chromestatus.com/metrics/feature/timeline/popularity/893
https://www.chromestatus.com/metrics/feature/timeline/popularity/894
https://www.chromestatus.com/metrics/feature/timeline/popularity/895

> Is there any web-compat risk if we only change the default to none? I'm sure
> some sites have some baked expectations, but the spec says "auto": "Hints to
the
> user agent that the user agent can put the user's needs first without risk to
> the server, up to and including optimistically downloading the entire
resource."
> Letting preload=auto mean preload=none for cellular networks seems to fit
inside
> the "put the user's needs first."

preload=auto started out as an autobuffer attribute, and even though the spec
treats the preload attribute as a hint without requiring anything, I think web
developers do assume that preload=auto will buffer at least as much as
preload=metadata.

If we do change the behavior, I think it would make sense to simply report
preload "none" so that this can be detected. In spec terms, the "missing value
default" should be "none" in these cases. So the change would be in the end of
HTMLMediaElement::preloadType()

> Note that this is already the shipping behavior on Android -- though I believe
> they lie and send an fake metadata event (use 0,0 for size, etc) -- it's just
> done in the bowels of the Android code:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/and...
> 
> I wasn't planning to restart the connections once the user moved over to say
> WiFi, since we don't do that today and I'd guess that's a small percentage of
> users.

If no changes in when the network connection is made will result from this
change, then an explicit preload=none (which could be overridden) at the
HTMLMediaElement level sure seems a lot better.

As for the risk, which events will currently fire on a media element on Android
when no preload attribute is present?

Also, are we talking only about changing the behavior when no preload attribute
is used, or do you also want to change what would happen for preload=metadata
and preload=auto? This would be a riskier change, I think.

[DaleCurtis, 2015-08-12 01:35:38]

On 2015/08/11 09:51:36, philipj wrote:
> On 2015/08/10 16:15:03, DaleCurtis wrote:
> > Ooh, effectivePreloadType() seems like a reasonable place.  Yes, the
> least-hacky
> > way to think about this is that preload defaults to none on "expensive"
> networks
> > like cellular -- though my current CL ignores preload entirely. Do you know
if
> > we have any statistics on the preload setting breakdown?
> 
> I added some but they haven't reached the stable channel:
> https://www.chromestatus.com/metrics/feature/timeline/popularity/892
> https://www.chromestatus.com/metrics/feature/timeline/popularity/893
> https://www.chromestatus.com/metrics/feature/timeline/popularity/894
> https://www.chromestatus.com/metrics/feature/timeline/popularity/895

Awesome, will those roll to stable with M45? No need to land this soon, so we
can wait until we have more data on the situation.

> 
> > Is there any web-compat risk if we only change the default to none? I'm sure
> > some sites have some baked expectations, but the spec says "auto": "Hints to
> the
> > user agent that the user agent can put the user's needs first without risk
to
> > the server, up to and including optimistically downloading the entire
> resource."
> > Letting preload=auto mean preload=none for cellular networks seems to fit
> inside
> > the "put the user's needs first."
> 
> preload=auto started out as an autobuffer attribute, and even though the spec
> treats the preload attribute as a hint without requiring anything, I think web
> developers do assume that preload=auto will buffer at least as much as
> preload=metadata.
> 
> If we do change the behavior, I think it would make sense to simply report
> preload "none" so that this can be detected. In spec terms, the "missing value
> default" should be "none" in these cases. So the change would be in the end of
> HTMLMediaElement::preloadType()

That seems fine to me, the spec says "The attribute's missing value default is
user-agent defined, though the Metadata state is suggested as a compromise
between reducing server load and providing an optimal user experience." Long
term, I actually think metadata is a better default for mobile but there are
bandwidth concerns relative to Android's existing "load-nothing-on-cellular"
approach.

> 
> > Note that this is already the shipping behavior on Android -- though I
believe
> > they lie and send an fake metadata event (use 0,0 for size, etc) -- it's
just
> > done in the bowels of the Android code:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/and...
> > 
> > I wasn't planning to restart the connections once the user moved over to say
> > WiFi, since we don't do that today and I'd guess that's a small percentage
of
> > users.
> 
> If no changes in when the network connection is made will result from this
> change, then an explicit preload=none (which could be overridden) at the
> HTMLMediaElement level sure seems a lot better.
> 
> As for the risk, which events will currently fire on a media element on
Android
> when no preload attribute is present?

watk@ was talking about testing this, I'll take a look on Thursday if he hasn't
had a chance by then.

> 
> Also, are we talking only about changing the behavior when no preload
attribute
> is used, or do you also want to change what would happen for preload=metadata
> and preload=auto? This would be a riskier change, I think.

I like the idea of only doing it for the lack of a preload attribute, but today
Android applies it to all cellular connections regardless of the attribute.

[philipj_slow, 2015-08-12 07:35:39]

On 2015/08/12 01:35:38, DaleCurtis wrote:
> On 2015/08/11 09:51:36, philipj wrote:
> > On 2015/08/10 16:15:03, DaleCurtis wrote:
> > > Ooh, effectivePreloadType() seems like a reasonable place.  Yes, the
> > least-hacky
> > > way to think about this is that preload defaults to none on "expensive"
> > networks
> > > like cellular -- though my current CL ignores preload entirely. Do you
know
> if
> > > we have any statistics on the preload setting breakdown?
> > 
> > I added some but they haven't reached the stable channel:
> > https://www.chromestatus.com/metrics/feature/timeline/popularity/892
> > https://www.chromestatus.com/metrics/feature/timeline/popularity/893
> > https://www.chromestatus.com/metrics/feature/timeline/popularity/894
> > https://www.chromestatus.com/metrics/feature/timeline/popularity/895
> 
> Awesome, will those roll to stable with M45? No need to land this soon, so we
> can wait until we have more data on the situation.

They're going to me in M46, I'm afraid. However, you should have access to
per-channel and per-platform data, so you could get a good idea of what the
distribution, and even predict what the eventual usage for each will stabilize
on.

> > > Is there any web-compat risk if we only change the default to none? I'm
sure
> > > some sites have some baked expectations, but the spec says "auto": "Hints
to
> > the
> > > user agent that the user agent can put the user's needs first without risk
> to
> > > the server, up to and including optimistically downloading the entire
> > resource."
> > > Letting preload=auto mean preload=none for cellular networks seems to fit
> > inside
> > > the "put the user's needs first."
> > 
> > preload=auto started out as an autobuffer attribute, and even though the
spec
> > treats the preload attribute as a hint without requiring anything, I think
web
> > developers do assume that preload=auto will buffer at least as much as
> > preload=metadata.
> > 
> > If we do change the behavior, I think it would make sense to simply report
> > preload "none" so that this can be detected. In spec terms, the "missing
value
> > default" should be "none" in these cases. So the change would be in the end
of
> > HTMLMediaElement::preloadType()
> 
> That seems fine to me, the spec says "The attribute's missing value default is
> user-agent defined, though the Metadata state is suggested as a compromise
> between reducing server load and providing an optimal user experience." Long
> term, I actually think metadata is a better default for mobile but there are
> bandwidth concerns relative to Android's existing "load-nothing-on-cellular"
> approach.

Yeah, I wish that metadata were the default on all platforms :)

> > > Note that this is already the shipping behavior on Android -- though I
> believe
> > > they lie and send an fake metadata event (use 0,0 for size, etc) -- it's
> just
> > > done in the bowels of the Android code:
> > > 
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/and...
> > > 
> > > I wasn't planning to restart the connections once the user moved over to
say
> > > WiFi, since we don't do that today and I'd guess that's a small percentage
> of
> > > users.
> > 
> > If no changes in when the network connection is made will result from this
> > change, then an explicit preload=none (which could be overridden) at the
> > HTMLMediaElement level sure seems a lot better.
> > 
> > As for the risk, which events will currently fire on a media element on
> Android
> > when no preload attribute is present?
> 
> watk@ was talking about testing this, I'll take a look on Thursday if he
hasn't
> had a chance by then.
> 
> > 
> > Also, are we talking only about changing the behavior when no preload
> attribute
> > is used, or do you also want to change what would happen for
preload=metadata
> > and preload=auto? This would be a riskier change, I think.
> 
> I like the idea of only doing it for the lack of a preload attribute, but
today
> Android applies it to all cellular connections regardless of the attribute.

Oh, that sounds pretty broken, so the preload attribute on Android is simply
ignored? In that case it would be good to at least default preload to false to
reflect this, although there isn't a way of exposing the effective preload state
to let web developers know that their preload=auto has been ignored.

[DaleCurtis, 2015-08-18 18:54:48]

On 2015/08/12 07:35:39, philipj wrote:
> On 2015/08/12 01:35:38, DaleCurtis wrote:
> > 
> > I like the idea of only doing it for the lack of a preload attribute, but
> today
> > Android applies it to all cellular connections regardless of the attribute.
> 
> Oh, that sounds pretty broken, so the preload attribute on Android is simply
> ignored? In that case it would be good to at least default preload to false to
> reflect this, although there isn't a way of exposing the effective preload
state
> to let web developers know that their preload=auto has been ignored.

Sorry for the delay, yes, it's ignored on cellular connections. preload=metadata
is used on WiFi, though the first frame is not decoded.

[philipj_slow, 2015-08-19 10:24:36]

OK, so in summary I think a reasonable change would be to make the fallback in
HTMLMediaElement::preloadType() depend on the connection type.

In order to mirror the current Android behavior of ignoring the preload
attribute, an additional change in HTMLMediaElement::effectivePreloadType()
would be needed, but if it seems possible to avoid based on use counter data (no
attribute is the most common case) that would be nice.

[DaleCurtis, 2015-09-30 00:50:21]

Patchset #2 (id:20001) has been deleted

[DaleCurtis, 2015-09-30 00:51:33]

Patchset #2 (id:40001) has been deleted

[DaleCurtis, 2015-09-30 00:52:54]

Okay finally got back to this, updated mostly as discussed, but it ended up
possible to do everything withing preloadType().  Added tests and fixed a
singleton state issue with the test helper for connection notifier.  PTAL.

[watk, 2015-09-30 01:21:53]

lgtm

https://codereview.chromium.org/1276143002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/1276143002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:1969: preloadType =
forcePreloadNone ? WebMediaPlayer::PreloadNone : WebMediaPlayer::PreloadAuto;
It might be clearer to just set to Auto here, and let the override take place
below? Then you can inline the call to shouldForcePre...()

[DaleCurtis, 2015-09-30 01:33:45]

https://codereview.chromium.org/1276143002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/1276143002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:1969: preloadType =
forcePreloadNone ? WebMediaPlayer::PreloadNone : WebMediaPlayer::PreloadAuto;
On 2015/09/30 01:21:52, watk wrote:
> It might be clearer to just set to Auto here, and let the override take place
> below? Then you can inline the call to shouldForcePre...()

Done.

[philipj_slow, 2015-09-30 11:18:07]

Unless you can change the base URL
(https://chromium.googlesource.com/chromium/blink.git@master) I think you'll
have to create a new review for this.

[philipj_slow, 2015-09-30 11:19:30]

https://codereview.chromium.org/1276143002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/1276143002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:1968: preloadType =
WebMediaPlayer::PreloadNone;
Making this an early return to avoid even looking the attribute would be nice.

[DaleCurtis, 2015-09-30 16:18:19]

Ack, rebased the git commit, but forgot to use a new CL, done here:
https://codereview.chromium.org/1381613003/

https://codereview.chromium.org/1276143002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/1276143002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:1968: preloadType =
WebMediaPlayer::PreloadNone;
On 2015/09/30 11:19:30, philipj wrote:
> Making this an early return to avoid even looking the attribute would be nice.

I didn't do that so that we could keep the use counters for the attribute.

[philipj_slow, 2015-10-01 09:49:07]

https://codereview.chromium.org/1276143002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/1276143002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:1968: preloadType =
WebMediaPlayer::PreloadNone;
On 2015/09/30 16:18:19, DaleCurtis_OOO_Until_Oct_16 wrote:
> On 2015/09/30 11:19:30, philipj wrote:
> > Making this an early return to avoid even looking the attribute would be
nice.
> 
> I didn't do that so that we could keep the use counters for the attribute.

Hmm, good point, but if they're counted without being used it might be
misleading. I suggest instead adding a
UseCounter::HTMLMediaElementPreloadNoneDueToConnectionType or similar.