Make media tracks aware of their associated track list

[jdm]

Per #22622 (comment) the AudioTrack, VideoTrack, and TextTrack interfaces will need to store a member pointing at the track list object that they are associated with. This will look something like DomRefCell<Option<Dom<AudioTrackList>>> and it will be updated when the track object is added to a list or removed from one.

Code: components/script/dom/audiotrack.rs, components/script/dom/videotrack.rs, components/texttrack.rs
Tests: ./mach test-wpt tests/wpt/web-platform-tests/html/semantics/embedded-content/media-elements/track/track-element/

[jdm]

@moonlightdrive This might be a good first issue!

[moonlightdrive]

Thanks @jdm! I'll take a look at this one

[kunalmohan]

@jdm what changes are still required here? I see that the required member has been defined but without the DomRefCell wrapper

[jdm]

@kunalmohan I believe we still need the DomRefCell so that the associated track list can be updated when it's added or removed. That's the work that still needs to happen here.

[kunalmohan]

Okay. I'll start working on this issue.

[kunalmohan]

I am a bit confused here so I just want to confirm I am thinking in the right direction here. I will have to update the track_list member of struct AudioTrack in this function, right?

servo/components/script/dom/audiotracklist.rs

Lines 105 to 107 in bac9903

	pub fn add(&self, track: &AudioTrack) {
	self.tracks.borrow_mut().push(Dom::from_ref(track));
	}

I don't see any function that removes a track from a tracklist, so what about updating it during removal of track object?

[jdm]

servo/components/script/dom/audiotracklist.rs

Lines 109 to 111 in bac9903

	pub fn clear(&self) {
	self.tracks.borrow_mut().clear();
	}

removes all tracks from a track list.

Also, as part of this work we should implement the setting behaviour from https://html.spec.whatwg.org/multipage/media.html#dom-videotrack-selected, which is the motivation for this issue.

[jdm]

And to explicitly answer your question: yes.

[kunalmohan]

texttracklist.rs does not have a clear() function, but has the following function:

servo/components/script/dom/texttracklist.rs

Lines 100 to 109 in bb5cd02

	// FIXME(#22314, dlrobertson) allow TextTracks to be
	// removed from the TextTrackList.
	#[allow(dead_code)]
	pub fn remove(&self, idx: usize) {
	self.dom_tracks.borrow_mut().remove(idx);
	self.upcast::<EventTarget>()
	.fire_event(atom!("removetrack"));
	}
	}

Is there a need for clear() function here or should I proceed with the current code?

[kunalmohan]

Also, as part of this work we should implement the setting behaviour from https://html.spec.whatwg.org/multipage/media.html#dom-videotrack-selected, which is the motivation for this issue.

I think the setting behaviour is already implemented here.

servo/components/script/dom/videotracklist.rs

Lines 73 to 107 in bb5cd02

	pub fn set_selected(&self, idx: usize, value: bool) {
	let track = match self.item(idx) {
	Some(t) => t,
	None => return,
	};
	// If the chosen tracks selected status is the same as the new status, return early.
	if track.selected() == value {
	return;
	}
	let global = &self.global();
	let this = Trusted::new(self);
	let (source, canceller) = global
	.as_window()
	.task_manager()
	.media_element_task_source_with_canceller();
	if let Some(current) = self.selected_index() {
	self.tracks.borrow()[current].set_selected(false);
	}
	track.set_selected(value);
	if let Some(media_element) = self.media_element.as_ref() {
	media_element.set_video_track(idx, value);
	}
	let _ = source.queue_with_canceller(
	task!(media_track_change: move || {
	let this = this.root();
	this.upcast::<EventTarget>().fire_event(atom!("change"));
	}),
	&canceller,
	);
	}

Or is there something that need to be implemented yet?

Sorry for troubling you with so many questions 😅 . Being a beginner at rust and web-platforms, I am still trying to understand the concepts implemented here.

Thanks for the help 😄

[jdm]

The setting that I was describing is implemented in

servo/components/script/dom/videotrack.rs

Lines 104 to 112 in bb5cd02

	// https://html.spec.whatwg.org/multipage/#dom-videotrack-selected
	fn SetSelected(&self, value: bool) {
	if let Some(list) = self.track_list.as_ref() {
	if let Some(idx) = list.find(self) {
	list.set_selected(idx, value);
	}
	}
	self.set_selected(value);
	}

. This would not match the specification if the video track list was cleared, and then some JS invoked the selected setter on a video track that was cleared. Does that make sense?

[kunalmohan]

Yeah, that makes sense. So, on clearing a tracklist member I have updated the track_list member of each associated track to None. I think that should solve it.

[kunalmohan]

texttracklist.rs does not have a clear() function, but has the following function:

servo/components/script/dom/texttracklist.rs

Lines 100 to 109 in bb5cd02

	// FIXME(#22314, dlrobertson) allow TextTracks to be
	// removed from the TextTrackList.
	#[allow(dead_code)]
	pub fn remove(&self, idx: usize) {
	self.dom_tracks.borrow_mut().remove(idx);
	self.upcast::<EventTarget>()
	.fire_event(atom!("removetrack"));
	}
	}

Is there a need for clear() function here or should I proceed with the current code?

What about this?

[jdm]

The remove method isn't used yet, but it would be nice to add the same support for updating the removed track's associated list.