Bug 1153734 part 1 - Remove AnimationEffect; r=smaug

Most of this is fairly obvious. However, the addition of 'override' to
ElementPropertyTransition::Name() is not strictly necessary. It was simply added
because while making these changes I accidentally dropped the 'virtual' keyword
from the method in the superclass and the compiler didn't complain. Adding this
will hopefully make it harder to create the same bug in the future.

--HG--
rename : dom/animation/test/css-animations/test_animation-effect-name.html => dom/animation/test/css-animations/test_animation-name.html
rename : dom/animation/test/css-transitions/test_animation-effect-name.html => dom/animation/test/css-transitions/test_animation-name.html

dom/animation/Animation.cpp

@@ -5,7 +5,6 @@
 
 #include "mozilla/dom/Animation.h"
 #include "mozilla/dom/AnimationBinding.h"
-#include "mozilla/dom/AnimationEffect.h"
 #include "mozilla/FloatingPoint.h"
 #include "AnimationCommon.h"
 #include "nsCSSPropertySet.h"
@@ -71,13 +70,6 @@ Animation::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
   return AnimationBinding::Wrap(aCx, this, aGivenProto);
 }
 
-already_AddRefed<AnimationEffect>
-Animation::GetEffect()
-{
-  nsRefPtr<AnimationEffect> effect = new AnimationEffect(this);
-  return effect.forget();
-}
-
 void
 Animation::SetParentTime(Nullable<TimeDuration> aParentTime)
 {

dom/animation/Animation.h

@@ -184,8 +184,6 @@ struct ElementPropertyTransition;
 
 namespace dom {
 
-class AnimationEffect;
-
 class Animation : public nsWrapperCache
 {
 public:
@@ -219,9 +217,6 @@ public:
   }
 
   // Animation interface
-  // This currently returns a new object each time when used from C++ but is
-  // cached when used from JS.
-  already_AddRefed<AnimationEffect> GetEffect();
   Element* GetTarget() const {
     // Currently we only implement Element.getAnimations() which only
     // returns animations targetting Elements so this should never
@@ -231,6 +226,10 @@ public:
                " pseudo-element is not yet supported.");
     return mTarget;
   }
+  void GetName(nsString& aRetVal) const
+  {
+    aRetVal = Name();
+  }
 
   // Temporary workaround to return both the target element and pseudo-type
   // until we implement PseudoElement.
@@ -239,6 +238,12 @@ public:
     aTarget = mTarget;
     aPseudoType = mPseudoType;
   }
+  // Alternative to GetName that returns a reference to the member for
+  // more efficient internal usage.
+  virtual const nsString& Name() const
+  {
+    return mName;
+  }
 
   void SetParentTime(Nullable<TimeDuration> aParentTime);
 
@@ -249,10 +254,6 @@ public:
     return mTiming;
   }
 
-  virtual const nsString& Name() const {
-    return mName;
-  }
-
   // Return the duration from the start the active interval to the point where
   // the animation begins playback. This is zero unless the animation has
   // a negative delay in which case it is the absolute value of the delay.

dom/animation/AnimationEffect.cpp

