[Web Animations] Implement more CSSPropertyBlendingClient methods
Created attachment 336651 [details] Patch
Comment on attachment 336651 [details] Patch Attachment 336651 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7123663 New failing tests: imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/addition-per-property.html imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html animations/trigger-container-scroll-empty.html
Created attachment 336652 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 336651 [details] Patch Attachment 336651 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7123668 New failing tests: imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/addition-per-property.html imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property.html imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/accumulation-per-property.html
Created attachment 336653 [details] Archive of layout-test-results from ews105 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 336655 [details] Patch
Comment on attachment 336655 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336655&action=review > Source/WebCore/animation/KeyframeEffectReadOnly.cpp:733 > + // An emtpy transform list matches anything. Typo empty x 3 Is there a way to unify these matching implementations > LayoutTests/fast/shapes/shape-outside-floats/shape-outside-shape-image-threshold-animation.html:1 > +<!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] --> Missing doctype?
(In reply to Jon Lee from comment #7) > Comment on attachment 336655 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=336655&action=review > > > Source/WebCore/animation/KeyframeEffectReadOnly.cpp:733 > > + // An emtpy transform list matches anything. > > Typo empty x 3 I blame whoever wrote that code originally :) > Is there a way to unify these matching implementations I'll look into it. > > LayoutTests/fast/shapes/shape-outside-floats/shape-outside-shape-image-threshold-animation.html:1 > > +<!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] --> > > Missing doctype? Some tests just don't have them. I'd like to make minimal changes as much as possible.
Comment on attachment 336655 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336655&action=review Shouldn't we be copying all these tests into a new directory, and have that version include the flag to use Web Animations? Otherwise we're only testing one of the code paths. >>> Source/WebCore/animation/KeyframeEffectReadOnly.cpp:733 >>> + // An emtpy transform list matches anything. >> >> Typo empty x 3 >> >> Is there a way to unify these matching implementations > > I blame whoever wrote that code originally :) 50% chance it was me.
Comment on attachment 336655 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336655&action=review > LayoutTests/platform/mac-sierra/imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property-expected.txt:141 > +FAIL filter: percentage or numeric-specifiable functions (number value) assert_equals: The value should be brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3) at 500ms expected "brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3)" but got "brightness(0.30000000000000004) contrast(0.30000000000000004) grayscale(0.30000000000000004) invert(0.30000000000000004) opacity(0.30000000000000004) saturate(0.30000000000000004) sepia(0.30000000000000004)" > +FAIL filter: percentage or numeric-specifiable functions (percentage value) assert_equals: The value should be brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3) at 500ms expected "brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3)" but got "brightness(0.30000000000000004) contrast(0.30000000000000004) grayscale(0.30000000000000004) invert(0.30000000000000004) opacity(0.30000000000000004) saturate(0.30000000000000004) sepia(0.30000000000000004)" These changes are a bit concerning. Why?
(In reply to Dean Jackson from comment #9) > Comment on attachment 336655 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=336655&action=review > > Shouldn't we be copying all these tests into a new directory, and have that > version include the flag to use Web Animations? Otherwise we're only testing > one of the code paths. We don't care about the old code path, it'll go away as soon as we're done moving all the tests. > >>> Source/WebCore/animation/KeyframeEffectReadOnly.cpp:733 > >>> + // An emtpy transform list matches anything. > >> > >> Typo empty x 3 > >> > >> Is there a way to unify these matching implementations > > > > I blame whoever wrote that code originally :) > > 50% chance it was me. One could find out :)
(In reply to Dean Jackson from comment #10) > Comment on attachment 336655 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=336655&action=review > > > LayoutTests/platform/mac-sierra/imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/interpolation-per-property-expected.txt:141 > > +FAIL filter: percentage or numeric-specifiable functions (number value) assert_equals: The value should be brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3) at 500ms expected "brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3)" but got "brightness(0.30000000000000004) contrast(0.30000000000000004) grayscale(0.30000000000000004) invert(0.30000000000000004) opacity(0.30000000000000004) saturate(0.30000000000000004) sepia(0.30000000000000004)" > > +FAIL filter: percentage or numeric-specifiable functions (percentage value) assert_equals: The value should be brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3) at 500ms expected "brightness(0.3) contrast(0.3) grayscale(0.3) invert(0.3) opacity(0.3) saturate(0.3) sepia(0.3)" but got "brightness(0.30000000000000004) contrast(0.30000000000000004) grayscale(0.30000000000000004) invert(0.30000000000000004) opacity(0.30000000000000004) saturate(0.30000000000000004) sepia(0.30000000000000004)" > > These changes are a bit concerning. Why? We'll fix this test eventually, we want to get 100% conformance on the WPT web-animations tests.
Committed r230033: <https://trac.webkit.org/changeset/230033>
<rdar://problem/38962430>
The tests modified with this change are failing an assertion: ASSERTION FAILED: !frame().animation().hasAnimations() ./page/FrameView.cpp(593) : void WebCore::FrameView::didDestroyRenderTree() 1 0x10ba8361d WTFCrash 2 0x11d3f3e37 WebCore::FrameView::didDestroyRenderTree() 3 0x11ca5da7e WebCore::Document::destroyRenderTree() 4 0x11ca5dd21 WebCore::Document::prepareForDestruction() 5 0x11d3d5a60 WebCore::Frame::setView(WTF::RefPtr<WebCore::FrameView, WTF::DumbPtrTraits<WebCore::FrameView> >&&) 6 0x1162ca053 WebFrameLoaderClient::transitionToCommittedForNewPage() 7 0x11d23431f WebCore::FrameLoader::transitionToCommitted(WebCore::CachedPage*) 8 0x11d2333ba WebCore::FrameLoader::commitProvisionalLoad() 9 0x11d1dcb3c WebCore::DocumentLoader::commitIfReady() 10 0x11d1dcea2 WebCore::DocumentLoader::finishedLoading() 11 0x11d1e6d99 WebCore::DocumentLoader::maybeLoadEmpty() 12 0x11d1e6f0e WebCore::DocumentLoader::startLoadingMainResource() 13 0x11d252884 WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, bool, WebCore::AllowNavigationToInvalidURL)::$_14::operator()() const 14 0x11d2525c9 WTF::Function<void ()>::CallableWrapper<WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, bool, WebCore::AllowNavigationToInvalidURL)::$_14>::call() 15 0x11ac5be2b WTF::Function<void ()>::operator()() const 16 0x11d23164c WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, bool, WebCore::AllowNavigationToInvalidURL) 17 0x11d24f4ac WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WebCore::FormState*, WebCore::AllowNavigationToInvalidURL, WTF::CompletionHandler<void ()>&&)::$_9::operator()(WebCore::ResourceRequest const&, WebCore::FormState*, bool) const 18 0x11d24f392 WTF::Function<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>::CallableWrapper<WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WebCore::FormState*, WebCore::AllowNavigationToInvalidURL, WTF::CompletionHandler<void ()>&&)::$_9>::call(WebCore::ResourceRequest&&, WebCore::FormState*, bool) 19 0x11d28208d WTF::Function<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>::operator()(WebCore::ResourceRequest&&, WebCore::FormState*, bool) const 20 0x11d274e19 WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>::operator()(WebCore::ResourceRequest&&, WebCore::FormState*, bool) const 21 0x11d27463c WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest&&, bool, WebCore::DocumentLoader*, WebCore::FormState*, WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>&&) 22 0x11d230e4c WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WebCore::FormState*, WebCore::AllowNavigationToInvalidURL, WTF::CompletionHandler<void ()>&&) 23 0x11d22d46b WebCore::FrameLoader::load(WebCore::DocumentLoader*) 24 0x11d23024d WebCore::FrameLoader::load(WebCore::FrameLoadRequest&&) 25 0x1162ba996 -[WebFrame loadRequest:] 26 0x10b10e577 runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) 27 0x10b10c233 runTestingServerLoop() 28 0x10b10b742 dumpRenderTree(int, char const**) 29 0x10b10ea22 DumpRenderTreeMain(int, char const**) 30 0x10b190922 main 31 0x7fff4fed8115 start https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK1%20(Tests)/r230034%20(3046)/results.html
Reverted r230033 for reason: The LayoutTests modified in this change fail an assertion on WK1. Committed r230043: <https://trac.webkit.org/changeset/230043>
Wish EWS had caught that earlier. Anyway, we shouldn't even be calling into the CSSAnimationController in this codepath, I will conditionalize this ASSERT.
Committed r230068: <https://trac.webkit.org/changeset/230068>