[Video] Add dummy video/audio tracks inactive to select them later.

[Video] Add dummy video/audio tracks inactive to select them later.

Also relax some DCHECKs w.r.t the MediaTracks API being enabled.

BUG=669159
TEST=manual + existing tests.

Committed: https://crrev.com/9ff62f749846d5550a3f90abff959e0745afba5a
Cr-Commit-Position: refs/heads/master@{#435437}

[whywhat, 2016-11-28 20:41:02]

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

[commit-bot: I haz the power, 2016-11-28 20:42:16]

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

[whywhat, 2016-11-28 20:42:17]

avayvod@chromium.org changed reviewers:
+ mlamouri@chromium.org

[whywhat, 2016-11-28 20:42:17]

PTaL

[commit-bot: I haz the power, 2016-11-28 23:35:09]

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

[commit-bot: I haz the power, 2016-11-28 23:35:10]

Dry run: This issue passed the CQ dry run.

[whywhat, 2016-11-29 21:19:22]

avayvod@chromium.org changed reviewers:
+ chcunningham@chromium.org

[whywhat, 2016-11-29 21:19:22]

+Chris for MediaStream/SourceBuffer

[chcunningham, 2016-11-29 22:35:57]

lgtm

[chcunningham, 2016-11-29 22:39:18]

nit

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:287: bool
hasAudioVideoTracks() {
You can make this a media:: namespaced utilitiy method. See
https://cs.chromium.org/chromium/src/media/base/media.h

[chcunningham, 2016-11-29 22:40:28]

On 2016/11/29 22:39:18, chcunningham wrote:
> nit
> 
>
https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):
> 
>
https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:287: bool
> hasAudioVideoTracks() {
> You can make this a media:: namespaced utilitiy method. See
> https://cs.chromium.org/chromium/src/media/base/media.h

Would probably rename to something more like media::audioVideoTracksEnabled ---
enabled is more clear than "has" IMHO

[mlamouri (slow - plz ping), 2016-11-30 09:34:00]

lgtm - should servok@ have a look at this?

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:287: bool
hasAudioVideoTracks() {
On 2016/11/29 at 22:39:18, chcunningham wrote:
> You can make this a media:: namespaced utilitiy method. See
https://cs.chromium.org/chromium/src/media/base/media.h

That wouldn't be available from Blink. Or did you mean creating something
similar in Blink?

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:290: }
nit: can you name the method hasAudioVideoTracksEnabled()? I think the name
might be very confusing otherwise.

[whywhat, 2016-11-30 20:43:26]

Extracted checks into HTMLMediaElement

[whywhat, 2016-11-30 20:45:39]

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:287: bool
hasAudioVideoTracks() {
On 2016/11/30 at 09:34:00, mlamouri wrote:
> On 2016/11/29 at 22:39:18, chcunningham wrote:
> > You can make this a media:: namespaced utilitiy method. See
https://cs.chromium.org/chromium/src/media/base/media.h
> 
> That wouldn't be available from Blink. Or did you mean creating something
similar in Blink?

I've put it into HTMLMediaElement as a public static method. Both MediaSource
and SourceBuffer already use HTMLMediaElement and depend on it supporting media
tracks so seems fair.

https://codereview.chromium.org/2532233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:290: }
On 2016/11/30 at 09:34:00, mlamouri wrote:
> nit: can you name the method hasAudioVideoTracksEnabled()? I think the name
might be very confusing otherwise.

The meaning is not about the feature enabled but if the media element uses media
tracks for the default source (if the page is not using the tracks itself).

mediaTracksEnabledInternally it is :)

[whywhat, 2016-11-30 20:45:44]

The CQ bit was checked by avayvod@chromium.org

[whywhat, 2016-11-30 20:45:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
chcunningham@chromium.org, mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2532233003/#ps20001
(title: "Extracted checks into HTMLMediaElement")

[commit-bot: I haz the power, 2016-11-30 20:46:13]

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-11-30 22:06:10]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480538744125230,
"parent_rev": "640dd6fdd41cb214262d4d14d35163f1e6933bef", "commit_rev":
"1db481a9f3ee3a165d851d2cc9ea5e11a25f3a1a"}

[commit-bot: I haz the power, 2016-11-30 22:06:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-30 22:09:33]

Description was changed from

==========
[Video] Add dummy video/audio tracks inactive to select them later.

Also relax some DCHECKs w.r.t the MediaTracks API being enabled.

BUG=669159
TEST=manual + existing tests.
==========

to

==========
[Video] Add dummy video/audio tracks inactive to select them later.

Also relax some DCHECKs w.r.t the MediaTracks API being enabled.

BUG=669159
TEST=manual + existing tests.

Committed: https://crrev.com/9ff62f749846d5550a3f90abff959e0745afba5a
Cr-Commit-Position: refs/heads/master@{#435437}
==========

[commit-bot: I haz the power, 2016-11-30 22:09:34]

Patchset 2 (id:??) landed as
https://crrev.com/9ff62f749846d5550a3f90abff959e0745afba5a
Cr-Commit-Position: refs/heads/master@{#435437}

[servolk, 2016-12-05 19:55:43]

On 2016/11/30 22:09:34, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/9ff62f749846d5550a3f90abff959e0745afba5a
> Cr-Commit-Position: refs/heads/master@{#435437}

Sorry for the delay, I was OOO and didn't get a chance to look at this earlier.
TBH I'm not sure this CL actually fixes the issue. Because the DCHECK in
crbug.com/669159 has nothing to do with track selected/deselected status. It's
actually about not being able to map selected video track id to a DemuxerStream
object, since the video track id wasn't found in the
ChunkDemuxer::track_id_to_demux_stream_map_. The track id must be in that map
regardless of whether or not the video track is selected.
I'd love to help with investigating this issue, but so far I haven't been able
to reproduce it on Chromium master ToT, even after reverting commit
9ff62f749846d5550a3f90abff959e0745afba5a and running with
--enable-features=BackgroundVideoTrackOptimization flag, which causes the
background video track to be disabled/deselected (which is actually happening,
I've confirmed via chrome://media-internals and looking at the logs).
If adding new audio/video tracks in the initial selected state does indeed
somehow cause that DCHECK, then let's try to create a unit test that would
demonstrate the bad behavior.