Web Animations: blink::EffectInput::Convert incorrectly early-exits for null elements |
||||
Issue description
In EffectInput::Convert we early-exit if the element is null:
EffectModel* EffectInput::Convert(
Element* element,
const DictionarySequenceOrDictionary& effect_input,
ExecutionContext* execution_context,
ExceptionState& exception_state) {
if (effect_input.IsNull() || !element)
return nullptr;
...
}
This is not correct via the spec (http://w3c.github.io/web-animations/#processing-a-keyframes-argument), which makes no mention of the value of element* - the implementation is required to process the keyframes argument irregardless.
This bug causes us to fail the below test, because all properties are considered unsupported, no test()s are registered, and the test times out.
external/wpt/web-animations/animation-model/animation-types/interpolation-per-property.html
All properties are considered unsupported because the test uses an accessor hook to determine if the property is supported. The spec requires that the property is accessed irregardless of the value of element*. The requirement comes from one of two places:
"3. Let input properties be the result of calling the EnumerableOwnNames operation with keyframe input as the object."
OR
"6. For each property name in animation properties,
1. Let raw value be the result of calling the [[Get]] internal method on keyframe input, with property name as the property key and keyframe input as the receiver."
It's not clear why EffectInput::Convert early-exits on null element, but we should likely not be doing that (and should remove the requirement to dereference the element).
,
Oct 5 2017
,
Oct 6 2017
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c48db31689105ce81dcbe640416e45860299c248 commit c48db31689105ce81dcbe640416e45860299c248 Author: Stephen McGruer <smcgruer@chromium.org> Date: Thu Oct 12 20:34:35 2017 Update crbug links for some failing WPT web-animations tests Bug: 600248 , 771722, 771751, 771977, 771985 , 772014 , 772048, 772060, 772076 Change-Id: Ie08474f751fd45484627c8a52d84db13ca6b39ac Reviewed-on: https://chromium-review.googlesource.com/702536 Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: Robert Flack <flackr@chromium.org> Cr-Commit-Position: refs/heads/master@{#508447} [modify] https://crrev.com/c48db31689105ce81dcbe640416e45860299c248/third_party/WebKit/LayoutTests/TestExpectations
,
Feb 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a4de97d98d814d010155ca0a9ee5fad035eaec4 commit 2a4de97d98d814d010155ca0a9ee5fad035eaec4 Author: Stephen McGruer <smcgruer@chromium.org> Date: Tue Feb 27 18:48:54 2018 Allow null elements in EffectInput::Convert By the spec, a KeyframeEffect may be created with a null target, although unfortunately the spec doesn't then say what Document to use to resolve style/etc. This CL adds null-target handling, falling back to the Document extracted from the ScriptState if there is no target. In terms of changes to tests: - Three WPT tests no longer timeout (woo), but unfortunately they currently crash on a DCHECK for setting transform-box to 'border-box' and so are still disabled. See http://crbug.com/816534 - A number of tests for processing a keyframes argument start failing due to a bug in that code; previously this went unseen because the tests pass null for the element. Bug: 772014 , 816534 Change-Id: I01502c82bf758a796fe83c87ed395c6b9f6c4e11 Reviewed-on: https://chromium-review.googlesource.com/934625 Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: Robert Flack <flackr@chromium.org> Cr-Commit-Position: refs/heads/master@{#539509} [modify] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/LayoutTests/external/wpt/web-animations/animation-model/animation-types/accumulation-per-property-expected.txt [add] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/LayoutTests/external/wpt/web-animations/animation-model/animation-types/addition-per-property-expected.txt [add] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/LayoutTests/external/wpt/web-animations/animation-model/animation-types/interpolation-per-property-expected.txt [modify] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/LayoutTests/external/wpt/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001-expected.txt [modify] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/Source/core/animation/AnimationInputHelpers.cpp [modify] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/Source/core/animation/AnimationInputHelpers.h [modify] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/Source/core/animation/EffectInput.cpp [modify] https://crrev.com/2a4de97d98d814d010155ca0a9ee5fad035eaec4/third_party/WebKit/Source/core/animation/EffectInput.h
,
Feb 27 2018
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d11642dfab4c938c44f4b3c96ed5d7da75ccd25b commit d11642dfab4c938c44f4b3c96ed5d7da75ccd25b Author: Stephen McGruer <smcgruer@chromium.org> Date: Thu Mar 01 19:34:07 2018 Add Mac-specific expectations for three web-animation WPT tests These tests originally passed the CQ mac bots because they are allowed to crash due to http://crbug.com/816534 , and the CQ mac bot runs with DCHECK enabled. Unfortunately however that hide the fact that on non-DCHECK macs, the test has different output. Bug: 772014 Change-Id: I4bbaeb853426ad48488111a667a6caf4ba7284f6 Reviewed-on: https://chromium-review.googlesource.com/939670 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#540255} [modify] https://crrev.com/d11642dfab4c938c44f4b3c96ed5d7da75ccd25b/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/d11642dfab4c938c44f4b3c96ed5d7da75ccd25b/third_party/WebKit/LayoutTests/platform/mac/external/wpt/web-animations/animation-model/animation-types/accumulation-per-property-expected.txt [add] https://crrev.com/d11642dfab4c938c44f4b3c96ed5d7da75ccd25b/third_party/WebKit/LayoutTests/platform/mac/external/wpt/web-animations/animation-model/animation-types/addition-per-property-expected.txt [add] https://crrev.com/d11642dfab4c938c44f4b3c96ed5d7da75ccd25b/third_party/WebKit/LayoutTests/platform/mac/external/wpt/web-animations/animation-model/animation-types/interpolation-per-property-expected.txt |
||||
►
Sign in to add a comment |
||||
Comment 1 by smcgruer@chromium.org
, Oct 5 2017