1335938 - stylo: Update CSS animations during StyleDocument() instead of RecreateStyleContexts().

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

So that we can incorporate initial animation styles in the first place.

To do this we need to:
1) Make KeyframeEffectReadOnly::SetKeyframes() independent from nsStyleContext.
2) Make all relevant stuff thread-safe

From what I understand, computing StyleAnimationValue for the compositor is the last barrier of this issue.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Initially I thought we can create update_animations() in rust which is equivalent of nsAnimationManager::UpdateAnimations(), but today I realized that we can't do it because of nsAutoAnimationMutationBatch.  We are using nsAutoAnimationMutationBatch, which is a stack base class, to notify animation state changes to devtools, we need to call another variant of nsAnimationManager::UpdateAnimations() to use the stack class from update_aniamtions on servo side.  The another UpdateAnimations requires servo's computed values instead of nsStyleContext* just like what I am doing in bug 1338927.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

It's a tough work to rewrite UpdateAnimations(). It's not only using servo's computed values instead of nsStyleContext but also dropping StyleAnimation.  We need to go back and forth between gecko and servo.  I am trying to bring minimum maintenance cost of both of gecko and stylo, but I am not really sure.  Anyway once we did this here, we can take a similar way for nsTransitionManager.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I missed that nsAutoAnimationMutationBatch is not thread safe now.  If we notified animation change's state on some threads simultaneously, what happens?  We should wait for servo's parallel styling (I mean the finish of StyleDocument()) and the notify all of the changes at once?

Comment 4

[Boris Chiou [:boris]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)
> I missed that nsAutoAnimationMutationBatch is not thread safe now.  If we
> notified animation change's state on some threads simultaneously, what
> happens?  We should wait for servo's parallel styling (I mean the finish of
> StyleDocument()) and the notify all of the changes at once?

Oh, it is not thread-safe, so I think the answer is yes. I also have a similar problem, so need to maintain a list during parallel styling, but the problem is maintaining the list should be thread-safe, so I'm not sure what kind of list could be used efficiently and is there any overhead of those read-write locks.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Bug 1340340 noticed me that CSSAnimation has the same problem.

Comment 6

[Bobby Holley (:bholley)]

Assigning to Hiro just to have an owner. Feel free to reassign or reprioritize.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

This has been also fixed by bug 1341985.