Deliver change notifications to SourceBuffer track lists.

Deliver change notifications to SourceBuffer track lists.

Also update the activeSourceBuffers list on track status changes.

BUG=639151
Committed: https://crrev.com/01d67d202e8af55c43274d346ed3fb56a16bac65
Cr-Commit-Position: refs/heads/master@{#414475}

[servolk, 2016-08-20 01:21:49]

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

[commit-bot: I haz the power, 2016-08-20 01:22:26]

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

[servolk, 2016-08-20 01:23:09]

Description was changed from

==========
Deliver change notifications to SourceBuffer track lists.

BUG=639151
==========

to

==========
Deliver change notifications to SourceBuffer track lists.

BUG=639151
==========

[servolk, 2016-08-20 01:23:09]

servolk@chromium.org changed reviewers:
+ foolip@chromium.org, mlamouri@chromium.org, wolenetz@chromium.org

[commit-bot: I haz the power, 2016-08-20 02:58:44]

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

[commit-bot: I haz the power, 2016-08-20 02:58:45]

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

[mlamouri (slow - plz ping), 2016-08-22 02:44:50]

HTMLMediaElement.* changes sgtm. Do you want me to look beyond this?

[servolk, 2016-08-22 17:30:48]

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

[commit-bot: I haz the power, 2016-08-22 17:32:05]

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

[servolk, 2016-08-22 17:37:26]

On 2016/08/22 02:44:50, mlamouri (slow) wrote:
> HTMLMediaElement.* changes sgtm. Do you want me to look beyond this?

Thanks for looking, Mounir. I've added you because I thought you might be
interested in those change notifications being added/fixed as a part of media
tracks-related functionality. But it looks like Philip is the sole owner of
third_party/WebKit/Source/core/html/track/, so I guess I'll have to wait for him
to take a look anyway.

[servolk, 2016-08-22 17:37:36]

https://codereview.chromium.org/2263823002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/avtrack/video-track-selected.html
(left):

https://codereview.chromium.org/2263823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/avtrack/video-track-selected.html:26:
e.videoTracks.onchange = t.unreached_func("setting selected to false fired
change event");
Note: I'm changing this intentionally. It looks like the current check is
incorrect, the change event must be fired even when the video track is
deselected. According to
https://www.w3.org/TR/html/single-page.html#audiotracklist-and-videotracklist...:
...
Whenever a track in a VideoTrackList that was previously not selected is
selected, and whenever the selected track in a VideoTrackList is unselected
without a new track being selected in its stead, the user agent must queue a
task to fire a simple event named change at the VideoTrackList object.
...

[commit-bot: I haz the power, 2016-08-22 19:13:09]

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

[commit-bot: I haz the power, 2016-08-22 19:13:10]

Dry run: This issue passed the CQ dry run.

[servolk, 2016-08-22 22:45:32]

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

[servolk, 2016-08-22 22:46:10]

Description was changed from

==========
Deliver change notifications to SourceBuffer track lists.

BUG=639151
==========

to

==========
Deliver change notifications to SourceBuffer track lists.

Also update the activeSourceBuffers list on track status changes.

BUG=639151
==========

[commit-bot: I haz the power, 2016-08-22 22:46:26]

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, 2016-08-23 03:15:34]

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

[commit-bot: I haz the power, 2016-08-23 03:15:35]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2016-08-23 13:12:27]

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

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2403: void
HTMLMediaElement::audioTrackChanged(AudioTrack* track, bool enabled)
Do you still need `enabled`? It sounds like you could do `track->enabled()`
instead, couldn't you?

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2460: void
HTMLMediaElement::selectedVideoTrackChanged(VideoTrack* track, bool selected)
ditto for `selected`

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/track/TrackListBase.h (right):

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/track/TrackListBase.h:90:
scheduleEvent(event);
style: sounds like you could write this in one line.

[servolk, 2016-08-23 17:24:47]

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

[servolk, 2016-08-23 17:25:02]

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

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2403: void
HTMLMediaElement::audioTrackChanged(AudioTrack* track, bool enabled)
On 2016/08/23 13:12:27, mlamouri (slow) wrote:
> Do you still need `enabled`? It sounds like you could do `track->enabled()`
> instead, couldn't you?

Done.

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2460: void
HTMLMediaElement::selectedVideoTrackChanged(VideoTrack* track, bool selected)
On 2016/08/23 13:12:27, mlamouri (slow) wrote:
> ditto for `selected`

Done.

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/track/TrackListBase.h (right):

https://codereview.chromium.org/2263823002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/track/TrackListBase.h:90:
scheduleEvent(event);
On 2016/08/23 13:12:27, mlamouri (slow) wrote:
> style: sounds like you could write this in one line.

Done.

[commit-bot: I haz the power, 2016-08-23 17:25:31]

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, 2016-08-23 17:27:41]

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

[commit-bot: I haz the power, 2016-08-23 17:27:41]

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

[servolk, 2016-08-23 17:38:35]

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

[commit-bot: I haz the power, 2016-08-23 17:39:14]

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, 2016-08-23 19:30:35]

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

