Bug 1418220 - Drop AnimationUtils::IsCoreAPIEnabled(ForCaller) and use nsContentUtils::AnimationsAPICoreEnabled / nsDocument::IsWebAnimationsEnabled instead; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Mon, 20 Nov 2017 14:18:43 +0900
changeset 700436 f9ec3deb7e924f3f7081fdcbc264af38d28b8f1d
parent 700338 dd08f8b19cc32da161811abb2f7093e0f5392e69
child 740865 af0adbc3f28848b4bbdac9687ae57edb015ccbc1
push id89827
push userbbirtles@mozilla.com
push dateMon, 20 Nov 2017 05:45:28 +0000
reviewershiro
bugs1418220
milestone59.0a1
Bug 1418220 - Drop AnimationUtils::IsCoreAPIEnabled(ForCaller) and use nsContentUtils::AnimationsAPICoreEnabled / nsDocument::IsWebAnimationsEnabled instead; r?hiro The difference between nsDocument::IsWebAnimationsEnabled and nsContentUtils::AnimationsAPICoreEnabled is that the former checks the caller type and treats the preference as set for system callers which is particularly needed for enabling things like the getProperties() API for DevTools etc. Generally in API-facing call sites we have a JS context / CallerType and so we want to distinguish between system callers and non-system callers. However, for a few internal uses--specifically filling-in missing keyframes--we don't care about the caller type and always follow the pref setting. That may or not be quite what we want, but this patch doesn't change that except for one call site: KeyframeUtils::GetKeyframesFromObject. This patch changes GetKeyframesFromObject from *not* checking the caller type to checking the caller type. That seems to be the correct behavior here since this is called from KeyframeEffectReadOnly::SetKeyframes(JSContext*, JS::Handle<JSObject*>, ErrorResult&) (i.e. a JS API-facing call site) where we *should* enable the full API when the caller is chrome code. MozReview-Commit-ID: FQJBk3zytwd
dom/animation/AnimationUtils.cpp
dom/animation/AnimationUtils.h
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeUtils.cpp
dom/base/nsDocument.cpp
dom/base/nsDocument.h
--- a/dom/animation/AnimationUtils.cpp
+++ b/dom/animation/AnimationUtils.cpp
@@ -60,37 +60,16 @@ AnimationUtils::IsOffscreenThrottlingEna
     Preferences::AddBoolVarCache(&sOffscreenThrottlingEnabled,
                                  "dom.animations.offscreen-throttling");
   }
 
   return sOffscreenThrottlingEnabled;
 }
 
 /* static */ bool
-AnimationUtils::IsCoreAPIEnabled()
-{
-  static bool sCoreAPIEnabled;
-  static bool sPrefCached = false;
-
-  if (!sPrefCached) {
-    sPrefCached = true;
-    Preferences::AddBoolVarCache(&sCoreAPIEnabled,
-                                 "dom.animations-api.core.enabled");
-  }
-
-  return sCoreAPIEnabled;
-}
-
-/* static */ bool
-AnimationUtils::IsCoreAPIEnabledForCaller(dom::CallerType aCallerType)
-{
-  return IsCoreAPIEnabled() || aCallerType == dom::CallerType::System;
-}
-
-/* static */ bool
 AnimationUtils::EffectSetContainsAnimatedScale(EffectSet& aEffects,
                                                const nsIFrame* aFrame)
 {
   for (const dom::KeyframeEffectReadOnly* effect : aEffects) {
     if (effect->ContainsAnimatedScale(aFrame)) {
       return true;
     }
   }
--- a/dom/animation/AnimationUtils.h
+++ b/dom/animation/AnimationUtils.h
@@ -63,28 +63,16 @@ public:
 
   /**
    * Checks if offscreen animation throttling is enabled.
    */
   static bool
   IsOffscreenThrottlingEnabled();
 
   /**
-   * Returns true if the preference to enable the core Web Animations API is
-   * true.
-   */
-  static bool IsCoreAPIEnabled();
-
-  /**
-   * Returns true if the preference to enable the core Web Animations API is
-   * true or the caller is chrome.
-   */
-  static bool IsCoreAPIEnabledForCaller(dom::CallerType aCallerType);
-
-  /**
    * Returns true if the given EffectSet contains a current effect that animates
    * scale. |aFrame| is used for calculation of scale values.
    */
   static bool EffectSetContainsAnimatedScale(EffectSet& aEffects,
                                              const nsIFrame* aFrame);
 };
 
 } // namespace mozilla
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -5,16 +5,17 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/KeyframeEffect.h"
 
 #include "mozilla/dom/KeyframeAnimationOptionsBinding.h"
   // For UnrestrictedDoubleOrKeyframeAnimationOptions
 #include "mozilla/dom/AnimationEffectTiming.h"
 #include "mozilla/dom/KeyframeEffectBinding.h"
