[Web Animations] Distinguish between an omitted and a null timeline a…

…rgument to the Animation constructor

https://bugs.webkit.org/show_bug.cgi?id=179065
LayoutTests/imported/w3c:

Reviewed by Dean Jackson.

Update WPT test output with progressions.

* web-platform-tests/web-animations/interfaces/Animation/constructor-expected.txt:
* web-platform-tests/web-animations/timing-model/animations/reversing-an-animation-expected.txt:
* web-platform-tests/web-animations/timing-model/animations/set-the-timeline-of-an-animation-expected.txt:

Source/WebCore:

<rdar://problem/36869046>

Reviewed by Dean Jackson.

The Web Animations specification requires that a missing or undefined "timeline" parameter means that the
document's timeline should be used, but a null value should be supported. To support this, we need to provide
a custom Animation constructor where we can check on the ExecState whether the second argument passed is
undefined, which is true if an explicit "undefined" value is passed or if the argument does not exist.

* Sources.txt: Add the new JSWebAnimationCustom.cpp file.
* WebCore.xcodeproj/project.pbxproj: Add the new JSWebAnimationCustom.cpp file.
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::create): Add a create() variant that doesn't provide an AnimationTimeline parameter
to clearly indicate that the provided Document's timeline should be used.
* animation/WebAnimation.h:
* animation/WebAnimation.idl:
* bindings/js/JSWebAnimationCustom.cpp: Added.
(WebCore::constructJSWebAnimation): Provide a custom Animation constructor where we check whether the second
argument, the timeline, is undefined.
* dom/Element.cpp:
(WebCore::Element::animate): Use the new create() variant since passing "nullptr" now means a null timeline.

Canonical link: https://commits.webkit.org/198023@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227714 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog

@@ -1,3 +1,16 @@
+2018-01-26  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Distinguish between an omitted and a null timeline argument to the Animation constructor
+        https://bugs.webkit.org/show_bug.cgi?id=179065
+
+        Reviewed by Dean Jackson.
+
+        Update WPT test output with progressions.
+
+        * web-platform-tests/web-animations/interfaces/Animation/constructor-expected.txt:
+        * web-platform-tests/web-animations/timing-model/animations/reversing-an-animation-expected.txt:
+        * web-platform-tests/web-animations/timing-model/animations/set-the-timeline-of-an-animation-expected.txt:
+
 2018-01-26  Youenn Fablet  <youenn@apple.com>
 
         CSP post checks should be done for service worker responses

LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/constructor-expected.txt

@@ -1,5 +1,5 @@
 
-FAIL Animation can be constructed with null effect and null timeline assert_equals: Animation timeline should be null expected null but got object "[object DocumentTimeline]"
+PASS Animation can be constructed with null effect and null timeline 
 PASS Animation can be constructed with null effect and non-null timeline 
 PASS Animation can be constructed with null effect and no timeline parameter 
 FAIL Animation can be constructed with non-null effect and null timeline Can't find variable: KeyframeEffectReadOnly

LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/animations/reversing-an-animation-expected.txt

@@ -13,5 +13,5 @@ PASS When reversing throws an exception, the playback rate remains unchanged
 PASS Reversing animation when playbackRate = 0 and currentTime < 0 and the target effect end is positive infinity should NOT throw an exception 
 PASS Reversing an animation when playbackRate < 0 and currentTime < 0 and the target effect end is positive infinity should make it play from the start 
 PASS Reversing when when playbackRate == 0 should preserve the current time and playback rate 
-FAIL Reversing an animation without an active timeline throws an InvalidStateError assert_throws: function "() => { animation.reverse(); }" did not throw
+PASS Reversing an animation without an active timeline throws an InvalidStateError 
 

LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/animations/set-the-timeline-of-an-animation-expected.txt

@@ -2,8 +2,8 @@
 PASS After setting timeline on paused animation it is still paused 
 PASS After setting timeline on animation paused outside active interval it is still paused 
 PASS After setting timeline on an idle animation without a start time it is still idle 
-FAIL After setting timeline on an idle animation with a start time it is running assert_equals: expected "idle" but got "running"
-FAIL After setting timeline on an idle animation with a sufficiently ancient start time it is finished assert_equals: expected "idle" but got "finished"
+PASS After setting timeline on an idle animation with a start time it is running 
+PASS After setting timeline on an idle animation with a sufficiently ancient start time it is finished 
 FAIL After setting timeline on a play-pending animation it begins playing after pending assert_true: Animation is initially play-pending expected true got false
 FAIL After setting timeline on a pause-pending animation it becomes paused after pending assert_true: Animation is initially pause-pending expected true got false
 PASS After clearing timeline on paused animation it is still paused 

