Summary: | [Web Animations] Implement Element.animate() | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||||||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | dino, ews-watchlist, rniwa, webkit-bug-importer | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=181116 https://bugs.webkit.org/show_bug.cgi?id=181119 https://bugs.webkit.org/show_bug.cgi?id=181121 https://bugs.webkit.org/show_bug.cgi?id=181122 https://bugs.webkit.org/show_bug.cgi?id=181123 |
||||||||||||||||||
Attachments: |
|
Description
Antoine Quint
2017-12-05 02:28:44 PST
Created attachment 330115 [details]
Patch
Created attachment 330118 [details]
Patch
Comment on attachment 330118 [details] Patch Attachment 330118 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5801251 New failing tests: imported/w3c/web-platform-tests/css-timing-1/step-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/frames-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/cubic-bezier-timing-functions-output.html imported/blink/virtual/threaded/animations/compositor-rotate-zero-degrees.html Created attachment 330119 [details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 330118 [details] Patch Attachment 330118 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5801268 New failing tests: imported/blink/virtual/threaded/animations/compositor-rotate-zero-degrees.html imported/w3c/web-platform-tests/css-timing-1/step-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/frames-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/cubic-bezier-timing-functions-output.html Created attachment 330120 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 330118 [details] Patch Attachment 330118 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5801279 New failing tests: imported/w3c/web-platform-tests/css-timing-1/step-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/frames-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/cubic-bezier-timing-functions-output.html imported/blink/virtual/threaded/animations/compositor-rotate-zero-degrees.html Created attachment 330121 [details]
Archive of layout-test-results from ews113 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 330118 [details] Patch Attachment 330118 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5801290 New failing tests: imported/w3c/web-platform-tests/css-timing-1/step-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/frames-timing-functions-output.html imported/w3c/web-platform-tests/css-timing-1/cubic-bezier-timing-functions-output.html imported/blink/virtual/threaded/animations/compositor-rotate-zero-degrees.html Created attachment 330122 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 330124 [details]
Patch
Comment on attachment 330124 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330124&action=review > Source/WebCore/animation/AnimationEffectTiming.cpp:55 > + double durationAsDouble = WTF::get<double>(duration); auto > Source/WebCore/animation/AnimationEffectTiming.cpp:59 > + m_duration = Seconds::fromMilliseconds(durationAsDouble); > + } else if (WTF::holds_alternative<String>(duration)) { Return here. The only other option is String. > Source/WebCore/animation/AnimationEffectTiming.cpp:61 > + String durationAsString = WTF::get<String>(duration); > + if (durationAsString != "auto") No need for the local variable. > Source/WebCore/animation/AnimationEffectTimingProperties.h:44 > +struct AnimationEffectTimingProperties { > + double delay; > + double endDelay; > + FillMode fill; > + double iterationStart; > + double iterations; > + Variant<double, String> duration; > + PlaybackDirection direction; > + String easing; > +}; Explicitly set all the default values? Or does the bindings code do that? > Source/WebCore/animation/KeyframeEffect.cpp:383 > + if (options) { > + Variant<double, String> bindingsDuration; > + if (WTF::holds_alternative<double>(options.value())) > + bindingsDuration = WTF::get<double>(options.value()); > + else > + bindingsDuration = WTF::get<KeyframeEffectOptions>(options.value()).duration; > + auto setBindingsDurationResult = keyframeEffect->timing()->setBindingsDuration(WTFMove(bindingsDuration)); > + if (setBindingsDurationResult.hasException()) > + return setBindingsDurationResult.releaseException(); > + } Get the .value() as a local variable to avoid calling it so many times. Also, you could use switch_on here, rather than holds_alternative, etc. > Source/WebCore/animation/KeyframeEffect.h:45 > + ~KeyframeEffect() { } Indentation. > Source/WebCore/animation/KeyframeEffectOptions.h:38 > +struct KeyframeEffectOptions : AnimationEffectTimingProperties { > + IterationCompositeOperation iterationComposite; > + CompositeOperation composite; > +}; Ditto on default values. > Source/WebCore/dom/Element.cpp:3700 > + if (WTF::holds_alternative<double>(options.value())) > + keyframeEffectOptionsVariant = WTF::get<double>(options.value()); > + else > + keyframeEffectOptionsVariant = WTF::get<KeyframeAnimationOptions>(options.value()); Another case where you could use switch_on, and use a local variable for the value (you wouldn't even need it with switch_on) Committed r226289: <https://trac.webkit.org/changeset/226289> |