Backed out changeset c3f16a179c93 (bug 1414674)
authorTooru Fujisawa <arai_a@mac.com>
Sun, 18 Feb 2018 01:24:08 +0900
changeset 456792 bb75a5c7f4cc69ae9b9fd520d6028c69a25f2c32
parent 456786 48f24f31126e921105177d9e8d1d9b35dfb7d09b
child 456793 7742669c40dbb2b66970981e95577aabae244702
push id8799
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 16:46:23 +0000
treeherdermozilla-beta@15334014dc67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1414674
milestone60.0a1
backs outc3f16a179c93d9b75dd4581280967fa6ef364cc3
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset c3f16a179c93 (bug 1414674)
browser/components/extensions/test/browser/browser-common.ini
browser/components/extensions/test/browser/browser_ext_contentscript_animate.js
dom/animation/AnimationUtils.cpp
dom/animation/AnimationUtils.h
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/test/chrome.ini
dom/animation/test/chrome/file_animate_xrays.html
dom/animation/test/chrome/test_animate_xrays.html
dom/animation/test/chrome/test_keyframe_effect_xrays.html
dom/base/Element.cpp
dom/bindings/Codegen.py
dom/bindings/parser/WebIDL.py
dom/webidl/KeyframeEffect.webidl
--- a/browser/components/extensions/test/browser/browser-common.ini
+++ b/browser/components/extensions/test/browser/browser-common.ini
@@ -1,11 +1,9 @@
 [DEFAULT]
-prefs =
-    dom.animations-api.core.enabled=true
 support-files =
   head.js
   head_pageAction.js
   head_sessions.js
   head_webNavigation.js
   profilerSymbols.sjs
   context.html
   context_frame.html
@@ -207,9 +205,8 @@ tags = fullscreen
 [browser_ext_windows_create_params.js]
 [browser_ext_windows_create_tabId.js]
 [browser_ext_windows_create_url.js]
 [browser_ext_windows_events.js]
 [browser_ext_windows_size.js]
 skip-if = os == 'mac' # Fails when windows are randomly opened in fullscreen mode
 [browser_ext_windows_update.js]
 tags = fullscreen