Source/WebCore/ChangeLog

@@ -1,3 +1,29 @@
+2018-01-26  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Distinguish between an omitted and a null timeline argument to the Animation constructor
+        https://bugs.webkit.org/show_bug.cgi?id=179065
+        <rdar://problem/36869046>
+
+        Reviewed by Dean Jackson.
+
+        The Web Animations specification requires that a missing or undefined "timeline" parameter means that the
+        document's timeline should be used, but a null value should be supported. To support this, we need to provide
+        a custom Animation constructor where we can check on the ExecState whether the second argument passed is
+        undefined, which is true if an explicit "undefined" value is passed or if the argument does not exist.
+
+        * Sources.txt: Add the new JSWebAnimationCustom.cpp file.
+        * WebCore.xcodeproj/project.pbxproj: Add the new JSWebAnimationCustom.cpp file.
+        * animation/WebAnimation.cpp:
+        (WebCore::WebAnimation::create): Add a create() variant that doesn't provide an AnimationTimeline parameter
+        to clearly indicate that the provided Document's timeline should be used.
+        * animation/WebAnimation.h:
+        * animation/WebAnimation.idl:
+        * bindings/js/JSWebAnimationCustom.cpp: Added.
+        (WebCore::constructJSWebAnimation): Provide a custom Animation constructor where we check whether the second
+        argument, the timeline, is undefined.
+        * dom/Element.cpp:
+        (WebCore::Element::animate): Use the new create() variant since passing "nullptr" now means a null timeline.
+
 2018-01-26  Ricky Mondello  <rmondello@apple.com>
 
         Use the standard -webkit-autofill color on iOS

Source/WebCore/Sources.txt

@@ -438,6 +438,7 @@ bindings/js/JSTrackCustom.cpp
 bindings/js/JSTreeWalkerCustom.cpp
 bindings/js/JSVideoTrackCustom.cpp
 bindings/js/JSVideoTrackListCustom.cpp
+bindings/js/JSWebAnimationCustom.cpp
 bindings/js/JSWebGL2RenderingContextCustom.cpp
 bindings/js/JSWebGLRenderingContextCustom.cpp
 bindings/js/JSWebGPURenderPassAttachmentDescriptorCustom.cpp

Source/WebCore/WebCore.xcodeproj/project.pbxproj

@@ -8820,6 +8820,7 @@
 		71556CBB1F9F09FE00E78D08 /* JSAnimationEffectTiming.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSAnimationEffectTiming.cpp; sourceTree = "<group>"; };
 		7157E3D11DC1EE4B0094550E /* scrubbing-support.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "scrubbing-support.js"; sourceTree = "<group>"; };
 		7157F061150B6564006EAABD /* SVGAnimatedTransformList.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SVGAnimatedTransformList.cpp; sourceTree = "<group>"; };
+		715DA5D3201BB902002EF2B0 /* JSWebAnimationCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSWebAnimationCustom.cpp; sourceTree = "<group>"; };
 		716C8DF11E48B269005BD0DA /* volume-down-support.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "volume-down-support.js"; sourceTree = "<group>"; };
 		716C8DF21E48B269005BD0DA /* volume-up-support.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "volume-up-support.js"; sourceTree = "<group>"; };
 		716C8DF31E48B284005BD0DA /* volume-down-button.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "volume-down-button.js"; sourceTree = "<group>"; };
@@ -19320,6 +19321,7 @@
 				516BB7920CE91E6800512F79 /* JSTreeWalkerCustom.cpp */,
 				BE6DF708171CA2C500DD52B8 /* JSVideoTrackCustom.cpp */,
 				BE6DF70A171CA2C500DD52B8 /* JSVideoTrackListCustom.cpp */,
+				715DA5D3201BB902002EF2B0 /* JSWebAnimationCustom.cpp */,
 				D3F3D3591A69A3B00059FC2B /* JSWebGL2RenderingContextCustom.cpp */,
 				49EED14C1051971A00099FAB /* JSWebGLRenderingContextCustom.cpp */,
 				31A088C41E737B2C003B6609 /* JSWebGPURenderingContextCustom.cpp */,

Source/WebCore/animation/WebAnimation.cpp

