[Web Animations] Always expose "composite" in output of getKeyframes()

https://bugs.webkit.org/show_bug.cgi?id=182600

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Update expected results to account for a number of progressions.

* web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/composite-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt:

Source/WebCore:

The Web Animations spec has changed and whereas the output of getKeyframes() would not feature the "composite" property
for "null" values, it now lists "composite" in all cases.

To support this, we needed to fix an issue in the code generator such that a "null" default value for an optional enum
type was supported.

* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::getKeyframes):
* animation/KeyframeEffect.h:
* animation/KeyframeEffect.idl:
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateDefaultValue):
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::convertDictionary<TestObj::Dictionary>):
(WebCore::convertDictionaryToJS):
* bindings/scripts/test/TestObj.idl:

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

LayoutTests/imported/w3c/ChangeLog

@@ -1,3 +1,16 @@
+2018-02-08  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Always expose "composite" in output of getKeyframes()
+        https://bugs.webkit.org/show_bug.cgi?id=182600
+
+        Reviewed by Dean Jackson.
+
+        Update expected results to account for a number of progressions.
+
+        * web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/composite-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt:
+
 2018-02-07  Youenn Fablet  <youenn@apple.com>
 
         Remove www1/www2 URLs from service worker tests

LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/composite-expected.txt

@@ -1,6 +1,6 @@
 
 PASS Default value 
 PASS Change composite value 
-FAIL Unspecified keyframe composite value when setting effect composite assert_equals: unspecified keyframe composite value should be null even if effect composite is set expected (object) null but got (undefined) undefined
+PASS Unspecified keyframe composite value when setting effect composite 
 PASS Specified keyframe composite value when setting effect composite 
 

Source/WebCore/ChangeLog

@@ -1,3 +1,27 @@
+2018-02-08  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Always expose "composite" in output of getKeyframes()
+        https://bugs.webkit.org/show_bug.cgi?id=182600
+
+        Reviewed by Dean Jackson.
+
+        The Web Animations spec has changed and whereas the output of getKeyframes() would not feature the "composite" property
+        for "null" values, it now lists "composite" in all cases.
+
+        To support this, we needed to fix an issue in the code generator such that a "null" default value for an optional enum
+        type was supported.
+
+        * animation/KeyframeEffect.cpp:
+        (WebCore::KeyframeEffect::getKeyframes):
+        * animation/KeyframeEffect.h:
+        * animation/KeyframeEffect.idl:
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateDefaultValue):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::convertDictionary<TestObj::Dictionary>):
+        (WebCore::convertDictionaryToJS):
+        * bindings/scripts/test/TestObj.idl:
+
 2018-02-08  Youenn Fablet  <youenn@apple.com>
 
         CoreAudioCaptureSourceIOSListener should be a WebProcess singleton

Source/WebCore/animation/KeyframeEffect.cpp

@@ -451,33 +451,26 @@ Vector<Strong<JSObject>> KeyframeEffect::getKeyframes(ExecState& state)
         // 1. Initialize a dictionary object, output keyframe, using the following definition:
         //
         // dictionary BaseComputedKeyframe {
-        //      double?            offset = null;
-        //      double             computedOffset;
-        //      DOMString          easing = "linear";
-        //      CompositeOperation composite;
+        //      double?             offset = null;
+        //      double              computedOffset;
+        //      DOMString           easing = "linear";
+        //      CompositeOperation? composite = null;
         // };
 
-        // 2. Set offset, computedOffset, easing members of output keyframe to the respective values keyframe offset, computed keyframe offset,
-        // and keyframe-specific timing function of keyframe.
-
-        // 3. If keyframe has a keyframe-specific composite operation, set composite to that value.
+        // 2. Set offset, computedOffset, easing, composite members of output keyframe to the respective values keyframe offset, computed keyframe
+        // offset, keyframe-specific timing function and keyframe-specific composite operation of keyframe.
         BaseComputedKeyframe computedKeyframe;
         computedKeyframe.offset = m_offsets[i];
         computedKeyframe.computedOffset = keyframe.key();
         computedKeyframe.easing = m_timingFunctions[i]->cssText();
+        computedKeyframe.composite = m_compositeOperations[i];
 
         auto outputKeyframe = convertDictionaryToJS(state, *jsCast<JSDOMGlobalObject*>(state.lexicalGlobalObject()), computedKeyframe);
 
-        // If we have a non-null CompositeOperation for this keyframes, set the JS property for it.
-        if (auto compositeOperation = m_compositeOperations[i]) {
-            PropertyDescriptor propertyDescriptor(convertEnumerationToJS(state, compositeOperation.value()), 0);
-            JSObject::defineOwnProperty(outputKeyframe, &state, AtomicString("composite").impl(), propertyDescriptor, false);
-        }
-
         auto& style = *keyframe.style();
         auto computedStyleExtractor = ComputedStyleExtractor(m_target.get());
 