+#include "nsDocument.h" // For nsDocument::IsWebAnimationsEnabled
 #include "nsDOMMutationObserver.h" // For nsAutoAnimationMutationBatch
 #include "nsStyleContext.h"
 
 namespace mozilla {
 namespace dom {
 
 KeyframeEffect::KeyframeEffect(nsIDocument* aDocument,
                                const Maybe<OwningAnimationTarget>& aTarget,
@@ -133,17 +134,17 @@ KeyframeEffect::SetTarget(const Nullable
 
 void
 KeyframeEffect::SetIterationComposite(
   const IterationCompositeOperation& aIterationComposite,
   CallerType aCallerType)
 {
   // Ignore iterationComposite if the Web Animations API is not enabled,
   // then the default value 'Replace' will be used.
-  if (!AnimationUtils::IsCoreAPIEnabledForCaller(aCallerType)) {
+  if (!nsDocument::IsWebAnimationsEnabled(aCallerType)) {
     return;
   }
 
   if (mEffectOptions.mIterationComposite == aIterationComposite) {
     return;
   }
 
   if (mAnimation && mAnimation->IsRelevant()) {
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -26,16 +26,17 @@
 #include "mozilla/Telemetry.h"
 #include "mozilla/TypeTraits.h"
 #include "Layers.h" // For Layer
 #include "nsComputedDOMStyle.h" // nsComputedDOMStyle::GetStyleContext
 #include "nsContentUtils.h"
 #include "nsCSSPropertyIDSet.h"
 #include "nsCSSProps.h" // For nsCSSProps::PropHasFlags
 #include "nsCSSPseudoElements.h" // For CSSPseudoElementType
+#include "nsDocument.h" // For nsDocument::IsWebAnimationsEnabled
 #include "nsIFrame.h"
 #include "nsIPresShell.h"
 #include "nsIScriptError.h"
 #include "nsRefreshDriver.h"
 #include "nsStyleContextInlines.h"
 
 namespace mozilla {
 
@@ -824,17 +825,17 @@ template <class OptionsType>
 static KeyframeEffectParams
 KeyframeEffectParamsFromUnion(const OptionsType& aOptions,
                               CallerType aCallerType)
 {
   KeyframeEffectParams result;
   if (aOptions.IsUnrestrictedDouble() ||
       // Ignore iterationComposite if the Web Animations API is not enabled,
       // then the default value 'Replace' will be used.
-      !AnimationUtils::IsCoreAPIEnabledForCaller(aCallerType)) {
+      !nsDocument::IsWebAnimationsEnabled(aCallerType)) {
     return result;
   }
 
   const KeyframeEffectOptions& options =
     KeyframeEffectOptionsFromUnion(aOptions);
   result.mIterationComposite = options.mIterationComposite;
   result.mComposite = options.mComposite;
   return result;
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -1,35 +1,36 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/KeyframeUtils.h"
 
-#include "mozilla/AnimationUtils.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/Move.h"
 #include "mozilla/RangedArray.h"
 #include "mozilla/ServoBindings.h"
 #include "mozilla/ServoBindingTypes.h"
 #include "mozilla/StyleAnimationValue.h"
 #include "mozilla/TimingParams.h"
 #include "mozilla/dom/BaseKeyframeTypesBinding.h" // For FastBaseKeyframe etc.
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/KeyframeEffectBinding.h"
 #include "mozilla/dom/KeyframeEffectReadOnly.h" // For PropertyValuesPair etc.
 #include "jsapi.h" // For ForOfIterator etc.
 #include "nsClassHashtable.h"
-#include "nsContentUtils.h" // For GetContextForContent
+#include "nsContentUtils.h" // For GetContextForContent, and
+                            // AnimationsAPICoreEnabled
 #include "nsCSSParser.h"
 #include "nsCSSPropertyIDSet.h"
 #include "nsCSSProps.h"
 #include "nsCSSPseudoElements.h" // For CSSPseudoElementType
+#include "nsDocument.h" // For nsDocument::IsWebAnimationsEnabled
 #include "nsIScriptError.h"
 #include "nsStyleContext.h"
 #include "nsTArray.h"
 #include <algorithm> // For std::stable_sort, std::min
 
 namespace mozilla {
 
 // ------------------------------------------------------------------
@@ -433,17 +434,17 @@ KeyframeUtils::GetKeyframesFromObject(JS
   }
 
   if (aRv.Failed()) {
     MOZ_ASSERT(keyframes.IsEmpty(),
                "Should not set any keyframes when there is an error");
     return keyframes;
   }
 
-  if (!AnimationUtils::IsCoreAPIEnabled() &&
+  if (!nsDocument::IsWebAnimationsEnabled(aCx, nullptr) &&
       RequiresAdditiveAnimation(keyframes, aDocument)) {
     keyframes.Clear();
     aRv.Throw(NS_ERROR_DOM_ANIM_MISSING_PROPS_ERR);
   }
 
   return keyframes;
 }
 
@@ -1160,17 +1161,17 @@ HandleMissingInitialKeyframe(nsTArray<An
                              const KeyframeValueEntry& aEntry)
 {
   MOZ_ASSERT(aEntry.mOffset != 0.0f,
              "The offset of the entry should not be 0.0");
 
   // If the preference of the core Web Animations API is not enabled, don't fill
   // in the missing keyframe since the missing keyframe requires support for
   // additive animation which is guarded by this pref.
-  if (!AnimationUtils::IsCoreAPIEnabled()){
+  if (!nsContentUtils::AnimationsAPICoreEnabled()) {
     return nullptr;
   }
 
   AnimationProperty* result = aResult.AppendElement();
   result->mProperty = aEntry.mProperty;
 
   AppendInitialSegment(result, aEntry);
 
@@ -1183,17 +1184,17 @@ HandleMissingFinalKeyframe(nsTArray<Anim
                            AnimationProperty* aCurrentAnimationProperty)
 {
   MOZ_ASSERT(aEntry.mOffset != 1.0f,
              "The offset of the entry should not be 1.0");
 
   // If the preference of the core Web Animations API is not enabled, don't fill
   // in the missing keyframe since the missing keyframe requires support for
   // additive animation which is guarded by this pref.
-  if (!AnimationUtils::IsCoreAPIEnabled()){
+  if (!nsContentUtils::AnimationsAPICoreEnabled()) {
     // If we have already appended a new entry for the property so we have to
     // remove it.
     if (aCurrentAnimationProperty) {
       aResult.RemoveElementAt(aResult.Length() - 1);
     }
     return;
   }
 
@@ -1427,18 +1428,17 @@ GetKeyframeListFromPropertyIndexedKeyfra
     if (count == 0) {
       // No animation values for this property.
       continue;
     }
 
     // If we only have one value, we should animate from the underlying value
     // using additive animation--however, we don't support additive animation
     // when the core animation API pref is switched off.
-    if ((!AnimationUtils::IsCoreAPIEnabled()) &&
-        count == 1) {
+    if (!nsContentUtils::AnimationsAPICoreEnabled() && count == 1) {
       aRv.Throw(NS_ERROR_DOM_ANIM_MISSING_PROPS_ERR);
       return;
     }
 
     size_t n = pair.mValues.Length() - 1;
     size_t i = 0;
 
     for (const nsString& stringValue : pair.mValues) {
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -3497,16 +3497,25 @@ bool
 nsDocument::IsWebAnimationsEnabled(JSContext* aCx, JSObject* /*unused*/)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   return nsContentUtils::IsSystemCaller(aCx) ||
          nsContentUtils::AnimationsAPICoreEnabled();
 }
 
+bool
+nsDocument::IsWebAnimationsEnabled(CallerType aCallerType)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  return aCallerType == dom::CallerType::System ||
+         nsContentUtils::AnimationsAPICoreEnabled();
+}
+
 DocumentTimeline*
 nsDocument::Timeline()
 {
   if (!mDocumentTimeline) {
     mDocumentTimeline = new DocumentTimeline(this, TimeDuration(0));
   }
 
   return mDocumentTimeline;
--- a/dom/base/nsDocument.h
+++ b/dom/base/nsDocument.h
@@ -457,16 +457,17 @@ public:
                                              mozilla::StyleSetHandle aStyleSet)
     final;
   virtual void DeleteShell() override;
 
   virtual bool GetAllowPlugins() override;
 
   static bool IsElementAnimateEnabled(JSContext* aCx, JSObject* aObject);
   static bool IsWebAnimationsEnabled(JSContext* aCx, JSObject* aObject);
+  static bool IsWebAnimationsEnabled(mozilla::dom::CallerType aCallerType);
   virtual mozilla::dom::DocumentTimeline* Timeline() override;
   virtual void GetAnimations(
       nsTArray<RefPtr<mozilla::dom::Animation>>& aAnimations) override;
   mozilla::LinkedList<mozilla::dom::DocumentTimeline>& Timelines() override
   {
     return mTimelines;
   }