The DocumentTimeline constructor now takes a DocumentTimelineOptions argument.[1] We should update our implementation to match. [1] https://w3c.github.io/web-animations/#dictdef-documenttimelineoptions Spec changeset: https://github.com/w3c/web-animations/commit/61d65b7f77ad26536a7aa838ef2fc9a489446d9b

I forgot modify idlharness test. https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3659a5ee0ac

Comment on attachment 8794667 [details] Bug 1302038 part 1 - Add DocumentTimeline constructor tests. https://reviewboard.mozilla.org/r/80990/#review80180 r=me with the following changes ::: testing/web-platform/tests/web-animations/interfaces/DocumentTimeline/constructor.html:13 (Diff revision 1) > test(function(t) { > - var timeline = new DocumentTimeline(0); > + assert_throws({ name: 'TypeError'}, function() { new DocumentTimeline(0); }); > +}, 'We can\'t create the DocumentTimeline object with wrong type parameter.'); We don't need to test this since this is really just testing that the browser implements WebIDL correctly (and if we can't assume that, there are a *lot* of other things we need to test!) ::: testing/web-platform/tests/web-animations/interfaces/DocumentTimeline/constructor.html:21 (Diff revision 1) > > +test(function(t) { > + var timeline = new DocumentTimeline(); > + > + assert_times_equal(timeline.currentTime, document.timeline.currentTime); > +}, 'Use default origin time when it does not have the parameter.'); 'An origin time of zero is used when none is supplied' (Strictly speaking this is also testing the WebIDL bindings, but in this case it's probably ok.) ::: testing/web-platform/tests/web-animations/interfaces/DocumentTimeline/constructor.html:23 (Diff revision 1) > +test(function(t) { > + var timeline = new DocumentTimeline({ time: 100 }); > + assert_times_equal(timeline.currentTime, document.timeline.currentTime); > +}, 'Use default origin time when it does not have correct dictionary member.'); Again, this is testing the WebIDL bindings so let's just drop this test. ::: testing/web-platform/tests/web-animations/interfaces/DocumentTimeline/constructor.html:31 (Diff revision 1) > +}, 'Use default origin time when it does not have correct dictionary member.'); > + > +test(function(t) { > + var timeline = new DocumentTimeline({ originTime: 0 }); > assert_times_equal(timeline.currentTime, document.timeline.currentTime); > -}, 'zero origin time'); > +}, 'It is same with default document timeline if origin time is zero.'); 'A zero origin time produces a document timeline with a current time identical to the default document timeline' ::: testing/web-platform/tests/web-animations/interfaces/DocumentTimeline/constructor.html:38 (Diff revision 1) > test(function(t) { > - var timeline = new DocumentTimeline(10 * MS_PER_SEC); > + var timeline = new DocumentTimeline({ originTime: 10 * MS_PER_SEC }); > > assert_times_equal(timeline.currentTime, > (document.timeline.currentTime - 10 * MS_PER_SEC)); > }, 'positive origin time'); While we're modifying this file, we could make this test description more helpful. e.g. 'A positive origin time makes the document timeline\'s current time lag behind the default document timeline' ::: testing/web-platform/tests/web-animations/interfaces/DocumentTimeline/constructor.html:45 (Diff revision 1) > test(function(t) { > - var timeline = new DocumentTimeline(-10 * MS_PER_SEC); > + var timeline = new DocumentTimeline({ originTime: -10 * MS_PER_SEC }); > > assert_times_equal(timeline.currentTime, > (document.timeline.currentTime + 10 * MS_PER_SEC)); > }, 'negative origin time'); Likewise here, e.g. 'A negative origin time makes the document timeline\'s current time run ahead of the default document timeline'

Comment on attachment 8794668 [details] Bug 1302038 part 2 - Add DocumentTimelineOptions dictionary. https://reviewboard.mozilla.org/r/80992/#review80186 r=me with the following change ::: dom/animation/DocumentTimeline.cpp:57 (Diff revision 1) > + TimeDuration originTime; > + if (aOptions.mOriginTime) { > + originTime = TimeDuration::FromMilliseconds(aOptions.mOriginTime); > + } else { > + originTime = TimeDuration(0); > + } I don't think we need this branch right? Can't we just do: TimeDuration originTime = TimeDuration::FromMilliseconds(aOptions.mOriginTime);

Thanks Brian, I modified tests and parameter's check based on comment 6, comment 7. Hi Olli, I've changed the DocumentTimeline constructor's parameter based on Web Animations Spec[1]. [1] https://w3c.github.io/web-animations/#dictdef-documenttimelineoptions Could you please review the change of this interface?

Comment on attachment 8794668 [details] Bug 1302038 part 2 - Add DocumentTimelineOptions dictionary. https://reviewboard.mozilla.org/r/80992/#review80250

Pushed by mantaroh@gmail.com: https://hg.mozilla.org/integration/autoland/rev/d41cbb317f73 part 1 - Add DocumentTimeline constructor tests. r=birtles https://hg.mozilla.org/integration/autoland/rev/eff98b9440e9 part 2 - Add DocumentTimelineOptions dictionary. r=birtles,smaug

https://hg.mozilla.org/mozilla-central/rev/d41cbb317f73 https://hg.mozilla.org/mozilla-central/rev/eff98b9440e9

Updated the following MDN pages: https://developer.mozilla.org/en-US/docs/Web/API/DocumentTimeline/DocumentTimeline https://developer.mozilla.org/en-US/docs/Web/API/DocumentTimeline

Mark 51 as won't fix. If it's worth uplifting to 51, feel free to nominate it.