According to Bug 1335942 Comment 18, we still need to use RawServoAnimationValue for KeyframeEffectReadOnly::GetProperties(), so we don't need to store StyleAnimationValue on stylo anymore.

Comment on attachment 8835271 [details] Bug 1337313 - Part 1: Use AnimationValue in CreatePropertyValue. https://reviewboard.mozilla.org/r/110972/#review112234

Comment on attachment 8835272 [details] Bug 1337313 - Part 2: Add AnimationValue::SerializeSpecifiedValue. https://reviewboard.mozilla.org/r/110974/#review112236 ::: layout/style/StyleAnimationValue.cpp:5239 (Diff revision 1) > + if (mServo) { > + nsTArray<const RawServoAnimationValue*> valueArray { mServo }; > + RefPtr<RawServoDeclarationBlock> declaration = > + Servo_AnimationValues_Uncompute(&valueArray).Consume(); > + MOZ_ASSERT(declaration, "failed to uncompute RawServoAnimationValue"); > + Servo_DeclarationBlock_SerializeOneValue(declaration, aProperty, &aString); This looks to me that it's a bit wasted. I wonder we can add an FFI to convert single AnimationValue to String.

Comment on attachment 8835272 [details] Bug 1337313 - Part 2: Add AnimationValue::SerializeSpecifiedValue. https://reviewboard.mozilla.org/r/110974/#review112236 > This looks to me that it's a bit wasted. I wonder we can add an FFI to convert single AnimationValue to String. Yes, we can. Let me revise it! Thanks for suggestion.

With these patches can we drop the call of ComputeValuesFromStyleContext() in StyleAnimationValue::ComputeValues()?

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6) > With these patches can we drop the call of ComputeValuesFromStyleContext() > in StyleAnimationValue::ComputeValues()? I will try to remove it. If there are no other problems, I will file a bug and attach my patches. Thanks for reminder. :)

Comment on attachment 8835272 [details] Bug 1337313 - Part 2: Add AnimationValue::SerializeSpecifiedValue. https://reviewboard.mozilla.org/r/110974/#review112236 > Yes, we can. Let me revise it! Thanks for suggestion. I cannot use overloading on FFI, so use a different name for uncomputing and serializeing.

Comment on attachment 8835272 [details] Bug 1337313 - Part 2: Add AnimationValue::SerializeSpecifiedValue. https://reviewboard.mozilla.org/r/110974/#review112282 Yay! My review part became simpler! ::: layout/style/ServoBindingList.h:130 (Diff revision 2) > RawServoAnimationValueBorrowed to, > double progress) > SERVO_BINDING_FUNC(Servo_AnimationValues_Uncompute, > RawServoDeclarationBlockStrong, > RawServoAnimationValueBorrowedListBorrowed value) > +SERVO_BINDING_FUNC(Servo_AnimationValues_Uncompute_and_Serialize, void, I think Servo_AnimationValue_Serialize() would be sufficient, but I'd like to hear opitions from Manish. Anyway we should use AnimationValue instead of AnimationValues. ::: layout/style/ServoBindingList.h:134 (Diff revision 2) > RawServoAnimationValueBorrowedListBorrowed value) > +SERVO_BINDING_FUNC(Servo_AnimationValues_Uncompute_and_Serialize, void, > + RawServoAnimationValueBorrowed value, > + nsCSSPropertyID property, > + nsAString* buffer) > SERVO_BINDING_FUNC(Servo_AnimationValues_GetOpacity, float, Oh, I just realized we used AnimationValues here and below as well...

Comment on attachment 8835272 [details] Bug 1337313 - Part 2: Add AnimationValue::SerializeSpecifiedValue. https://reviewboard.mozilla.org/r/110974/#review112282 > I think Servo_AnimationValue_Serialize() would be sufficient, but I'd like to hear opitions from Manish. Anyway we should use AnimationValue instead of AnimationValues. |opinions|.

Comment on attachment 8835272 [details] Bug 1337313 - Part 2: Add AnimationValue::SerializeSpecifiedValue. https://reviewboard.mozilla.org/r/110974/#review112282 > Oh, I just realized we used AnimationValues here and below as well... OK, I can rename these function as: Servo_AnimationValue_Serialize (wait for Manish's opinions) Servo_AnimationValue_GetOpacity Servo_AninationValue_GetTransform

Comment on attachment 8835326 [details] Bug 1337313 - [Servo] Add Servo_AnimationValue_Serialize. https://reviewboard.mozilla.org/r/111014/#review112672

Comment on attachment 8835859 [details] Bug 1337313 - Part 3: Rename Servo_AnimationValues_XXX with Servo_AnimationValue_XXX. https://reviewboard.mozilla.org/r/111424/#review112726 Nice!

gecko try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0241c7941f5e

Pushed by bchiou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/74aa5f52c7c5 Part 1: Use AnimationValue in CreatePropertyValue. r=hiro https://hg.mozilla.org/integration/autoland/rev/4184cf052f15 Part 2: Add AnimationValue::SerializeSpecifiedValue. r=hiro https://hg.mozilla.org/integration/autoland/rev/b83e2b2524c9 Part 3: Rename Servo_AnimationValues_XXX with Servo_AnimationValue_XXX. r=hiro

https://hg.mozilla.org/mozilla-central/rev/74aa5f52c7c5 https://hg.mozilla.org/mozilla-central/rev/4184cf052f15 https://hg.mozilla.org/mozilla-central/rev/b83e2b2524c9