Fix nullptr deref in InvalidateKeyframeEffect

Fix nullptr deref in InvalidateKeyframeEffect

Animation::InvalidateKeyframeEffect assumed that if it had content then
it had a target element. After implementing the Animation constructor,
it was possible to have created an Animation object with a
KeyframeEffect with no target element. This patch checks whether the
target is nullptr before dereferencing.

BUG=734721
Review-Url: https://codereview.chromium.org/2961573002
Cr-Commit-Position: refs/heads/master@{#482558}
Committed: https://chromium.googlesource.com/chromium/src/+/e4424b5ca106bdb36ad962a9daadb86131576e98

[suzyh_UTC10 (ex-contributor), 2017-06-26 01:58:23]

suzyh@chromium.org changed reviewers:
+ alancutter@chromium.org, meade@chromium.org

[suzyh_UTC10 (ex-contributor), 2017-06-26 01:58:34]

The CQ bit was checked by suzyh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-26 01:58:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-26 03:34:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-26 03:34:42]

Dry run: This issue passed the CQ dry run.

[alancutter (OOO until 2018), 2017-06-26 04:38:47]

lgtm

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/animations/keyframeeffect-no-target-crash.html
(right):

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/animations/keyframeeffect-no-target-crash.html:3:
let effect = new KeyframeEffect(null, null, { duration: 1000});
Remove space after {.

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/Animation.cpp (right):

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/Animation.cpp:1173:
CSSAnimations::IsAffectedByKeyframesFromScope(*target, tree_scope)) {
Please add a TODO to remove the use of CSSAnimations in the Animation code. They
should not be interdependent. This architectural smell is the source of the bug
and further issues unaddressed IMO.

[suzyh_UTC10 (ex-contributor), 2017-06-27 05:12:15]

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/animations/keyframeeffect-no-target-crash.html
(right):

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/animations/keyframeeffect-no-target-crash.html:3:
let effect = new KeyframeEffect(null, null, { duration: 1000});
On 2017/06/26 at 04:38:47, alancutter wrote:
> Remove space after {.

Done.

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/Animation.cpp (right):

https://codereview.chromium.org/2961573002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/Animation.cpp:1173:
CSSAnimations::IsAffectedByKeyframesFromScope(*target, tree_scope)) {
On 2017/06/26 at 04:38:47, alancutter wrote:
> Please add a TODO to remove the use of CSSAnimations in the Animation code.
They should not be interdependent. This architectural smell is the source of the
bug and further issues unaddressed IMO.

Done.

[suzyh_UTC10 (ex-contributor), 2017-06-27 05:12:52]

The CQ bit was checked by suzyh@chromium.org

[suzyh_UTC10 (ex-contributor), 2017-06-27 05:12:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2961573002/#ps20001
(title: "Response to review")

[commit-bot: I haz the power, 2017-06-27 05:13:04]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-27 07:06:25]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498540372618960,
"parent_rev": "17bda8398e00987e311824a5662b752e74d0241c", "commit_rev":
"e4424b5ca106bdb36ad962a9daadb86131576e98"}

[commit-bot: I haz the power, 2017-06-27 07:06:35]

Description was changed from

==========
Fix nullptr deref in InvalidateKeyframeEffect

Animation::InvalidateKeyframeEffect assumed that if it had content then
it had a target element. After implementing the Animation constructor,
it was possible to have created an Animation object with a
KeyframeEffect with no target element. This patch checks whether the
target is nullptr before dereferencing.

BUG=734721
==========

to

==========
Fix nullptr deref in InvalidateKeyframeEffect

Animation::InvalidateKeyframeEffect assumed that if it had content then
it had a target element. After implementing the Animation constructor,
it was possible to have created an Animation object with a
KeyframeEffect with no target element. This patch checks whether the
target is nullptr before dereferencing.

BUG=734721

Review-Url: https://codereview.chromium.org/2961573002
Cr-Commit-Position: refs/heads/master@{#482558}
Committed:
https://chromium.googlesource.com/chromium/src/+/e4424b5ca106bdb36ad962a9daad...
==========

[commit-bot: I haz the power, 2017-06-27 07:06:36]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/e4424b5ca106bdb36ad962a9daad...