@@ -39,16 +39,20 @@
 
 namespace WebCore {
 
-Ref<WebAnimation> WebAnimation::create(Document& document, AnimationEffect* effect, AnimationTimeline* timeline)
+Ref<WebAnimation> WebAnimation::create(Document& document, AnimationEffect* effect)
 {
     auto result = adoptRef(*new WebAnimation(document));
-
     result->setEffect(effect);
+    result->setTimeline(&document.timeline());
+    return result;
+}
 
-    // FIXME: the spec mandates distinguishing between an omitted timeline parameter
-    // and an explicit null or undefined value (webkit.org/b/179065).
-    result->setTimeline(timeline ? timeline : &document.timeline());
-
+Ref<WebAnimation> WebAnimation::create(Document& document, AnimationEffect* effect, AnimationTimeline* timeline)
+{
+    auto result = adoptRef(*new WebAnimation(document));
+    result->setEffect(effect);
+    if (timeline)
+        result->setTimeline(timeline);
     return result;
 }
 

Source/WebCore/animation/WebAnimation.h

@@ -46,6 +46,7 @@ class RenderStyle;
 
 class WebAnimation final : public RefCounted<WebAnimation>, public EventTargetWithInlineData, public ActiveDOMObject {
 public:
+    static Ref<WebAnimation> create(Document&, AnimationEffect*);
     static Ref<WebAnimation> create(Document&, AnimationEffect*, AnimationTimeline*);
     ~WebAnimation();
 

Source/WebCore/animation/WebAnimation.idl

@@ -35,8 +35,7 @@ enum AnimationPlayState {
     ActiveDOMObject,
     EnabledAtRuntime=WebAnimations,
     InterfaceName=Animation,
-    ConstructorCallWith=Document,
-    Constructor(optional AnimationEffect? effect = null, optional AnimationTimeline? timeline)
+    CustomConstructor()
 ] interface WebAnimation : EventTarget {
     attribute DOMString id;
     attribute AnimationEffect? effect;

Source/WebCore/bindings/js/JSWebAnimationCustom.cpp

@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "JSWebAnimation.h"
+
+#include "JSAnimationEffect.h"
+#include "JSAnimationTimeline.h"
+#include "JSDOMConstructor.h"
+
+namespace WebCore {
+
+using namespace JSC;
+
+EncodedJSValue JSC_HOST_CALL constructJSWebAnimation(ExecState& state)
+{
+    VM& vm = state.vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    UNUSED_PARAM(throwScope);
+    auto* jsConstructor = jsCast<JSDOMConstructorBase*>(state.jsCallee());
+    ASSERT(jsConstructor);
+    auto* context = jsConstructor->scriptExecutionContext();
+    if (UNLIKELY(!context))
+        return throwConstructorScriptExecutionContextUnavailableError(state, throwScope, "Animation");
+    auto& document = downcast<Document>(*context);
+    auto effect = convert<IDLNullable<IDLInterface<AnimationEffect>>>(state, state.argument(0), [](ExecState& state, ThrowScope& scope) {
+        throwArgumentTypeError(state, scope, 0, "effect", "Animation", nullptr, "AnimationEffect");
+    });
+    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+
+    if (state.argument(1).isUndefined()) {
+        auto object = WebAnimation::create(document, WTFMove(effect));
+        return JSValue::encode(toJSNewlyCreated<IDLInterface<WebAnimation>>(state, *jsConstructor->globalObject(), WTFMove(object)));
+    }
+
+    auto timeline = convert<IDLNullable<IDLInterface<AnimationTimeline>>>(state, state.uncheckedArgument(1), [](ExecState& state, ThrowScope& scope) {
+        throwArgumentTypeError(state, scope, 1, "timeline", "Animation", nullptr, "AnimationTimeline");
+    });
+    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+    auto object = WebAnimation::create(document, WTFMove(effect), WTFMove(timeline));
+    return JSValue::encode(toJSNewlyCreated<IDLInterface<WebAnimation>>(state, *jsConstructor->globalObject(), WTFMove(object)));
+}
+
+} // namespace WebCore

Source/WebCore/dom/Element.cpp

@@ -3718,7 +3718,7 @@ ExceptionOr<Ref<WebAnimation>> Element::animate(JSC::ExecState& state, JSC::Stro
     if (keyframeEffectResult.hasException())
         return keyframeEffectResult.releaseException();
 
-    auto animation = WebAnimation::create(document(), &keyframeEffectResult.returnValue().get(), nullptr);
+    auto animation = WebAnimation::create(document(), &keyframeEffectResult.returnValue().get());
     animation->setId(id);
 
     auto animationPlayResult = animation->play();