Commit be588ee8 authored by Fredrik Söderquist's avatar Fredrik Söderquist Committed by Commit Bot

Make TextTrackCue::CueDidChange ordering updates fine-grained

Only do an remove-add cycle for the cue when the start or end time is
updated. This will avoid changing the cue order for cues which need to
make use of the "tie-breaker" (have same 'startTime' and 'endTime'.)
It should also make non-time updates to a cue faster.

Remove media/track/opera/interfaces/VTTCue/snapToLines.html since it's
a duplicate of the WPT test.

Bug: 783779
Change-Id: I919b0acdb6ec2e6f0b6e11fb75996e7292168f95
Reviewed-on: https://chromium-review.googlesource.com/763527Reviewed-by: default avatarsrirama chandra sekhar <srirama.m@samsung.com>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#515845}
parent 9fcebbe1
This is a testharness.js-based test.
PASS VTTCue.snapToLines, script-created cue
FAIL VTTCue.snapToLines, parsed cue assert_true: expected true got false
Harness: the test ran to completion.
This is a testharness.js-based test.
PASS VTTCue.snapToLines, script-created cue
FAIL VTTCue.snapToLines, parsed cue assert_true: expected true got false
Harness: the test ran to completion.
<!doctype html>
<title>VTTCue.snapToLines</title>
<script src=../../../../../resources/testharness.js></script>
<script src=../../../../../resources/testharnessreport.js></script>
<div id=log></div>
<script>
setup(function(){
window.video = document.createElement('video');
window.t1 = video.addTextTrack('subtitles');
document.body.appendChild(video);
});
test(function(){
var c1 = new VTTCue(0, 1, 'text1');
assert_true(c1.snapToLines);
c1.line = 101;
c1.snapToLines = false;
assert_false(c1.snapToLines);
c1.snapToLines = true;
assert_true(c1.snapToLines);
c1.line = -1;
c1.snapToLines = false;
assert_false(c1.snapToLines);
c1.snapToLines = true;
assert_true(c1.snapToLines);
c1.line = 0;
c1.snapToLines = false;
assert_false(c1.snapToLines);
}, document.title+', script-created cue');
var t_parsed = async_test(document.title+', parsed cue');
t_parsed.step(function(){
var t = document.createElement('track');
t.onload = this.step_func(function(){
var c1 = t.track.cues[0];
assert_true(c1.snapToLines);
c1.line = 101;
c1.snapToLines = false;
assert_false(c1.snapToLines);
c1.snapToLines = true;
assert_true(c1.snapToLines);
c1.line = -1;
c1.snapToLines = false;
assert_false(c1.snapToLines);
c1.snapToLines = true;
assert_true(c1.snapToLines);
c1.line = 0;
c1.snapToLines = false;
assert_false(c1.snapToLines);
var c2 = t.track.cues[1];
assert_true(c2.snapToLines);
c2.line = 101;
c2.snapToLines = false;
assert_false(c2.snapToLines);
c2.snapToLines = true;
assert_true(c2.snapToLines);
c2.line = -1;
c2.snapToLines = false;
assert_false(c2.snapToLines);
c2.snapToLines = true;
assert_true(c2.snapToLines);
c2.line = 0;
c2.snapToLines = false;
assert_false(c2.snapToLines);
var c3 = t.track.cues[2];
assert_false(c3.snapToLines);
c3.snapToLines = false;
assert_false(c3.snapToLines);
c3.snapToLines = true;
assert_true(c3.snapToLines);
c3.line = 101;
c3.snapToLines = false;
assert_false(c3.snapToLines);
c3.snapToLines = true;
assert_true(c3.snapToLines);
c3.line = -1;
c3.snapToLines = false;
assert_false(c3.snapToLines);
c3.snapToLines = true;
assert_true(c3.snapToLines);
c3.line = 0;
c3.snapToLines = false;
assert_false(c3.snapToLines);
this.done();
});
t.src = 'data:text/vtt,'+encodeURIComponent('WEBVTT\n\n00:00:00.000 --> 00:00:00.001\ntest\n\n'+
'00:00:00.000 --> 00:00:00.001 line:0\ntest\n\n'+
'00:00:00.000 --> 00:00:00.001 line:0%\ntest');
t.track.mode = 'showing';
video.appendChild(t);
});
</script>
...@@ -286,14 +286,14 @@ void TextTrack::CueWillChange(TextTrackCue* cue) { ...@@ -286,14 +286,14 @@ void TextTrack::CueWillChange(TextTrackCue* cue) {
GetCueTimeline()->RemoveCue(this, cue); GetCueTimeline()->RemoveCue(this, cue);
} }
void TextTrack::CueDidChange(TextTrackCue* cue) { void TextTrack::CueDidChange(TextTrackCue* cue, bool update_cue_index) {
// This method is called through cue->track(), which should imply that this // This method is called through cue->track(), which should imply that this
// track has a list of cues. // track has a list of cues.
DCHECK(cues_ && cue->track() == this); DCHECK(cues_ && cue->track() == this);
// Make sure the TextTrackCueList order is up to date. // Make sure the TextTrackCueList order is up to date.
// FIXME: Only need to do this if the change was to any of the timestamps. if (update_cue_index)
cues_->UpdateCueIndex(cue); cues_->UpdateCueIndex(cue);
// Since a call to cueDidChange is always preceded by a call to // Since a call to cueDidChange is always preceded by a call to
// cueWillChange, the cue should no longer be active when we reach this // cueWillChange, the cue should no longer be active when we reach this
......
...@@ -100,7 +100,7 @@ class CORE_EXPORT TextTrack : public EventTargetWithInlineData, ...@@ -100,7 +100,7 @@ class CORE_EXPORT TextTrack : public EventTargetWithInlineData,
void removeCue(TextTrackCue*, ExceptionState&); void removeCue(TextTrackCue*, ExceptionState&);
void CueWillChange(TextTrackCue*); void CueWillChange(TextTrackCue*);
void CueDidChange(TextTrackCue*); void CueDidChange(TextTrackCue*, bool update_cue_index);
DEFINE_ATTRIBUTE_EVENT_LISTENER(cuechange); DEFINE_ATTRIBUTE_EVENT_LISTENER(cuechange);
......
...@@ -54,9 +54,9 @@ void TextTrackCue::CueWillChange() { ...@@ -54,9 +54,9 @@ void TextTrackCue::CueWillChange() {
track_->CueWillChange(this); track_->CueWillChange(this);
} }
void TextTrackCue::CueDidChange() { void TextTrackCue::CueDidChange(CueMutationAffectsOrder affects_order) {
if (track_) if (track_)
track_->CueDidChange(this); track_->CueDidChange(this, affects_order == kCueMutationAffectsOrder);
} }
TextTrack* TextTrackCue::track() const { TextTrack* TextTrackCue::track() const {
...@@ -87,7 +87,7 @@ void TextTrackCue::setStartTime(double value) { ...@@ -87,7 +87,7 @@ void TextTrackCue::setStartTime(double value) {
CueWillChange(); CueWillChange();
start_time_ = value; start_time_ = value;
CueDidChange(); CueDidChange(kCueMutationAffectsOrder);
} }
void TextTrackCue::setEndTime(double value) { void TextTrackCue::setEndTime(double value) {
...@@ -97,7 +97,7 @@ void TextTrackCue::setEndTime(double value) { ...@@ -97,7 +97,7 @@ void TextTrackCue::setEndTime(double value) {
CueWillChange(); CueWillChange();
end_time_ = value; end_time_ = value;
CueDidChange(); CueDidChange(kCueMutationAffectsOrder);
} }
void TextTrackCue::setPauseOnExit(bool value) { void TextTrackCue::setPauseOnExit(bool value) {
......
...@@ -102,8 +102,13 @@ class TextTrackCue : public EventTargetWithInlineData { ...@@ -102,8 +102,13 @@ class TextTrackCue : public EventTargetWithInlineData {
protected: protected:
TextTrackCue(double start, double end); TextTrackCue(double start, double end);
enum CueMutationAffectsOrder {
kCueMutationDoesNotAffectOrder,
kCueMutationAffectsOrder
};
void CueWillChange(); void CueWillChange();
virtual void CueDidChange(); virtual void CueDidChange(
CueMutationAffectsOrder = kCueMutationDoesNotAffectOrder);
DispatchEventResult DispatchEventInternal(Event*) override; DispatchEventResult DispatchEventInternal(Event*) override;
private: private:
......
...@@ -237,8 +237,8 @@ String VTTCue::ToString() const { ...@@ -237,8 +237,8 @@ String VTTCue::ToString() const {
} }
#endif #endif
void VTTCue::CueDidChange() { void VTTCue::CueDidChange(CueMutationAffectsOrder affects_order) {
TextTrackCue::CueDidChange(); TextTrackCue::CueDidChange(affects_order);
display_tree_should_change_ = true; display_tree_should_change_ = true;
} }
......
...@@ -161,7 +161,8 @@ class VTTCue final : public TextTrackCue { ...@@ -161,7 +161,8 @@ class VTTCue final : public TextTrackCue {
VTTCueBox* GetDisplayTree(); VTTCueBox* GetDisplayTree();
void CueDidChange() override; void CueDidChange(
CueMutationAffectsOrder = kCueMutationDoesNotAffectOrder) override;
void CreateVTTNodeTree(); void CreateVTTNodeTree();
void CopyVTTNodeToDOMTree(ContainerNode* vtt_node, ContainerNode* root); void CopyVTTNodeToDOMTree(ContainerNode* vtt_node, ContainerNode* root);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment