| Summary: | [Web Animations] Ensure that changing the timing model updates styles synchronously | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||
| Component: | New Bugs | Assignee: | Antoine Quint <graouts> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, dino, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Antoine Quint
2018-02-15 12:56:26 PST
Created attachment 333936 [details]
Patch
Comment on attachment 333936 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333936&action=review > Source/WebCore/animation/AnimationEffectReadOnly.cpp:47 > + m_timing->setEffect(nullptr); Won't m_timing go away and stop referencing the effect anyway? > Source/WebCore/animation/AnimationEffectTimingReadOnly.cpp:114 > - m_iterationStart = iterationStart; > + if (m_iterationStart != iterationStart) { > + m_iterationStart = iterationStart; > + propertyDidChange(); > + } > + While it is more core, we usually write it as an early return: if (m_iterationStart == iterationStart) return { }; m_iterationStart = iterationStart; propertyDidChange(); return { }; > Source/WebCore/animation/AnimationEffectTimingReadOnly.cpp:127 > + m_iterations = iterations; DItto (In reply to Dean Jackson from comment #2) > Comment on attachment 333936 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333936&action=review > > > Source/WebCore/animation/AnimationEffectReadOnly.cpp:47 > > + m_timing->setEffect(nullptr); > > Won't m_timing go away and stop referencing the effect anyway? Since both effect and timing objects can be referenced from JS, the timing may outlive the effect. > > Source/WebCore/animation/AnimationEffectTimingReadOnly.cpp:114 > > - m_iterationStart = iterationStart; > > + if (m_iterationStart != iterationStart) { > > + m_iterationStart = iterationStart; > > + propertyDidChange(); > > + } > > + > > While it is more core, we usually write it as an early return: > > if (m_iterationStart == iterationStart) > return { }; > > m_iterationStart = iterationStart; > propertyDidChange(); > > return { }; I wanted to avoid the double returns, but I'll adopt that style. > > Source/WebCore/animation/AnimationEffectTimingReadOnly.cpp:127 > > + m_iterations = iterations; > > DItto OK. Created attachment 333940 [details]
Patch for landing
Committed r228537: <https://trac.webkit.org/changeset/228537> |