[Web Animations] Distinguish between an omitted and a null timeline argument to the...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jan 2018 09:26:18 +0000 (09:26 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jan 2018 09:26:18 +0000 (09:26 +0000)
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.

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

12 files changed:
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/constructor-expected.txt
LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/animations/reversing-an-animation-expected.txt
LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/animations/set-the-timeline-of-an-animation-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/Sources.txt
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/animation/WebAnimation.cpp
Source/WebCore/animation/WebAnimation.h
Source/WebCore/animation/WebAnimation.idl
Source/WebCore/bindings/js/JSWebAnimationCustom.cpp [new file with mode: 0644]
Source/WebCore/dom/Element.cpp

index ae88471..7292bb3 100644 (file)
@@ -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
index 620ef46..32aba2b 100644 (file)
@@ -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
index 3981901..c0345a3 100644 (file)
@@ -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 
 
index 2b593b2..40b6d9b 100644 (file)
@@ -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 
index bda9470..d59b5af 100644 (file)
@@ -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
index 37eb959..5f653fd 100644 (file)
@@ -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
index 3b008e3..601f41b 100644 (file)
                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>"; };
                                516BB7920CE91E6800512F79 /* JSTreeWalkerCustom.cpp */,
                                BE6DF708171CA2C500DD52B8 /* JSVideoTrackCustom.cpp */,
                                BE6DF70A171CA2C500DD52B8 /* JSVideoTrackListCustom.cpp */,
+                               715DA5D3201BB902002EF2B0 /* JSWebAnimationCustom.cpp */,
                                D3F3D3591A69A3B00059FC2B /* JSWebGL2RenderingContextCustom.cpp */,
                                49EED14C1051971A00099FAB /* JSWebGLRenderingContextCustom.cpp */,
                                31A088C41E737B2C003B6609 /* JSWebGPURenderingContextCustom.cpp */,
index 3ed23c3..2e33a18 100644 (file)
 
 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;
 }
 
index 0c4f9f6..52dc8d4 100644 (file)
@@ -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();
 
index cef8401..b5c12c4 100644 (file)
@@ -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;
diff --git a/Source/WebCore/bindings/js/JSWebAnimationCustom.cpp b/Source/WebCore/bindings/js/JSWebAnimationCustom.cpp
new file mode 100644 (file)
index 0000000..8b56f86
--- /dev/null
@@ -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
index 1b1a446..c363ace 100644 (file)
@@ -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();