From 04e127873f9be93a97d923a3a977fab4d75eb992 Mon Sep 17 00:00:00 2001
From: Stephen McGruer <smcgruer@chromium.org>
Date: Wed, 11 Apr 2018 11:23:39 -0700
Subject: [PATCH] Web Animations: Fix bugs in procedure to process a keyframes
 argument

There were three minor bugs left in the implementation:

  - We threw on lists-in-custom-iterators instead of just ignoring them.
  - We returned all properties on the keyframe rather than just those
    defined on the keyframe itself (e.g. we would include prototype
    properties, against spec).
  - We didn't access the properties in ascending unicode order.

Bug: 827573
Change-Id: I213ae5b24e1f35d7f28d16625025122950a6ba88
---
 .../processing-a-keyframes-argument-001.html  | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001.html b/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001.html
index 133fd579041792..6f95cf365095e3 100644
--- a/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001.html
+++ b/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001.html
@@ -307,6 +307,20 @@
 }, 'Keyframes are read from a custom iterator with where an offset is'
    + ' specified');
 
+test(() => {
+  const test_error = { name: 'test' };
+  const bad_keyframe = { get left() { throw test_error; } };
+  assert_throws(test_error, () => {
+    new KeyframeEffect(null, createIterable([
+      { done: false, value: { left: '100px' } },
+      { done: false, value: bad_keyframe },
+      { done: false, value: { left: '200px' } },
+      { done: true },
+    ]));
+  });
+}, 'If a keyframe throws for an animatable property, that exception should be'
+    + ' propagated');
+
 test(() => {
   assert_throws({ name: 'TypeError' }, () => {
     new KeyframeEffect(null, createIterable([
@@ -319,6 +333,39 @@
 }, 'Reading from a custom iterator that returns a non-object keyframe'
    + ' should throw');
 
+// The following two tests are currently against spec, but instead match what
+// we intend the spec to be; see https://github.com/w3c/csswg-drafts/issues/2533
+
+test(() => {
+  const effect = new KeyframeEffect(null, createIterable([
+    { done: false, value: { left: '100px' } },
+    { done: false },  // No value member; keyframe is undefined.
+    { done: false, value: { left: '200px' } },
+    { done: true },
+  ]));
+  assert_frame_lists_equal(effect.getKeyframes(), [
+    { left: '100px', offset: null, computedOffset: 0, easing: 'linear', composite: null },
+    { offset: null, computedOffset: 0.5, easing: 'linear', composite: null },
+    { left: '200px', offset: null, computedOffset: 1, easing: 'linear', composite: null },
+  ]);
+}, 'An undefined keyframe returned from a custom iterator should be treated as a'
+    + ' default keyframe');
+
+test(() => {
+  const effect = new KeyframeEffect(null, createIterable([
+    { done: false, value: { left: '100px' } },
+    { done: false, value: null },
+    { done: false, value: { left: '200px' } },
+    { done: true },
+  ]));
+  assert_frame_lists_equal(effect.getKeyframes(), [
+    { left: '100px', offset: null, computedOffset: 0, easing: 'linear', composite: null },
+    { offset: null, computedOffset: 0.5, easing: 'linear', composite: null },
+    { left: '200px', offset: null, computedOffset: 1, easing: 'linear', composite: null },
+  ]);
+}, 'A null keyframe returned from a custom iterator should be treated as a'
+    + ' default keyframe');
+
 test(() => {
   const effect = new KeyframeEffect(null, createIterable([
     { done: false, value: { left: ['100px', '200px'] } },
@@ -329,6 +376,32 @@
   ]);
 }, 'A list of values returned from a custom iterator should be ignored');
 
+// Test handling of invalid Symbol.iterator
+
+test(() => {
+  const test_error = { name: 'test' };
+  const keyframe_obj = {
+    [Symbol.iterator]() {
+      throw test_error;
+    },
+  };
+  assert_throws(test_error, () => {
+    new KeyframeEffect(null, keyframe_obj);
+  });
+}, 'Accessing a Symbol.iterator property that throws should rethrow');
+
+test(() => {
+  const keyframe_obj = {
+    [Symbol.iterator]() {
+      return 42;  // Not an object.
+    },
+  };
+  assert_throws({ name: 'TypeError' }, () => {
+    new KeyframeEffect(null, keyframe_obj);
+  });
+}, 'A non-object returned from the Symbol.iterator property should cause a'
+    + ' TypeError to be thrown');
+
 test(() => {
   const keyframe = {};
   Object.defineProperty(keyframe, 'width', { value: '200px' });