[commit-bot: I haz the power, 2016-08-23 19:30:36]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[servolk, 2016-08-23 20:24:19]

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

[commit-bot: I haz the power, 2016-08-23 20:24:47]

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, 2016-08-23 21:19:57]

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

[commit-bot: I haz the power, 2016-08-23 21:19:58]

Dry run: This issue passed the CQ dry run.

[wolenetz, 2016-08-23 23:28:10]

LGTM

[wolenetz, 2016-08-23 23:29:30]

On 2016/08/23 23:28:10, wolenetz wrote:
> LGTM

Note, greater coverage of this in layout test is pending pulling MSE test suite
from upstream w3c w-p-t (see
https://github.com/w3c/web-platform-tests/pull/3497/ for a test that exposed the
previously broken behavior.)

[servolk, 2016-08-24 02:27:54]

The CQ bit was checked by servolk@chromium.org

[commit-bot: I haz the power, 2016-08-24 02:28:38]

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, 2016-08-24 02:36:33]

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

[commit-bot: I haz the power, 2016-08-24 02:36:35]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mlamouri (slow - plz ping), 2016-08-24 16:08:46]

lgtm with comment applied

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

https://codereview.chromium.org/2263823002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2482:
webMediaPlayer()->selectedVideoTrackChanged(track->selected() ? &id : nullptr);
Should you pass `&track->id()` instead of making a copy on the stack and passing
the pointer?

[servolk, 2016-08-25 00:50:18]

servolk@chromium.org changed reviewers:
+ fs@opera.com

[servolk, 2016-08-25 00:50:19]

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

https://codereview.chromium.org/2263823002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2482:
webMediaPlayer()->selectedVideoTrackChanged(track->selected() ? &id : nullptr);
On 2016/08/24 16:08:46, mlamouri (slow) wrote:
> Should you pass `&track->id()` instead of making a copy on the stack and
passing
> the pointer?

No, that causes an error:
../../third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2481:69: error:
taking the address of a temporary object of type 'WebMediaPlayer::TrackId' (aka
'blink::WebString') [-Waddress-of-temporary]
    webMediaPlayer()->selectedVideoTrackChanged(track->selected() ? &track->id()
: nullptr);
So I had to use an explicit temp variable.

[servolk, 2016-08-25 00:50:29]

The CQ bit was checked by servolk@chromium.org

[servolk, 2016-08-25 00:50:54]

On 2016/08/25 00:50:19, servolk wrote:
>
https://codereview.chromium.org/2263823002/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):
> 
>
https://codereview.chromium.org/2263823002/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2482:
> webMediaPlayer()->selectedVideoTrackChanged(track->selected() ? &id :
nullptr);
> On 2016/08/24 16:08:46, mlamouri (slow) wrote:
> > Should you pass `&track->id()` instead of making a copy on the stack and
> passing
> > the pointer?
> 
> No, that causes an error:
> ../../third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2481:69: error:
> taking the address of a temporary object of type 'WebMediaPlayer::TrackId'
(aka
> 'blink::WebString') [-Waddress-of-temporary]
>     webMediaPlayer()->selectedVideoTrackChanged(track->selected() ?
&track->id()
> : nullptr);
> So I had to use an explicit temp variable.

+fs@ for third_party/WebKit/Source/core/html/track/ changes

[commit-bot: I haz the power, 2016-08-25 00:51:07]

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, 2016-08-25 01:03:54]

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

[commit-bot: I haz the power, 2016-08-25 01:03:56]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[servolk, 2016-08-25 01:33:56]

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

[commit-bot: I haz the power, 2016-08-25 01:35:17]

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, 2016-08-25 03:37:31]

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

[commit-bot: I haz the power, 2016-08-25 03:37:32]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fs, 2016-08-25 08:08:20]

lgtm

[servolk, 2016-08-25 15:34:15]

The CQ bit was checked by servolk@chromium.org

[servolk, 2016-08-25 15:34:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org, wolenetz@chromium.org
Link to the patchset: https://codereview.chromium.org/2263823002/#ps100001
(title: "Call VideoTrackList::trackSelected also the SourceBuffer track list")

[commit-bot: I haz the power, 2016-08-25 15:35:10]

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, 2016-08-25 18:12:09]

Description was changed from

==========
Deliver change notifications to SourceBuffer track lists.

Also update the activeSourceBuffers list on track status changes.

BUG=639151
==========

to

==========
Deliver change notifications to SourceBuffer track lists.

Also update the activeSourceBuffers list on track status changes.

BUG=639151
==========

[commit-bot: I haz the power, 2016-08-25 18:12:11]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-25 18:16:07]

Description was changed from

==========
Deliver change notifications to SourceBuffer track lists.

Also update the activeSourceBuffers list on track status changes.

BUG=639151
==========

to

==========
Deliver change notifications to SourceBuffer track lists.

Also update the activeSourceBuffers list on track status changes.

BUG=639151

Committed: https://crrev.com/01d67d202e8af55c43274d346ed3fb56a16bac65
Cr-Commit-Position: refs/heads/master@{#414475}
==========

[commit-bot: I haz the power, 2016-08-25 18:16:08]

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