New issue
Advanced search Search tips

Issue 866833 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 26
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Web page crashes when updating a CSS animation property after setting its effect to null

Reported by anto...@graougraou.com, Jul 24

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0 Safari/605.1.15

Steps to reproduce the problem:
1. Ensure all the experimental Web Platform features are enabled
2. Load the test
3. There is no step 3

What is the expected behavior?
The page doesn't crash.

What went wrong?
The page crashed.

Did this work before? No 

Does this work in other browsers? No

Chrome version: 70.0.3501.0  Channel: canary
OS Version: OS X 10.13.6
Flash Version: 29.0.0.147

I came across this while looking at spots where we (WebKit) were accessing an animation's effect without a null check. I expect something similar is wrong in Chrome.
 
animation-crash.html
600 bytes View Download
Labels: Stability-Crash
Status: Available (was: Unconfirmed)
Owner: smcgruer@chromium.org
Status: Started (was: Available)
Thanks for the report Antoine! Grabbing a stack trace:

DevTools listening on ws://127.0.0.1:40875/devtools/browser/a1ade824-f1ee-412c-b8a2-14e766a4b355
Received signal 11 SEGV_MAPERR 0000000000e0
#0 0x7fc62dc75ccc base::debug::StackTrace::StackTrace()
#1 0x7fc62dc757a1 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7fc623b6e0c0 <unknown>
#3 0x7fc6270fa346 blink::CSSAnimations::MaybeApplyPendingUpdate()
#4 0x7fc627462c9c blink::Element::StyleForLayoutObject()
#5 0x7fc627463c98 blink::Element::RecalcOwnStyle()
#6 0x7fc6274634eb blink::Element::RecalcStyle()

Looks like MaybeApplyPendingUpdate uses the effect without any sort of null check: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/css/css_animations.cc?l=505&rcl=8a2c888acc4d52b4d01f20ed2137ea35f08d6ba1

The fix is likely to just null-check that usage, but I want to convince myself that its ok for it to be null there first :)
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c619e7874a19e5d09b753a8891d336f96eef6b2a

commit c619e7874a19e5d09b753a8891d336f96eef6b2a
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Thu Jul 26 13:11:24 2018

Null-check the effect when updating a CSS Animation

Using the Web Animations getAnimations() API, it is possible to set the
effect of a CSS animation to null. Doing so previously caused a crash
when processing updated animations - this CL fixes that.

Bug:  866833 
Change-Id: I3689b6d2b69cf9ccfca82010fa0d0d55ee931314
Reviewed-on: https://chromium-review.googlesource.com/1150404
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578278}
[add] https://crrev.com/c619e7874a19e5d09b753a8891d336f96eef6b2a/third_party/WebKit/LayoutTests/animations/stability/css-animation-null-effect-crash-expected.txt
[add] https://crrev.com/c619e7874a19e5d09b753a8891d336f96eef6b2a/third_party/WebKit/LayoutTests/animations/stability/css-animation-null-effect-crash.html
[modify] https://crrev.com/c619e7874a19e5d09b753a8891d336f96eef6b2a/third_party/blink/renderer/core/animation/css/css_animations.cc

Status: Fixed (was: Started)

Sign in to add a comment