Drop flush from nsSMILAnimationController::DoSample

NEW
Unassigned

Status

()

Core
SVG
P3
normal
11 months ago
7 months ago

People

(Reporter: birtles, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox55 affected)

Details

(Reporter)

Description

11 months ago
nsSMILAnimationController::DoSample includes a call to FlushPendingNotifications. This was introduced in bug 592477 because the flush in GetBaseValue was sometimes causing us to crash (see bug 592477 comment 25).

Now that bug 1315874 has removed the flush from GetBaseValue we should, in theory, be able to remove the call to FlushPendingNotifications. This call has been a source of security and performance bugs over the years so it would be nice to remove it.

However, since adding that flush, some code and tests have come to depend on it. If we simply drop it (and with it, the |isStyleFlushNeeded| local variable and |document| kungFuDeathGrip, I suspect) various tests will fail. We need to look through these tests and investigate the cause.

For example, in some cases it might be that the animVal getter for mapped attributes should be doing a flush. In other cases it might be that the test itself is at fault. There might be other cases where we really do need the flush in DoSample, I'm not sure. Nevertheless, it's worth looking into since it might help with performance, it might help avoid future security bugs, and it will probably be necessary to do if we later want to rebase SMIL on top of Web Animations.

[1] http://searchfox.org/mozilla-central/rev/fcd9f1480d65f1a5df2acda97eb07a7e133f6ed4/dom/smil/nsSMILAnimationController.cpp#443

Updated

7 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.