@@ -1,24 +0,0 @@
-/* vim: set shiftwidth=2 tabstop=8 autoindent cindent expandtab: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "mozilla/dom/AnimationEffect.h"
-#include "mozilla/dom/AnimationEffectBinding.h"
-
-namespace mozilla {
-namespace dom {
-
-NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(AnimationEffect, mAnimation)
-
-NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(AnimationEffect, AddRef)
-NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(AnimationEffect, Release)
-
-JSObject*
-AnimationEffect::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
-{
-  return AnimationEffectBinding::Wrap(aCx, this, aGivenProto);
-}
-
-} // namespace dom
-} // namespace mozilla

dom/animation/AnimationEffect.h

@@ -1,46 +0,0 @@
-/* vim: set shiftwidth=2 tabstop=8 autoindent cindent expandtab: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef mozilla_dom_AnimationEffect_h
-#define mozilla_dom_AnimationEffect_h
-
-#include "nsCycleCollectionParticipant.h"
-#include "nsWrapperCache.h"
-#include "mozilla/dom/Animation.h"
-
-struct JSContext;
-
-namespace mozilla {
-namespace dom {
-
-class AnimationEffect final : public nsWrapperCache
-{
-public:
-  explicit AnimationEffect(Animation* aAnimation)
-    : mAnimation(aAnimation)
-  {
-  }
-
-  NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(AnimationEffect)
-  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(AnimationEffect)
-
-  Animation* GetParentObject() const { return mAnimation; }
-  virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
-
-  // AnimationEffect interface
-  void GetName(nsString& aRetVal) const {
-    aRetVal = mAnimation->Name();
-  }
-
-private:
-  ~AnimationEffect() { }
-
-  nsRefPtr<Animation> mAnimation;
-};
-
-} // namespace dom
-} // namespace mozilla
-
-#endif // mozilla_dom_AnimationEffect_h

dom/animation/moz.build

@@ -9,7 +9,6 @@ MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini']
 
 EXPORTS.mozilla.dom += [
     'Animation.h',
-    'AnimationEffect.h',
     'AnimationPlayer.h',
     'AnimationTimeline.h',
     'DocumentTimeline.h',
@@ -22,7 +21,6 @@ EXPORTS.mozilla += [
 
 UNIFIED_SOURCES += [
     'Animation.cpp',
-    'AnimationEffect.cpp',
     'AnimationPlayer.cpp',
     'AnimationTimeline.cpp',
     'DocumentTimeline.cpp',

dom/animation/test/css-animations/test_animation-name.html

@@ -15,22 +15,22 @@
 test(function(t) {
   var div = addDiv(t);
   div.style.animation = 'xyz 100s';
-  assert_equals(div.getAnimations()[0].source.effect.name, 'xyz',
+  assert_equals(div.getAnimations()[0].source.name, 'xyz',
                 'Animation effect name matches keyframes rule name');
 }, 'Effect name makes keyframe rule');
 
 test(function(t) {
   var div = addDiv(t);
   div.style.animation = 'x\\yz 100s';
-  assert_equals(div.getAnimations()[0].source.effect.name, 'xyz',
+  assert_equals(div.getAnimations()[0].source.name, 'xyz',
                 'Escaped animation effect name matches keyframes rule name');
 }, 'Escaped animation name');
 
 test(function(t) {
   var div = addDiv(t);
   div.style.animation = 'x\\79 z 100s';
-  assert_equals(div.getAnimations()[0].source.effect.name, 'xyz',
-                'Hex-escaped animation effect name matches keyframes rule'
+  assert_equals(div.getAnimations()[0].source.name, 'xyz',
+                'Hex-escaped animation name matches keyframes rule'
                 + ' name');
 }, 'Animation name with hex-escape');
 

dom/animation/test/css-transitions/test_animation-name.html

@@ -16,7 +16,7 @@ test(function(t) {
   div.style.transition = 'all 100s';
   div.style.left = '100px';
 
-  assert_equals(div.getAnimations()[0].source.effect.name, 'left',
+  assert_equals(div.getAnimations()[0].source.name, 'left',
                 'The name for the transitions corresponds to the property ' +
                 'being transitioned');
 }, 'Effect name for transitions');

dom/animation/test/mochitest.ini

@@ -3,7 +3,7 @@ support-files =
   testcommon.js
 
 [css-animations/test_animations-dynamic-changes.html]
-[css-animations/test_animation-effect-name.html]
+[css-animations/test_animation-name.html]
 [css-animations/test_animation-pausing.html]
 [css-animations/test_animation-player-currenttime.html]
 [css-animations/test_animation-player-finished.html]
@@ -13,7 +13,7 @@ support-files =
 [css-animations/test_animation-target.html]
 [css-animations/test_element-get-animation-players.html]
 skip-if = buildapp == 'mulet'
-[css-transitions/test_animation-effect-name.html]
+[css-transitions/test_animation-name.html]
 [css-transitions/test_animation-pausing.html]
 [css-transitions/test_animation-player-currenttime.html]
 [css-transitions/test_animation-player-ready.html]

dom/tests/mochitest/general/test_interfaces.html

@@ -133,8 +133,6 @@ var interfaceNamesInGlobalScope =
     "AnimationEvent",
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "Animation", pref: "dom.animations-api.core.enabled"},
-// IMPORTANT: Do not change this list without review from a DOM peer!
-    {name: "AnimationEffect", pref: "dom.animations-api.core.enabled"},
 // IMPORTANT: Do not change this list without review from a DOM peer!
     {name: "AnimationPlayer", pref: "dom.animations-api.core.enabled"},
 // IMPORTANT: Do not change this list without review from a DOM peer!

dom/webidl/Animation.webidl

@@ -12,10 +12,7 @@
 
 [Func="nsDocument::IsWebAnimationsEnabled"]
 interface Animation {
-  // FIXME: |effect| should have type (AnimationEffect or EffectCallback)?
-  // but we haven't implemented EffectCallback yet.
-  [Cached,Pure]
-  readonly attribute AnimationEffect? effect;
   // FIXME: This should be writeable (bug 1067769)
-  readonly attribute Element?         target;
+  readonly attribute Element?  target;
+  readonly attribute DOMString name;
 };

dom/webidl/AnimationEffect.webidl

@@ -1,16 +0,0 @@
-/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/.
- *
- * The origin of this IDL file is
- * http://dev.w3.org/fxtf/web-animations/#the-animationeffect-interface
- *
- * Copyright © 2014 W3C® (MIT, ERCIM, Keio), All Rights Reserved. W3C
- * liability, trademark and document use rules apply.
- */
-
-[Func="nsDocument::IsWebAnimationsEnabled"]
-interface AnimationEffect {
-  readonly attribute DOMString name;
-};

dom/webidl/moz.build

@@ -23,7 +23,6 @@ WEBIDL_FILES = [
     'AnalyserNode.webidl',
     'Animatable.webidl',
     'Animation.webidl',
-    'AnimationEffect.webidl',
     'AnimationEvent.webidl',
     'AnimationPlayer.webidl',
     'AnimationTimeline.webidl',

layout/style/nsTransitionManager.h

@@ -42,7 +42,7 @@ struct ElementPropertyTransition : public dom::Animation
   virtual ElementPropertyTransition* AsTransition() { return this; }
   virtual const ElementPropertyTransition* AsTransition() const { return this; }
 
-  virtual const nsString& Name() const;
+  virtual const nsString& Name() const override;
 
   nsCSSProperty TransitionProperty() const {
     MOZ_ASSERT(Properties().Length() == 1,

toolkit/devtools/server/actors/animation.js

@@ -115,7 +115,7 @@ let AnimationPlayerActor = ActorClass({
     // the list.
     names = names.split(",").map(n => n.trim());
     for (let i = 0; i < names.length; i ++) {
-      if (names[i] === this.player.source.effect.name) {
+      if (names[i] === this.player.source.name) {
         return i;
       }
     }
@@ -207,7 +207,7 @@ let AnimationPlayerActor = ActorClass({
       currentTime: this.player.currentTime,
       playState: this.player.playState,
       playbackRate: this.player.playbackRate,
-      name: this.player.source.effect.name,
+      name: this.player.source.name,
       duration: this.getDuration(),
       delay: this.getDelay(),
       iterationCount: this.getIterationCount(),