Bug 209682

Summary: [Web Animations] Crash under `KeyframeEffect::getKeyframes` for a `DeclarativeAnimation`
Product: WebKit Reporter: Devin Rousso <hi>
Component: AnimationsAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, ews-watchlist, graouts, graouts, hi, joepeck, saagar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Devin Rousso 2020-03-27 14:52:47 PDT 
# STEPS TO REPRODUCE:
1. inspect any page with a CSS animation/transition
2. select a node with an active CSS animation/transition in the Elements Tab
3. evaluate:

```
let effect = $0.getAnimations()[0].effect;
effect.getKeyframes(); // should not crash
effect.target = null;
effect.getKeyframes(); // will crash
```
Comment 1 Devin Rousso 2020-03-27 14:53:06 PDT 
<rdar://problem/60962458>
Comment 2 Devin Rousso 2020-03-27 14:56:25 PDT 
Created attachment 394761 [details]
Patch
Comment 3 Antoine Quint 2020-03-28 02:40:54 PDT 
Comment on attachment 394761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=394761&action=review

Very nice, thanks for spotting – and fixing – that!

> LayoutTests/webanimations/css-animation-effect-target-change-and-get-keyframes-crash.html:1
> +<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] -->

We really don't need this but anymore, this is the default and there's No Going Back.
Comment 4 Devin Rousso 2020-03-30 10:04:59 PDT 
Comment on attachment 394761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=394761&action=review

Thanks for the review Antoine!

>> LayoutTests/webanimations/css-animation-effect-target-change-and-get-keyframes-crash.html:1
>> +<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] -->
> 
> We really don't need this but anymore, this is the default and there's No Going Back.

I appreciate that, but given that the flag still exists, I think it's safe to still have it.  Not to mention, it adds more information about what this test is actually testing.
Comment 5 EWS 2020-03-30 10:15:19 PDT 
Committed r259205: <https://trac.webkit.org/changeset/259205>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394761 [details].
Comment 6 Devin Rousso 2020-04-07 17:05:00 PDT 
*** Bug 210161 has been marked as a duplicate of this bug. ***