Disallow assigning a MediaSession after the resource has been selected

Disallow assigning a MediaSession after the resource has been selected

https://mediasession.spec.whatwg.org/#extensions-to-the-htmlmediaelement-interface

BUG=497735
Committed: https://crrev.com/8eea2f9b24f1903ec92d8d2bb707dff184f77841
Cr-Commit-Position: refs/heads/master@{#356299}

[davve, 2015-10-23 09:43:50]

The CQ bit was checked by davve@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-23 09:43:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420223002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420223002/1

[commit-bot: I haz the power, 2015-10-23 11:00:58]

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

[commit-bot: I haz the power, 2015-10-23 11:00:58]

Dry run: This issue passed the CQ dry run.

[davve, 2015-10-23 11:09:55]

davve@opera.com changed reviewers:
+ avayvod@chromium.org, mlamouri@chromium.org, philipj@opera.com

[davve, 2015-10-23 11:09:55]

Align with recent spec change. PTAL.

[philipj_slow, 2015-10-23 11:17:11]

https://codereview.chromium.org/1420223002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/mediasession/htmlmediaelement-set-session.html
(right):

https://codereview.chromium.org/1420223002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/mediasession/htmlmediaelement-set-session.html:27:
mediaElement.load();
Setting the src attribute should (per spec) already have run the media element
load algorithm. You could assert that networkState is now NETWORK_NO_SOURCE.

A separate test for NETWORK_EMPTY would also be nice, so I guess that would make
three tests in total.

https://codereview.chromium.org/1420223002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/mediasession/HTMLMediaElementMediaSession.cpp
(right):

https://codereview.chromium.org/1420223002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediasession/HTMLMediaElementMediaSession.cpp:23:
exceptionState.throwDOMException(InvalidStateError, "Media sessions must be
assigned before a resource has been selected");
Maybe say that the expected networkState is NETWORK_EMPTY or NETWORK_NO_SOURCE,
and also say what the actual networkState is? It has to be a pretty savvy web
developer to know about the resource selection algorithm and its connection to
networkState.

[philipj_slow, 2015-10-23 14:33:54]

lgtm

[mlamouri (slow - plz ping), 2015-10-26 10:37:29]

Code lgtm but I have some questions on the spec side of things. What would
happen if a website loads its scripts asynchronously and there is a media
element with 'autoplay'? Could we end up in a race with regards to when the
session can be set?

[philipj_slow, 2015-10-26 10:47:38]

On 2015/10/26 10:37:29, Mounir Lamouri wrote:
> Code lgtm but I have some questions on the spec side of things. What would
> happen if a website loads its scripts asynchronously and there is a media
> element with 'autoplay'? Could we end up in a race with regards to when the
> session can be set?

Definitely, but it's actually not the play we're racing with, but the load, i.e.
the creation of the WebMediaPlayer backend. The spec as written just doesn't
blend well with media elements in markup. The simplicity of a WebMediaPlayer not
being able to change its session during its lifetime is pretty compelling, but
we may certainly have to revisit this (relax the rules) based on feedback from
how people try to integrate media session in their web apps/sites.

[davve, 2015-10-26 12:15:17]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2015-10-26 12:15:20]

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

[commit-bot: I haz the power, 2015-10-26 12:16:44]

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

[commit-bot: I haz the power, 2015-10-26 12:16:45]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[davve, 2015-10-26 13:07:44]

On 2015/10/26 12:16:45, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
>   ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
>   mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
>   mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

third_party/WebKit/LayoutTests/platform/android/virtual/mediasession/ was
removed (among a lot of other android test expectations) in
http://crrev.com/355808

Will make an attempt to get the tests included in SmokeTests so that we get some
test coverage and a motivation for having the expected files.

[davve, 2015-10-26 13:35:07]

The CQ bit was checked by davve@opera.com

[davve, 2015-10-26 13:35:07]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1420223002/#ps80001
(title: "Update failing, non-android expected file")

[commit-bot: I haz the power, 2015-10-26 13:35:11]

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

[commit-bot: I haz the power, 2015-10-26 14:51:31]

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

[commit-bot: I haz the power, 2015-10-26 14:51:32]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[davve, 2015-10-26 15:02:03]

Description was changed from

==========
Disallow assigning a MediaSession after the resource has been selected

https://mediasession.spec.whatwg.org/#extensions-to-the-htmlmediaelement-inte...

BUG=497735
==========

to

==========
Disallow assigning a MediaSession after the resource has been selected

https://mediasession.spec.whatwg.org/#extensions-to-the-htmlmediaelement-inte...

BUG=497735
==========

[davve, 2015-10-26 15:03:01]

The CQ bit was checked by davve@opera.com

[davve, 2015-10-26 15:03:01]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1420223002/#ps100001
(title: "Pull expected file from the bots")

[commit-bot: I haz the power, 2015-10-26 15:03:24]

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

[commit-bot: I haz the power, 2015-10-26 17:38:58]

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

[commit-bot: I haz the power, 2015-10-26 17:38:59]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[davve, 2015-10-27 07:30:50]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2015-10-27 07:31:28]

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

[commit-bot: I haz the power, 2015-10-27 10:15:55]

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

[commit-bot: I haz the power, 2015-10-27 10:15:56]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[davve, 2015-10-27 13:09:56]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2015-10-27 13:10:26]

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

[commit-bot: I haz the power, 2015-10-27 15:11:28]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-10-27 15:12:20]

Patchset 6 (id:??) landed as
https://crrev.com/8eea2f9b24f1903ec92d8d2bb707dff184f77841
Cr-Commit-Position: refs/heads/master@{#356299}