Move "id" from KeyframeEffectOptions to KeyframeAnimationOptions and

change ElementAnimtaions to take KeyframeAnimationOptions as arg.

See https://w3c.github.io/web-animations/#dictdef-keyframeanimationoptions

NOTE: this IDL change doesn't affect out shipping behaviour, and has low compatibility risk. Hence there is no need for an intent-to-ship notification.

Blink currently deviates from the spec in that the "id" field should be
in KeyframeAnimationOptions instead of KeyframeEffectOptions. Interface
Element should also take KeyframeAnimationOptions as arg. This cl
contains the following changes
- Add a new KeyframeAnimationOptions interface.
- Move "id" from KeyframeEffectOptions to KeyframeAnimationOptions.
- Change ElementAnimations to take KeyframeAnimationOptions as arg.
- Updates TimingInput to support KeyframeAnimationOptions.
- Add unit tests for TimingInput to cover KeyframeAnimationOptions.

BUG=700267

Review-Url: https://codereview.chromium.org/2875673005
Cr-Commit-Position: refs/heads/master@{#471164}

third_party/WebKit/Source/bindings/core/v8/BUILD.gn

@@ -15,6 +15,8 @@ generated_core_dictionary_files = [
   "$blink_core_output_dir/animation/AnimationEffectTimingProperties.h",
   "$blink_core_output_dir/animation/ComputedTimingProperties.cpp",
   "$blink_core_output_dir/animation/ComputedTimingProperties.h",
+  "$blink_core_output_dir/animation/KeyframeAnimationOptions.cpp",
+  "$blink_core_output_dir/animation/KeyframeAnimationOptions.h",
   "$blink_core_output_dir/animation/KeyframeEffectOptions.cpp",
   "$blink_core_output_dir/animation/KeyframeEffectOptions.h",
   "$blink_core_output_dir/css/FontFaceDescriptors.cpp",
@@ -230,6 +232,8 @@ bindings_core_generated_union_type_files = [
   "$bindings_core_v8_output_dir/StringOrUnrestrictedDoubleSequence.h",
   "$bindings_core_v8_output_dir/USVStringSequenceSequenceOrUSVStringOrURLSearchParams.cpp",
   "$bindings_core_v8_output_dir/USVStringSequenceSequenceOrUSVStringOrURLSearchParams.h",
+  "$bindings_core_v8_output_dir/UnrestrictedDoubleOrKeyframeAnimationOptions.cpp",
+  "$bindings_core_v8_output_dir/UnrestrictedDoubleOrKeyframeAnimationOptions.h",
   "$bindings_core_v8_output_dir/UnrestrictedDoubleOrKeyframeEffectOptions.cpp",
   "$bindings_core_v8_output_dir/UnrestrictedDoubleOrKeyframeEffectOptions.h",
   "$bindings_core_v8_output_dir/UnrestrictedDoubleOrString.cpp",

third_party/WebKit/Source/core/animation/ElementAnimation.h

@@ -32,7 +32,7 @@
 #define ElementAnimation_h
 
 #include "bindings/core/v8/DictionarySequenceOrDictionary.h"
-#include "bindings/core/v8/UnrestrictedDoubleOrKeyframeEffectOptions.h"
+#include "bindings/core/v8/UnrestrictedDoubleOrKeyframeAnimationOptions.h"
 #include "core/animation/DocumentTimeline.h"
 #include "core/animation/EffectInput.h"
 #include "core/animation/ElementAnimations.h"
@@ -52,11 +52,12 @@ class ElementAnimation {
   STATIC_ONLY(ElementAnimation);
 
  public:
-  static Animation* animate(ScriptState* script_state,
-                            Element& element,
-                            const DictionarySequenceOrDictionary& effect_input,
-                            UnrestrictedDoubleOrKeyframeEffectOptions options,
-                            ExceptionState& exception_state) {
+  static Animation* animate(
+      ScriptState* script_state,
+      Element& element,
+      const DictionarySequenceOrDictionary& effect_input,
+      UnrestrictedDoubleOrKeyframeAnimationOptions options,
+      ExceptionState& exception_state) {
     EffectModel* effect = EffectInput::Convert(
         &element, effect_input, ExecutionContext::From(script_state),
         exception_state);
@@ -68,9 +69,9 @@ class ElementAnimation {
                               exception_state))
       return nullptr;
 
-    if (options.isKeyframeEffectOptions()) {
+    if (options.isKeyframeAnimationOptions()) {
       Animation* animation = animate(element, effect, timing);
-      animation->setId(options.getAsKeyframeEffectOptions().id());
+      animation->setId(options.getAsKeyframeAnimationOptions().id());
       return animation;
     }
     return animate(element, effect, timing);

third_party/WebKit/Source/core/animation/ElementAnimation.idl

@@ -36,6 +36,6 @@
 
 partial interface Element {
     // TODO(dstockwell): The argument types do not match the spec.
-    [CallWith=ScriptState, Measure, RaisesException] Animation animate((sequence<Dictionary> or Dictionary)? effect, optional (unrestricted double or KeyframeEffectOptions) options);
+    [CallWith=ScriptState, Measure, RaisesException] Animation animate((sequence<Dictionary> or Dictionary)? effect, optional (unrestricted double or KeyframeAnimationOptions) options);
     [RuntimeEnabled=WebAnimationsAPI] sequence<Animation> getAnimations();
 };

third_party/WebKit/Source/core/animation/KeyframeAnimationOptions.idl

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// https://w3c.github.io/web-animations/#dictdef-keyframeanimationoptions
+
+dictionary KeyframeAnimationOptions : KeyframeEffectOptions {
+    DOMString id = "";
+};

third_party/WebKit/Source/core/animation/KeyframeEffectOptions.idl

@@ -6,5 +6,4 @@
 
 dictionary KeyframeEffectOptions : AnimationEffectTimingProperties {
     // TODO(alancutter): Implement iterationComposite, composite and spacing.
-    DOMString id = "";
 };

third_party/WebKit/Source/core/animation/TimingInput.cpp

@@ -5,6 +5,7 @@
 #include "core/animation/TimingInput.h"
 
 #include "bindings/core/v8/ExceptionState.h"
+#include "bindings/core/v8/UnrestrictedDoubleOrKeyframeAnimationOptions.h"
 #include "bindings/core/v8/UnrestrictedDoubleOrKeyframeEffectOptions.h"
 #include "core/animation/AnimationInputHelpers.h"
 #include "core/animation/KeyframeEffectOptions.h"
@@ -138,6 +139,24 @@ bool TimingInput::Convert(
   return false;
 }
 
+bool TimingInput::Convert(
+    const UnrestrictedDoubleOrKeyframeAnimationOptions& options,
+    Timing& timing_output,
+    Document* document,
+    ExceptionState& exception_state) {
+  if (options.isKeyframeAnimationOptions()) {
+    return Convert(options.getAsKeyframeAnimationOptions(), timing_output,
+                   document, exception_state);
+  } else if (options.isUnrestrictedDouble()) {
+    return Convert(options.getAsUnrestrictedDouble(), timing_output,
+                   exception_state);
+  } else if (options.isNull()) {
+    return true;
+  }
+  NOTREACHED();
+  return false;
+}
+
 bool TimingInput::Convert(const KeyframeEffectOptions& timing_input,
                           Timing& timing_output,
                           Document* document,
@@ -170,6 +189,15 @@ bool TimingInput::Convert(const KeyframeEffectOptions& timing_input,
   return true;
 }
 
+bool TimingInput::Convert(const KeyframeAnimationOptions& timing_input,
+                          Timing& timing_output,
+                          Document* document,
+                          ExceptionState& exception_state) {
+  // The "id" field isn't used, so upcast to KeyframeEffectOptions.
+  const KeyframeEffectOptions* const timing_input_ptr = &timing_input;
+  return Convert(*timing_input_ptr, timing_output, document, exception_state);
+}
+
 bool TimingInput::Convert(double duration,
                           Timing& timing_output,
                           ExceptionState& exception_state) {

third_party/WebKit/Source/core/animation/TimingInput.h

@@ -13,8 +13,10 @@ namespace blink {
 
 class Document;
 class ExceptionState;
+class KeyframeAnimationOptions;
 class KeyframeEffectOptions;
 class UnrestrictedDoubleOrString;
+class UnrestrictedDoubleOrKeyframeAnimationOptions;
 class UnrestrictedDoubleOrKeyframeEffectOptions;
 
 class CORE_EXPORT TimingInput {
@@ -25,10 +27,19 @@ class CORE_EXPORT TimingInput {
                       Timing& timing_output,
                       Document*,
                       ExceptionState&);
+  static bool Convert(const UnrestrictedDoubleOrKeyframeAnimationOptions&,
+                      Timing& timing_output,
+                      Document*,
+                      ExceptionState&);
   static bool Convert(const KeyframeEffectOptions& timing_input,
                       Timing& timing_output,
                       Document*,
                       ExceptionState&);
+  static bool Convert(const KeyframeAnimationOptions& timing_input,
+                      Timing& timing_output,
+                      Document*,
+                      ExceptionState&);
+
   static bool Convert(double duration, Timing& timing_output, ExceptionState&);
 
   static void SetStartDelay(Timing&, double start_delay);

third_party/WebKit/Source/core/animation/TimingInputTest.cpp

@@ -5,6 +5,7 @@
 #include "core/animation/TimingInput.h"
 
 #include "bindings/core/v8/V8BindingForTesting.h"
+#include "bindings/core/v8/V8KeyframeAnimationOptions.h"
 #include "bindings/core/v8/V8KeyframeEffectOptions.h"
 #include "core/animation/AnimationEffectTiming.h"
 #include "core/animation/AnimationTestHelper.h"
@@ -17,38 +18,61 @@ namespace blink {
 Timing ApplyTimingInputNumber(v8::Isolate* isolate,
                               String timing_property,
                               double timing_property_value,
-                              bool& timing_conversion_success) {
+                              bool& timing_conversion_success,
+                              bool is_keyframeeffectoptions = true) {
   v8::Local<v8::Object> timing_input = v8::Object::New(isolate);
   SetV8ObjectPropertyAsNumber(isolate, timing_input, timing_property,
                               timing_property_value);
-  KeyframeEffectOptions timing_input_dictionary;
   DummyExceptionStateForTesting exception_state;
-  V8KeyframeEffectOptions::toImpl(isolate, timing_input,
-                                  timing_input_dictionary, exception_state);
   Timing result;
-  timing_conversion_success =
-      TimingInput::Convert(timing_input_dictionary, result, nullptr,
-                           exception_state) &&
-      !exception_state.HadException();
+  if (is_keyframeeffectoptions) {
+    KeyframeEffectOptions timing_input_dictionary;
+    V8KeyframeEffectOptions::toImpl(isolate, timing_input,
+                                    timing_input_dictionary, exception_state);
+    timing_conversion_success =
+        TimingInput::Convert(timing_input_dictionary, result, nullptr,
+                             exception_state) &&
+        !exception_state.HadException();
+  } else {
+    KeyframeAnimationOptions timing_input_dictionary;
+    V8KeyframeAnimationOptions::toImpl(
+        isolate, timing_input, timing_input_dictionary, exception_state);
+    timing_conversion_success =
+        TimingInput::Convert(timing_input_dictionary, result, nullptr,
+                             exception_state) &&
+        !exception_state.HadException();
+  }
   return result;
 }
 
 Timing ApplyTimingInputString(v8::Isolate* isolate,
                               String timing_property,
                               String timing_property_value,
-                              bool& timing_conversion_success) {
+                              bool& timing_conversion_success,
+                              bool is_keyframeeffectoptions = true) {
   v8::Local<v8::Object> timing_input = v8::Object::New(isolate);
   SetV8ObjectPropertyAsString(isolate, timing_input, timing_property,
                               timing_property_value);
-  KeyframeEffectOptions timing_input_dictionary;
+
   DummyExceptionStateForTesting exception_state;
-  V8KeyframeEffectOptions::toImpl(isolate, timing_input,
-                                  timing_input_dictionary, exception_state);
   Timing result;
-  timing_conversion_success =
-      TimingInput::Convert(timing_input_dictionary, result, nullptr,
-                           exception_state) &&
-      !exception_state.HadException();
+  if (is_keyframeeffectoptions) {
+    KeyframeEffectOptions timing_input_dictionary;
+    V8KeyframeEffectOptions::toImpl(isolate, timing_input,
+                                    timing_input_dictionary, exception_state);
+    timing_conversion_success =
+        TimingInput::Convert(timing_input_dictionary, result, nullptr,
+                             exception_state) &&
+        !exception_state.HadException();
+  } else {
+    KeyframeAnimationOptions timing_input_dictionary;
+    V8KeyframeAnimationOptions::toImpl(
+        isolate, timing_input, timing_input_dictionary, exception_state);
+    timing_conversion_success =
+        TimingInput::Convert(timing_input_dictionary, result, nullptr,
+                             exception_state) &&
+        !exception_state.HadException();
+  }
   return result;
 }
 
@@ -81,6 +105,35 @@ TEST(AnimationTimingInputTest, TimingInputStartDelay) {
                    .start_delay);
 }
 
+TEST(AnimationTimingInputTest, TimingInputStartDelayKeyframeAnimationOptions) {
+  V8TestingScope scope;
+  bool ignored_success;
+  EXPECT_EQ(1.1, ApplyTimingInputNumber(scope.GetIsolate(), "delay", 1100,
+                                        ignored_success, false)
+                     .start_delay);
+  EXPECT_EQ(-1, ApplyTimingInputNumber(scope.GetIsolate(), "delay", -1000,
+                                       ignored_success, false)
+                    .start_delay);
+  EXPECT_EQ(1, ApplyTimingInputString(scope.GetIsolate(), "delay", "1000",
+                                      ignored_success, false)
+                   .start_delay);
+  EXPECT_EQ(0, ApplyTimingInputString(scope.GetIsolate(), "delay", "1s",
+                                      ignored_success, false)
+                   .start_delay);
+  EXPECT_EQ(0, ApplyTimingInputString(scope.GetIsolate(), "delay", "Infinity",
+                                      ignored_success, false)
+                   .start_delay);
+  EXPECT_EQ(0, ApplyTimingInputString(scope.GetIsolate(), "delay", "-Infinity",
+                                      ignored_success, false)
+                   .start_delay);
+  EXPECT_EQ(0, ApplyTimingInputString(scope.GetIsolate(), "delay", "NaN",
+                                      ignored_success, false)
+                   .start_delay);
+  EXPECT_EQ(0, ApplyTimingInputString(scope.GetIsolate(), "delay", "rubbish",
+                                      ignored_success, false)
+                   .start_delay);
+}
+
 TEST(AnimationTimingInputTest, TimingInputEndDelay) {
   V8TestingScope scope;
   bool ignored_success;
@@ -375,4 +428,23 @@ TEST(AnimationTimingInputTest, TimingInputEmpty) {
   EXPECT_EQ(*control_timing.timing_function, *updated_timing.timing_function);
 }
 
+TEST(AnimationTimingInputTest, TimingInputEmptyKeyframeAnimationOptions) {
+  DummyExceptionStateForTesting exception_state;
+  Timing control_timing;
+  Timing updated_timing;
+  bool success = TimingInput::Convert(KeyframeAnimationOptions(),
+                                      updated_timing, nullptr, exception_state);
+  EXPECT_TRUE(success);
+  EXPECT_FALSE(exception_state.HadException());
+
+  EXPECT_EQ(control_timing.start_delay, updated_timing.start_delay);
+  EXPECT_EQ(control_timing.fill_mode, updated_timing.fill_mode);
+  EXPECT_EQ(control_timing.iteration_start, updated_timing.iteration_start);
+  EXPECT_EQ(control_timing.iteration_count, updated_timing.iteration_count);
+  EXPECT_TRUE(std::isnan(updated_timing.iteration_duration));
+  EXPECT_EQ(control_timing.playback_rate, updated_timing.playback_rate);
+  EXPECT_EQ(control_timing.direction, updated_timing.direction);
+  EXPECT_EQ(*control_timing.timing_function, *updated_timing.timing_function);
+}
+
 }  // namespace blink

third_party/WebKit/Source/core/core_idl_files.gni

@@ -512,6 +512,7 @@ core_dictionary_idl_files =
     get_path_info([
                     "animation/AnimationEffectTimingProperties.idl",
                     "animation/ComputedTimingProperties.idl",
+                    "animation/KeyframeAnimationOptions.idl",
                     "animation/KeyframeEffectOptions.idl",
                     "css/FontFaceDescriptors.idl",
                     "css/FontFaceSetLoadEventInit.idl",