New issue
Advanced search Search tips

Issue 631879 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Calling unpauseAnimations after seek causes crash

Project Member Reported by suzyh@chromium.org, Jul 27 2016

Issue description

In the attached testcase, I pause animations, seek into the timespan of the animation, and then unpause, which causes a crash in debug mode on Linux with "ASSERTION FAILED: m_activeState != Active".
 
svg-unpauseAnimations-crash.html
556 bytes View Download

Comment 1 by f...@opera.com, Jul 27 2016

Cc: -f...@opera.com
Labels: -Pri-2 Pri-3
Owner: f...@opera.com
Status: Assigned (was: Untriaged)
I'll assign this to me for the time being (on vacation for 2.5 more weeks though, so consider this up for grabs...)
Cc: hyunjune...@samsung.com
I will also check it. :)
Cc: -hyunjune...@samsung.com
Oops, https://codereview.chromium.org/2248643003/

Comment 4 by f...@opera.com, Aug 16 2016

Yeah, that will fix this too - indirectly =)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 19 2016

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

commit 0b3f0bda7fd496aa15541eedf44d1fe660c730dc
Author: fs <fs@opera.com>
Date: Fri Aug 19 23:29:39 2016

Simplify time tracking in SMILTimeContainer

Instead of 5 difference time fields, use two - one to track the last
seek/pause time in the container ("presentation time"), and one to
track the document time corresponding to that.
Use two bool flags for tracking 'paused' and 'started' state.

Also straighten out code-flow in SMILTimeContainer::begin() to make it
a bit more obvious that we're essentially mirroring the contents of
updateAnimationsAndScheduleFrameIfNeeded. begin() is also renamed into
start(). Pass double to SMILTimeContainer::scheduleAnimationFrame, do
some ASSERT->DCHECK transformations when touching code and touch up
some comments.

BUG= 631879 

Review-Url: https://codereview.chromium.org/2248643003
Cr-Commit-Position: refs/heads/master@{#413283}

[add] https://crrev.com/0b3f0bda7fd496aa15541eedf44d1fe660c730dc/third_party/WebKit/LayoutTests/svg/animations/pause-setcurrenttime-unpause-before-timeline-start.html
[modify] https://crrev.com/0b3f0bda7fd496aa15541eedf44d1fe660c730dc/third_party/WebKit/Source/core/svg/SVGDocumentExtensions.cpp
[modify] https://crrev.com/0b3f0bda7fd496aa15541eedf44d1fe660c730dc/third_party/WebKit/Source/core/svg/SVGSVGElement.cpp
[modify] https://crrev.com/0b3f0bda7fd496aa15541eedf44d1fe660c730dc/third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp
[modify] https://crrev.com/0b3f0bda7fd496aa15541eedf44d1fe660c730dc/third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.h

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 20 2016

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

commit ebd6dde89d88e789ecc2751668506910acba165a
Author: fs <fs@opera.com>
Date: Sat Aug 20 08:57:45 2016

Refactor SMILTimeContainer can-schedule-frame predicate

begin() and updateAnimationsAndScheduleFrameIfNeeded() use the same
predicate to check if they should schedule an animation frame - although
they phrase it slightly differently (because of local knowledge.)
Move the generic version to a canScheduleFrame() method and use that in
both cases.

BUG= 631879 

Review-Url: https://codereview.chromium.org/2257803002
Cr-Commit-Position: refs/heads/master@{#413328}

[modify] https://crrev.com/ebd6dde89d88e789ecc2751668506910acba165a/third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp
[modify] https://crrev.com/ebd6dde89d88e789ecc2751668506910acba165a/third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.h

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 20 2016

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

commit 74765e5a1cda876448a6b44c23c3a9e20e59f53d
Author: fs <fs@opera.com>
Date: Sat Aug 20 14:27:56 2016

Don't schedule a wake-up if the timeline hasn't started

Before the timeline has started we shouldn't update animations. This
makes resume() symmetric with pause().

BUG= 631879 

Review-Url: https://codereview.chromium.org/2254303005
Cr-Commit-Position: refs/heads/master@{#413336}

[modify] https://crrev.com/74765e5a1cda876448a6b44c23c3a9e20e59f53d/third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 20 2016

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

commit a7db22cfba4e58f6335d3e61193018e93164f53b
Author: fs <fs@opera.com>
Date: Sat Aug 20 18:29:29 2016

Replace SMILTime with double for elapsed time in SMILTimeContainer

We don't really make use of of the special properties of SMILTime for
this case, and using double means slightly less impedance mismatching.

BUG= 631879 

Review-Url: https://codereview.chromium.org/2261443002
Cr-Commit-Position: refs/heads/master@{#413345}

[modify] https://crrev.com/a7db22cfba4e58f6335d3e61193018e93164f53b/third_party/WebKit/Source/core/svg/SVGSVGElement.cpp
[modify] https://crrev.com/a7db22cfba4e58f6335d3e61193018e93164f53b/third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.cpp
[modify] https://crrev.com/a7db22cfba4e58f6335d3e61193018e93164f53b/third_party/WebKit/Source/core/svg/animation/SMILTimeContainer.h

Comment 9 by f...@opera.com, Aug 22 2016

Status: Fixed (was: Assigned)

Sign in to add a comment