-[browser_ext_contentscript_animate.js]
deleted file mode 100644
--- a/browser/components/extensions/test/browser/browser_ext_contentscript_animate.js
+++ /dev/null
@@ -1,95 +0,0 @@
-/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim: set sts=2 sw=2 et tw=80: */
-"use strict";
-
-add_task(async function test_animate() {
-  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/");
-
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      "content_scripts": [
-        {
-          "matches": ["http://mochi.test/*"],
-          "js": ["content-script.js"],
-        },
-      ],
-    },
-
-    files: {
-      "content-script.js": function() {
-        let elem = document.getElementsByTagName("body")[0];
-        elem.style.border = "2px solid red";
-
-        let anim = elem.animate({opacity: [1, 0]}, 2000);
-        let frames = anim.effect.getKeyframes();
-        browser.test.assertEq(frames.length, 2,
-                              "frames for Element.animate should be non-zero");
-        browser.test.assertEq(frames[0].opacity, "1",
-                              "first frame opacity for Element.animate should be specified value");
-        browser.test.assertEq(frames[0].computedOffset, 0,
-                              "first frame offset for Element.animate should be 0");
-        browser.test.assertEq(frames[1].opacity, "0",
-                              "last frame opacity for Element.animate should be specified value");
-        browser.test.assertEq(frames[1].computedOffset, 1,
-                              "last frame offset for Element.animate should be 1");
-
-        browser.test.notifyPass("contentScriptAnimate");
-      },
-    },
-  });
-
-  await extension.startup();
-  await extension.awaitFinish("contentScriptAnimate");
-  await extension.unload();
-
-  await BrowserTestUtils.removeTab(tab);
-});
-
-add_task(async function test_KeyframeEffect() {
-  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/");
-
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      "content_scripts": [
-        {
-          "matches": ["http://mochi.test/*"],
-          "js": ["content-script.js"],
-        },
-      ],
-    },
-
-    files: {
-      "content-script.js": function() {
-        let elem = document.getElementsByTagName("body")[0];
-        elem.style.border = "2px solid red";
-
-        let effect = new KeyframeEffect(elem, [
-          {opacity: 1, offset: 0},
-          {opacity: 0, offset: 1},
-        ], {duration: 1000, fill: "forwards"});
-        let frames = effect.getKeyframes();
-        browser.test.assertEq(frames.length, 2,
-                              "frames for KeyframeEffect ctor should be non-zero");
-        browser.test.assertEq(frames[0].opacity, "1",
-                              "first frame opacity for KeyframeEffect ctor should be specified value");
-        browser.test.assertEq(frames[0].computedOffset, 0,
-                              "first frame offset for KeyframeEffect ctor should be 0");
-        browser.test.assertEq(frames[1].opacity, "0",
-                              "last frame opacity for KeyframeEffect ctor should be specified value");
-        browser.test.assertEq(frames[1].computedOffset, 1,
-                              "last frame offset for KeyframeEffect ctor should be 1");
-
-        let animation = new Animation(effect, document.timeline);
-        animation.play();
-
-        browser.test.notifyPass("contentScriptKeyframeEffect");
-      },
-    },
-  });
-
-  await extension.startup();
-  await extension.awaitFinish("contentScriptKeyframeEffect");
-  await extension.unload();
-
-  await BrowserTestUtils.removeTab(tab);
-});
--- a/dom/animation/AnimationUtils.cpp
+++ b/dom/animation/AnimationUtils.cpp
@@ -44,26 +44,16 @@ AnimationUtils::GetCurrentRealmDocument(
 {
   nsGlobalWindowInner* win = xpc::CurrentWindowOrNull(aCx);
   if (!win) {
     return nullptr;
   }
   return win->GetDoc();
 }
 
-/* static */ nsIDocument*
-AnimationUtils::GetDocumentFromGlobal(JSObject* aGlobalObject)
-{
-  nsGlobalWindowInner* win = xpc::WindowOrNull(aGlobalObject);
-  if (!win) {
-    return nullptr;
-  }
-  return win->GetDoc();
-}
-
 /* static */ bool
 AnimationUtils::IsOffscreenThrottlingEnabled()
 {
   static bool sOffscreenThrottlingEnabled;
   static bool sPrefCached = false;
 
   if (!sPrefCached) {
     sPrefCached = true;
--- a/dom/animation/AnimationUtils.h
+++ b/dom/animation/AnimationUtils.h
@@ -57,24 +57,16 @@ public:
 
   /**
    * Get the document from the JS context to use when parsing CSS properties.
    */
   static nsIDocument*
   GetCurrentRealmDocument(JSContext* aCx);
 
   /**
-   * Get the document from the global object, or nullptr if the document has
-   * no window, to use when constructing DOM object without entering the
-   * target window's compartment (see KeyframeEffect constructor).
-   */
-  static nsIDocument*
-  GetDocumentFromGlobal(JSObject* aGlobalObject);
-
-  /**
    * Checks if offscreen animation throttling is enabled.
    */
   static bool
   IsOffscreenThrottlingEnabled();
 
   /**
    * Returns true if the given EffectSet contains a current effect that animates
    * scale. |aFrame| is used for calculation of scale values.
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -898,27 +898,17 @@ template <class KeyframeEffectType, clas
 /* static */ already_AddRefed<KeyframeEffectType>
 KeyframeEffectReadOnly::ConstructKeyframeEffect(
     const GlobalObject& aGlobal,
     const Nullable<ElementOrCSSPseudoElement>& aTarget,
     JS::Handle<JSObject*> aKeyframes,
     const OptionsType& aOptions,
     ErrorResult& aRv)
 {
-  // We should get the document from `aGlobal` instead of the current Realm
-  // to make this works in Xray case.
-  //
-  // In all non-Xray cases, `aGlobal` matches the current Realm, so this
-  // matches the spec behavior.
-  //
-  // In Xray case, the new objects should be created using the document of
-  // the target global, but KeyframeEffect and KeyframeEffectReadOnly
-  // constructors are called in the caller's compartment to access
-  // `aKeyframes` object.
-  nsIDocument* doc = AnimationUtils::GetDocumentFromGlobal(aGlobal.Get());
+  nsIDocument* doc = AnimationUtils::GetCurrentRealmDocument(aGlobal.Context());
   if (!doc) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   TimingParams timingParams =
     TimingParams::FromOptionsUnion(aOptions, doc, aRv);
   if (aRv.Failed()) {
--- a/dom/animation/test/chrome.ini
+++ b/dom/animation/test/chrome.ini
@@ -3,17 +3,16 @@ support-files =
   testcommon.js
   ../../imptests/testharness.js
   ../../imptests/testharnessreport.js
   !/dom/animation/test/chrome/file_animate_xrays.html
 
 [chrome/test_animate_xrays.html]
 # file_animate_xrays.html needs to go in mochitest.ini since it is served
 # over HTTP
-[chrome/test_keyframe_effect_xrays.html]
 [chrome/test_animation_observers_async.html]
 [chrome/test_animation_observers_sync.html]
 [chrome/test_animation_performance_warning.html]
 [chrome/test_animation_properties.html]
 [chrome/test_cssanimation_missing_keyframes.html]
 [chrome/test_generated_content_getAnimations.html]
 [chrome/test_running_on_compositor.html]
 [chrome/test_simulate_compute_values_failure.html]
--- a/dom/animation/test/chrome/file_animate_xrays.html
+++ b/dom/animation/test/chrome/file_animate_xrays.html
@@ -1,18 +1,19 @@
 <!doctype html>
 <html>
 <head>
 <meta charset=utf-8>
 <script>
 Element.prototype.animate = function() {
   throw 'Called animate() as defined in content document';
 }
-for (let name of ["KeyframeEffect", "KeyframeEffectReadOnly", "Animation"]) {
-  this[name] = function() {
-    throw `Called overridden ${name} constructor`;
+// Bug 1211783: Use KeyframeEffect (not KeyframeEffectReadOnly) here
+for (var obj of [KeyframeEffectReadOnly, Animation]) {
+  obj = function() {
+    throw 'Called overridden ' + String(obj) + ' constructor';
   };
 }
 </script>
 <body>
 <div id="target"></div>
 </body>
 </html>
--- a/dom/animation/test/chrome/test_animate_xrays.html
+++ b/dom/animation/test/chrome/test_animate_xrays.html
@@ -1,40 +1,31 @@
 <!doctype html>
 <head>
 <meta charset=utf-8>
 <script type="application/javascript" src="../testharness.js"></script>
 <script type="application/javascript" src="../testharnessreport.js"></script>
 <script type="application/javascript" src="../testcommon.js"></script>
 </head>
 <body>
-<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1414674"
-  target="_blank">Mozilla Bug 1414674</a>
+<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1045994"
+  target="_blank">Mozilla Bug 1045994</a>
 <div id="log"></div>
 <iframe id="iframe"
   src="http://example.org/tests/dom/animation/test/chrome/file_animate_xrays.html"></iframe>
 <script>
 'use strict';
 
 var win = document.getElementById('iframe').contentWindow;
 
 async_test(function(t) {
   window.addEventListener('load', t.step_func(function() {
     var target = win.document.getElementById('target');
-    var anim = target.animate({opacity: [ 1, 0 ]}, 100 * MS_PER_SEC);
-    // The frames object should be accessible via x-ray.
-    var frames = anim.effect.getKeyframes();
-    assert_equals(frames.length, 2,
-                  "frames for Element.animate should be non-zero");
-    assert_equals(frames[0].opacity, "1",
-                  "first frame opacity for Element.animate should be specified value");
-    assert_equals(frames[0].computedOffset, 0,
-                  "first frame offset for Element.animate should be 0");
-    assert_equals(frames[1].opacity, "0",
-                  "last frame opacity for Element.animate should be specified value");
-    assert_equals(frames[1].computedOffset, 1,
-                  "last frame offset for Element.animate should be 1");
+    var anim = target.animate({ opacity: [ 0, 1 ] }, 100 * MS_PER_SEC);
+    // In the x-ray case, the frames object will be given an opaque wrapper
+    // so it won't be possible to fetch any frames from it.
+    assert_equals(anim.effect.getKeyframes().length, 0);
     t.done();
   }));
 }, 'Calling animate() across x-rays');
 
 </script>
 </body>
deleted file mode 100644
--- a/dom/animation/test/chrome/test_keyframe_effect_xrays.html
+++ /dev/null
@@ -1,45 +0,0 @@
-<!doctype html>
-<head>
-<meta charset=utf-8>
-<script type="application/javascript" src="../testharness.js"></script>
-<script type="application/javascript" src="../testharnessreport.js"></script>
-<script type="application/javascript" src="../testcommon.js"></script>
-</head>
-<body>
-<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1414674"
-  target="_blank">Mozilla Bug 1414674</a>
-<div id="log"></div>
-<iframe id="iframe"
-  src="http://example.org/tests/dom/animation/test/chrome/file_animate_xrays.html"></iframe>
-<script>
-'use strict';
-
-var win = document.getElementById('iframe').contentWindow;
-
-async_test(function(t) {
-  window.addEventListener('load', t.step_func(function() {
-    var target = win.document.getElementById('target');
-    var effect = new win.KeyframeEffect(target, [
-      {opacity: 1, offset: 0},
-      {opacity: 0, offset: 1},
-    ], {duration: 100 * MS_PER_SEC, fill: "forwards"});
-    // The frames object should be accessible via x-ray.
-    var frames = effect.getKeyframes();
-    assert_equals(frames.length, 2,
-                  "frames for KeyframeEffect ctor should be non-zero");
-    assert_equals(frames[0].opacity, "1",
-                  "first frame opacity for KeyframeEffect ctor should be specified value");
-    assert_equals(frames[0].computedOffset, 0,
-                  "first frame offset for KeyframeEffect ctor should be 0");
-    assert_equals(frames[1].opacity, "0",
-                  "last frame opacity for KeyframeEffect ctor should be specified value");
-    assert_equals(frames[1].computedOffset, 1,
-                  "last frame offset for KeyframeEffect ctor should be 1");
-    var animation = new win.Animation(effect, document.timeline);
-    animation.play();
-    t.done();
-  }));
-}, 'Calling KeyframeEffect() ctor across x-rays');
-
-</script>
-</body>
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -3799,33 +3799,32 @@ Element::Animate(const Nullable<ElementO
   nsCOMPtr<nsIGlobalObject> ownerGlobal = referenceElement->GetOwnerGlobal();
   if (!ownerGlobal) {
     aError.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
   GlobalObject global(aContext, ownerGlobal->GetGlobalJSObject());
   MOZ_ASSERT(!global.Failed());
 
-  // KeyframeEffect constructor doesn't follow the standard Xray calling
-  // convention and needs to be called in caller's compartment.
-  // This should match to RunConstructorInCallerCompartment attribute in
-  // KeyframeEffect.webidl.
-  RefPtr<KeyframeEffect> effect =
-    KeyframeEffect::Constructor(global, aTarget, aKeyframes, aOptions,
-                                aError);
-  if (aError.Failed()) {
-    return nullptr;
-  }
-
-  // Animation constructor follows the standard Xray calling convention and
-  // needs to be called in the target element's compartment.
+  // Wrap the aKeyframes object for the cross-compartment case.
+  JS::Rooted<JSObject*> keyframes(aContext);
+  keyframes = aKeyframes;
   Maybe<JSAutoCompartment> ac;
   if (js::GetContextCompartment(aContext) !=
       js::GetObjectCompartment(ownerGlobal->GetGlobalJSObject())) {
     ac.emplace(aContext, ownerGlobal->GetGlobalJSObject());
+    if (!JS_WrapObject(aContext, &keyframes)) {
+      return nullptr;
+    }
+  }
+
+  RefPtr<KeyframeEffect> effect =
+    KeyframeEffect::Constructor(global, aTarget, keyframes, aOptions, aError);
+  if (aError.Failed()) {
+    return nullptr;
   }
 
   AnimationTimeline* timeline = referenceElement->OwnerDoc()->Timeline();
   RefPtr<Animation> animation =
     Animation::Constructor(global, effect,
                            Optional<AnimationTimeline*>(timeline), aError);
   if (aError.Failed()) {
     return nullptr;
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -7801,20 +7801,17 @@ class CGPerSignatureCall(CGThing):
         # since GlobalObject already contains the context.
         needsCx = needCx(returnType, arguments, self.extendedAttributes,
                          not descriptor.interface.isJSImplemented(), static)
         if needsCx:
             argsPre.append("cx")
 
         needsUnwrap = False
         argsPost = []
-        runConstructorInCallerCompartment = \
-            descriptor.interface.getExtendedAttribute(
-                'RunConstructorInCallerCompartment')
-        if isConstructor and not runConstructorInCallerCompartment:
+        if isConstructor:
             needsUnwrap = True
             needsUnwrappedVar = False
             unwrappedVar = "obj"
             if descriptor.interface.isJSImplemented():
                 # We need the desired proto in our constructor, because the
                 # constructor will actually construct our reflector.
                 argsPost.append("desiredProto")
         elif descriptor.interface.isJSImplemented():
--- a/dom/bindings/parser/WebIDL.py
+++ b/dom/bindings/parser/WebIDL.py
@@ -1738,17 +1738,16 @@ class IDLInterface(IDLInterfaceOrNamespa
                     member.addExtendedAttributes([attr])
             elif (identifier == "NeedResolve" or
                   identifier == "OverrideBuiltins" or
                   identifier == "ChromeOnly" or
                   identifier == "Unforgeable" or
                   identifier == "LegacyEventInit" or
                   identifier == "ProbablyShortLivingWrapper" or
                   identifier == "LegacyUnenumerableNamedProperties" or
-                  identifier == "RunConstructorInCallerCompartment" or
                   identifier == "NonOrdinaryGetPrototypeOf"):
                 # Known extended attributes that do not take values
                 if not attr.noArguments():
                     raise WebIDLError("[%s] must take no arguments" % identifier,
                                       [attr.location])
             elif identifier == "Exposed":
                 convertExposedAttrToGlobalNameSet(attr,
                                                   self._exposureGlobalNames)
--- a/dom/webidl/KeyframeEffect.webidl
+++ b/dom/webidl/KeyframeEffect.webidl
@@ -15,20 +15,17 @@ enum IterationCompositeOperation {
   "accumulate"
 };
 
 dictionary KeyframeEffectOptions : AnimationEffectTimingProperties {
   IterationCompositeOperation iterationComposite = "replace";
   CompositeOperation          composite = "replace";
 };
 
-// KeyframeEffectReadOnly should run in the caller's compartment to do custom
-// processing on the `keyframes` object.
 [Func="nsDocument::IsWebAnimationsEnabled",
- RunConstructorInCallerCompartment,
  Constructor ((Element or CSSPseudoElement)? target,
               object? keyframes,
               optional (unrestricted double or KeyframeEffectOptions) options),
  Constructor (KeyframeEffectReadOnly source)]
 interface KeyframeEffectReadOnly : AnimationEffectReadOnly {
   readonly attribute (Element or CSSPseudoElement)?  target;
   readonly attribute IterationCompositeOperation iterationComposite;
   readonly attribute CompositeOperation          composite;
@@ -52,20 +49,17 @@ dictionary AnimationPropertyDetails {
            DOMString                               warning;
   required sequence<AnimationPropertyValueDetails> values;
 };
 
 partial interface KeyframeEffectReadOnly {
   [ChromeOnly, Throws] sequence<AnimationPropertyDetails> getProperties();
 };
 
-// KeyframeEffect should run in the caller's compartment to do custom
-// processing on the `keyframes` object.
 [Func="nsDocument::IsWebAnimationsEnabled",
- RunConstructorInCallerCompartment,
  Constructor ((Element or CSSPseudoElement)? target,
               object? keyframes,
               optional (unrestricted double or KeyframeEffectOptions) options),
  Constructor (KeyframeEffectReadOnly source)]
 interface KeyframeEffect : KeyframeEffectReadOnly {
   inherit attribute (Element or CSSPseudoElement)? target;
   [NeedsCallerType]
   inherit attribute IterationCompositeOperation    iterationComposite;