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

--HG--
extra : rebase_source : 577bca1e551e39fecfab309f64c993eba110337f

dom/animation/AnimationUtils.cpp

@@ -64,27 +64,6 @@ AnimationUtils::IsOffscreenThrottlingEnabled()
   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)

dom/animation/AnimationUtils.h

@@ -67,18 +67,6 @@ public:
   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.

dom/animation/KeyframeEffect.cpp

@@ -10,6 +10,7 @@
   // 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"
 
@@ -138,7 +139,7 @@ KeyframeEffect::SetIterationComposite(
 {
   // 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;
   }
 

dom/animation/KeyframeEffectReadOnly.cpp

@@ -31,6 +31,7 @@
 #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"
@@ -829,7 +830,7 @@ KeyframeEffectParamsFromUnion(const OptionsType& aOptions,
   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;
   }
 

dom/animation/KeyframeUtils.cpp

@@ -6,7 +6,6 @@
 
 #include "mozilla/KeyframeUtils.h"
 
-#include "mozilla/AnimationUtils.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/Move.h"
 #include "mozilla/RangedArray.h"
@@ -20,11 +19,13 @@
 #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"
@@ -438,7 +439,7 @@ KeyframeUtils::GetKeyframesFromObject(JSContext* aCx,
     return keyframes;
   }
 
-  if (!AnimationUtils::IsCoreAPIEnabled() &&
+  if (!nsDocument::IsWebAnimationsEnabled(aCx, nullptr) &&
       RequiresAdditiveAnimation(keyframes, aDocument)) {
     keyframes.Clear();
     aRv.Throw(NS_ERROR_DOM_ANIM_MISSING_PROPS_ERR);
@@ -1165,7 +1166,7 @@ HandleMissingInitialKeyframe(nsTArray<AnimationProperty>& aResult,
   // 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;
   }
 
@@ -1188,7 +1189,7 @@ HandleMissingFinalKeyframe(nsTArray<AnimationProperty>& aResult,
   // 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) {
@@ -1432,8 +1433,7 @@ GetKeyframeListFromPropertyIndexedKeyframe(JSContext* aCx,
     // 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;
     }

dom/base/nsDocument.cpp

@@ -3502,6 +3502,15 @@ nsDocument::IsWebAnimationsEnabled(JSContext* aCx, JSObject* /*unused*/)
          nsContentUtils::AnimationsAPICoreEnabled();
 }
 
+bool
+nsDocument::IsWebAnimationsEnabled(CallerType aCallerType)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  return aCallerType == dom::CallerType::System ||
+         nsContentUtils::AnimationsAPICoreEnabled();
+}
+
 DocumentTimeline*
 nsDocument::Timeline()
 {

dom/base/nsDocument.h

@@ -462,6 +462,7 @@ public:
 
   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;