-        // 4. For each animation property-value pair specified on keyframe, declaration, perform the following steps:
+        // 3. For each animation property-value pair specified on keyframe, declaration, perform the following steps:
         for (auto cssPropertyId : keyframe.properties()) {
             // 1. Let property name be the result of applying the animation property name to IDL attribute name algorithm to the property name of declaration.
             auto propertyName = CSSPropertyIDToIDLAttributeName(cssPropertyId);
@@ -490,7 +483,7 @@ Vector<Strong<JSObject>> KeyframeEffect::getKeyframes(ExecState& state)
             JSObject::defineOwnProperty(outputKeyframe, &state, AtomicString(propertyName).impl(), PropertyDescriptor(value, 0), false);
         }
 
-        // 5. Append output keyframe to result.
+        // 4. Append output keyframe to result.
         result.append(JSC::Strong<JSC::JSObject> { state.vm(), outputKeyframe });
     }
 

Source/WebCore/animation/KeyframeEffect.h

@@ -73,6 +73,7 @@ class KeyframeEffect final : public AnimationEffect
         std::optional<double> offset;
         double computedOffset;
         String easing { "linear" };
+        std::optional<CompositeOperation> composite;
     };
 
     Element* target() const { return m_target.get(); }

Source/WebCore/animation/KeyframeEffect.idl

@@ -48,4 +48,5 @@ dictionary BasePropertyIndexedKeyframe {
      double? offset = null;
      double computedOffset;
      DOMString easing = "linear";
+     CompositeOperation? composite = null;
 };

Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

@@ -2188,6 +2188,10 @@ sub GenerateDefaultValue
     if ($codeGenerator->IsEnumType($type)) {
         # FIXME: Would be nice to report an error if the value does not have quote marks around it.
         # FIXME: Would be nice to report an error if the value is not one of the enumeration values.
+        if ($defaultValue eq "null") {
+            die if !$type->isNullable;
+            return "std::nullopt";
+        }
         my $className = GetEnumerationClassName($type, $typeScope);
         my $enumerationValueName = GetEnumerationValueName(substr($defaultValue, 1, -1));
         return $className . "::" . $enumerationValueName;

Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp

@@ -637,6 +637,18 @@ template<> TestObj::Dictionary convertDictionary<TestObj::Dictionary>(ExecState&
         RETURN_IF_EXCEPTION(throwScope, { });
     } else
         result.largeIntegerWithDefault = 0;
+    JSValue nullableEnumValue;
+    if (isNullOrUndefined)
+        nullableEnumValue = jsUndefined();
+    else {
+        nullableEnumValue = object->get(&state, Identifier::fromString(&state, "nullableEnum"));
+        RETURN_IF_EXCEPTION(throwScope, { });
+    }
+    if (!nullableEnumValue.isUndefined()) {
+        result.nullableEnum = convert<IDLNullable<IDLEnumeration<TestObj::EnumType>>>(state, nullableEnumValue);
+        RETURN_IF_EXCEPTION(throwScope, { });
+    } else
+        result.nullableEnum = std::nullopt;
     JSValue nullableIntegerWithDefaultValue;
     if (isNullOrUndefined)
         nullableIntegerWithDefaultValue = jsUndefined();
@@ -999,6 +1011,8 @@ JSC::JSObject* convertDictionaryToJS(JSC::ExecState& state, JSDOMGlobalObject& g
     }
     auto largeIntegerWithDefaultValue = toJS<IDLLongLong>(dictionary.largeIntegerWithDefault);
     result->putDirect(vm, JSC::Identifier::fromString(&vm, "largeIntegerWithDefault"), largeIntegerWithDefaultValue);
+    auto nullableEnumValue = toJS<IDLNullable<IDLEnumeration<TestObj::EnumType>>>(state, dictionary.nullableEnum);
+    result->putDirect(vm, JSC::Identifier::fromString(&vm, "nullableEnum"), nullableEnumValue);
     auto nullableIntegerWithDefaultValue = toJS<IDLNullable<IDLLong>>(dictionary.nullableIntegerWithDefault);
     result->putDirect(vm, JSC::Identifier::fromString(&vm, "nullableIntegerWithDefault"), nullableIntegerWithDefaultValue);
     auto nullableNodeValue = toJS<IDLNullable<IDLInterface<Node>>>(state, globalObject, dictionary.nullableNode);

Source/WebCore/bindings/scripts/test/TestObj.idl

@@ -490,6 +490,7 @@ typedef any AnyTypedef;
     unsigned long long unsignedLargeIntegerWithDefault = 0;
     long? nullableIntegerWithDefault = null;
     Node? nullableNode = null;
+    TestEnumType? nullableEnum = null;
     any anyValue;
     any anyValueWithNullDefault = null;
     [ImplementedAs=foo] any fooAlias;