From e0ef378b20e4d52dfe76ef9d48675686ede21cce Mon Sep 17 00:00:00 2001
From: Hiroyuki Ikezoe <hikezoe@mozilla.com>
Date: Wed, 4 Oct 2017 12:55:22 +0900
Subject: [PATCH] Use atom for animation-name property

---
 components/style/properties/gecko.mako.rs | 31 ++++++++---------------
 components/style/values/mod.rs            | 11 ++++++++
 ports/geckolib/glue.rs                    |  4 +--
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index c20470a27aa..e1625ff7491 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -3215,42 +3215,33 @@ fn static_assert() {
         where I: IntoIterator<Item = longhands::animation_name::computed_value::single_value::T>,
               I::IntoIter: ExactSizeIterator
     {
-
         let v = v.into_iter();
         debug_assert!(v.len() != 0);
         unsafe { self.gecko.mAnimations.ensure_len(v.len()) };
 
         self.gecko.mAnimationNameCount = v.len() as u32;
         for (servo, gecko) in v.zip(self.gecko.mAnimations.iter_mut()) {
-            // TODO This is inefficient. We should fix this in bug 1329169.
-            gecko.mName.assign(match servo.0 {
-                Some(ref name) => name.as_atom().as_slice(),
-                None => &[],  // Empty string for 'none'
-            });
+            let atom = match servo.0 {
+                None => atom!(""),
+                Some(ref name) => name.as_atom().clone(),
+            };
+            unsafe { bindings::Gecko_SetAnimationName(gecko, atom.into_addrefed()); }
         }
     }
     pub fn animation_name_at(&self, index: usize)
         -> longhands::animation_name::computed_value::SingleComputedValue {
         use properties::longhands::animation_name::single_value::SpecifiedValue as AnimationName;
-        // XXX: Is there any effective ways?
-        let atom = &self.gecko.mAnimations[index].mName;
-        if atom.is_empty() {
+
+        let atom = self.gecko.mAnimations[index].mName.mRawPtr;
+        if atom == atom!("").as_ptr() {
             AnimationName(None)
         } else {
-            AnimationName(Some(KeyframesName::from_ident(&atom.to_string())))
+            AnimationName(Some(KeyframesName::from_atom(atom.into())))
         }
     }
     pub fn copy_animation_name_from(&mut self, other: &Self) {
-        unsafe { self.gecko.mAnimations.ensure_len(other.gecko.mAnimations.len()) };
-
-        let count = other.gecko.mAnimationNameCount;
-        self.gecko.mAnimationNameCount = count;
-
-        // The length of mAnimations is often greater than mAnimationXXCount,
-        // don't copy values over the count.
-        for (index, animation) in self.gecko.mAnimations.iter_mut().enumerate().take(count as usize) {
-            animation.mName.assign(&*other.gecko.mAnimations[index].mName);
-        }
+        self.gecko.mAnimationNameCount = other.gecko.mAnimationNameCount;
+        unsafe { bindings::Gecko_CopyAnimationNames(&mut self.gecko.mAnimations, &other.gecko.mAnimations); }
     }
 
     pub fn reset_animation_name(&mut self, other: &Self) {
diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs
index 15978aa04a3..f338cb357c0 100644
--- a/components/style/values/mod.rs
+++ b/components/style/values/mod.rs
@@ -146,6 +146,17 @@ impl KeyframesName {
         }
     }
 
+    /// Create a new KeyframesName from Atom.
+    #[cfg(feature = "gecko")]
+    pub fn from_atom(atom: Atom) -> Self {
+        debug_assert_ne!(atom, atom!(""));
+
+        // FIXME: We might want to preserve <string>, but currently Gecko
+        // stores both of <custom-ident> and <string> into nsAtom, so
+        // we can't tell it.
+        KeyframesName::Ident(CustomIdent(atom))
+    }
+
     /// The name as an Atom
     pub fn as_atom(&self) -> &Atom {
         match *self {
diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs
index 8e5751c5c89..e41ff305d59 100644
--- a/ports/geckolib/glue.rs
+++ b/ports/geckolib/glue.rs
@@ -3625,14 +3625,14 @@ fn fill_in_missing_keyframe_values(
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetBorrowed,
-                                                     name: *const nsACString,
+                                                     name: *mut nsAtom,
                                                      inherited_timing_function: nsTimingFunctionBorrowed,
                                                      keyframes: RawGeckoKeyframeListBorrowedMut) -> bool {
     debug_assert!(keyframes.len() == 0,
                   "keyframes should be initially empty");
 
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
-    let name = unsafe { Atom::from(name.as_ref().unwrap().as_str_unchecked()) };
+    let name = Atom::from(name);
 
     let animation = match data.stylist.get_animation(&name) {
         Some(animation) => animation,