Backed out 4 changesets (bug 1760222) for causing reftest failures on disable-apz-for-sle-pages.html CLOSED TREE

Backed out changeset 1bf5e1ca3746 (bug 1760222)
Backed out changeset aff6bf37365d (bug 1760222)
Backed out changeset e9b3e3f52aec (bug 1760222)
Backed out changeset f65d2d719277 (bug 1760222)

dom/base/Document.cpp

@@ -1349,6 +1349,7 @@ Document::Document(const char* aContentType)
       mHasDisplayDocument(false),
       mFontFaceSetDirty(true),
       mDidFireDOMContentLoaded(true),
+      mHasScrollLinkedEffect(false),
       mFrameRequestCallbacksScheduled(false),
       mIsTopLevelContentDocument(false),
       mIsContentDocument(false),
@@ -16030,31 +16031,15 @@ FontFaceSet* Document::Fonts() {
   return mFontFaceSet;
 }
 
-void Document::ReportHasScrollLinkedEffect(const TimeStamp& aTimeStamp) {
-  MOZ_ASSERT(!aTimeStamp.IsNull());
-
-  if (!mLastScrollLinkedEffectDetectionTime.IsNull() &&
-      mLastScrollLinkedEffectDetectionTime >= aTimeStamp) {
+void Document::ReportHasScrollLinkedEffect() {
+  if (mHasScrollLinkedEffect) {
+    // We already did this once for this document, don't do it again.
     return;
   }
-
-  if (mLastScrollLinkedEffectDetectionTime.IsNull()) {
-    // Report to console just once.
-    nsContentUtils::ReportToConsole(
-        nsIScriptError::warningFlag, "Async Pan/Zoom"_ns, this,
-        nsContentUtils::eLAYOUT_PROPERTIES, "ScrollLinkedEffectFound3");
-  }
-
-  mLastScrollLinkedEffectDetectionTime = aTimeStamp;
-}
-
-bool Document::HasScrollLinkedEffect() const {
-  if (nsPresContext* pc = GetPresContext()) {
-    return mLastScrollLinkedEffectDetectionTime ==
-           pc->RefreshDriver()->MostRecentRefresh();
-  }
-
-  return false;
+  mHasScrollLinkedEffect = true;
+  nsContentUtils::ReportToConsole(
+      nsIScriptError::warningFlag, "Async Pan/Zoom"_ns, this,
+      nsContentUtils::eLAYOUT_PROPERTIES, "ScrollLinkedEffectFound3");
 }
 
 void Document::SetSHEntryHasUserInteraction(bool aHasInteraction) {

dom/base/Document.h

@@ -3683,8 +3683,8 @@ class Document : public nsINode,
 
   bool HasScriptsBlockedBySandbox();
 
-  void ReportHasScrollLinkedEffect(const TimeStamp& aTimeStamp);
-  bool HasScrollLinkedEffect() const;
+  void ReportHasScrollLinkedEffect();
+  bool HasScrollLinkedEffect() const { return mHasScrollLinkedEffect; }
 
 #ifdef DEBUG
   void AssertDocGroupMatchesKey() const;
@@ -4496,9 +4496,6 @@ class Document : public nsINode,
   // focus has never occurred then mLastFocusTime.IsNull() will be true.
   TimeStamp mLastFocusTime;
 
-  // Last time we found any scroll linked effect in this document.
-  TimeStamp mLastScrollLinkedEffectDetectionTime;
-
   EventStates mDocumentState{NS_DOCUMENT_STATE_LTR_LOCALE};
 
   RefPtr<Promise> mReadyForIdle;
@@ -4657,6 +4654,9 @@ class Document : public nsINode,
   // (e.g. we're not being parsed at all).
   bool mDidFireDOMContentLoaded : 1;
 
+  // True if ReportHasScrollLinkedEffect() has been called.
+  bool mHasScrollLinkedEffect : 1;
+
   // True if we have frame request callbacks scheduled with the refresh driver.
   // This should generally be updated only via
   // UpdateFrameRequestCallbackSchedulingState.

dom/base/nsDOMWindowUtils.cpp

@@ -4736,13 +4736,3 @@ nsDOMWindowUtils::GetSuspendedByBrowsingContextGroup(bool* aResult) {
   *aResult = inner->GetWasSuspendedByGroup();
   return NS_OK;
 }
-
-NS_IMETHODIMP
-nsDOMWindowUtils::GetHasScrollLinkedEffect(bool* aResult) {
-  Document* doc = GetDocument();
-  if (!doc) {
-    return NS_ERROR_FAILURE;
-  }
-  *aResult = doc->HasScrollLinkedEffect();
-  return NS_OK;
-}

dom/interfaces/base/nsIDOMWindowUtils.idl

@@ -2250,14 +2250,6 @@ interface nsIDOMWindowUtils : nsISupports {
 
   // Used for testing to check the suspend status.
   readonly attribute bool suspendedByBrowsingContextGroup;
-
-  // Whether there's any scroll-linked effect in this document. This is only
-  // meaningful after the script in question tried to mutate something in a
-  // scroll event callback until the next refresh driver tick happens.
-  //
-  // See https://firefox-source-docs.mozilla.org/performance/scroll-linked_effects.html
-  // about scroll-linked effects.
-  readonly attribute bool hasScrollLinkedEffect;
 };
 
 [scriptable, uuid(c694e359-7227-4392-a138-33c0cc1f15a6)]

gfx/layers/apz/test/mochitest/helper_scroll_linked_effect_detector.html

@@ -1,125 +0,0 @@
-<!DOCTYPE html>
-<html>
-<meta charset="utf-8">
-<script src="/tests/SimpleTest/SimpleTest.js"></script>
-<script src="/tests/SimpleTest/paint_listener.js"></script>
-<script src="apz_test_utils.js"></script>
-<script src="apz_test_native_event_utils.js"></script>
-<title>ScrollLinkedEffectDetector tests</title>
-<style>
-html, body { margin: 0; }
-body {
-  height: 1000vh;
-}
-#target {
-  position: absolute;
-  height: 800px;
-  background-color: #cc00cc;
-  top: 0;
-  left: 0;
-  right: 0;
-}
-</style>
-<div id="target"></div>
-<script>
-async function test() {
-  let eventTimeStamp;
-  // Setup a no-op scroll event callback.
-  let scrollEventPromise = new Promise(resolve => {
-    window.addEventListener("scroll", () => {
-      eventTimeStamp = document.timeline.currentTime;
-      resolve();
-    }, { once: true });
-  });
-
-  async function promiseScrollAndEvent() {
-    synthesizeNativeWheel(window, 50, 50, 0, -10);
-    await scrollEventPromise;
-    // Wait a rAF to make sure we are outside of the micro tasks for the scroll
-    // event callback so that we can ensure our stack based
-    // ScrollLinkedEffectDetector has been scoped out from the function firing
-    // scroll events.
-    await promiseFrame();
-  }
-
-  await promiseScrollAndEvent();
-  is(eventTimeStamp, document.timeline.currentTime,
-     `We are in same time frame where we got a scroll event at ${eventTimeStamp}`);
-  ok(!SpecialPowers.DOMWindowUtils.hasScrollLinkedEffect,
-     "No scroll-linked effect found yet");
-
-  // Setup a scroll-linked effect callback.
-  scrollEventPromise = new Promise(resolve => {
-    window.addEventListener("scroll", () => {
-      isnot(window.scrollY, 0, "we've already scrolled some amount");
-      target.style.top = window.scrollY + "px";
-      eventTimeStamp = document.timeline.currentTime;
-      resolve();
-    }, { once: true });
-  });
-  await promiseScrollAndEvent();
-  is(eventTimeStamp, document.timeline.currentTime,
-     `We are in same time frame where we got a scroll event at ${eventTimeStamp}`);
-  ok(SpecialPowers.DOMWindowUtils.hasScrollLinkedEffect,
-     "Scroll-linked effect found");
-
-  // A no-op again.
-  scrollEventPromise = new Promise(resolve => {
-    window.addEventListener("scroll", () => {
-      eventTimeStamp = document.timeline.currentTime;
-      resolve();
-    }, { once: true });
-  });
-  await promiseScrollAndEvent();
-  is(eventTimeStamp, document.timeline.currentTime,
-     `We are in same time frame where we got a scroll event at ${eventTimeStamp}`);
-  ok(!SpecialPowers.DOMWindowUtils.hasScrollLinkedEffect,
-     "No scroll-linked effect found");
-
-  // Setup a non-effective scroll-linked effect callback.
-  scrollEventPromise = new Promise(resolve => {
-    window.addEventListener("scroll", () => {
-      target.style.top = getComputedStyle(target).top;
-      eventTimeStamp = document.timeline.currentTime;
-      resolve();
-    }, { once: true });
-  });
-  await promiseScrollAndEvent();
-  is(eventTimeStamp, document.timeline.currentTime,
-     `We are in same time frame where we got a scroll event at ${eventTimeStamp}`);
-  ok(!SpecialPowers.DOMWindowUtils.hasScrollLinkedEffect,
-     "No scroll-linked effect found");
-
-  // Setup a callback to remove the style.
-  scrollEventPromise = new Promise(resolve => {
-    window.addEventListener("scroll", () => {
-      target.style.top = "";
-      eventTimeStamp = document.timeline.currentTime;
-      resolve();
-    }, { once: true });
-  });
-  await promiseScrollAndEvent();
-  is(eventTimeStamp, document.timeline.currentTime,
-     `We are in same time frame where we got a scroll event at ${eventTimeStamp}`);
-  ok(SpecialPowers.DOMWindowUtils.hasScrollLinkedEffect,
-     "Scroll-linked effect found");
-
-  // Setup a no-op callback.
-  scrollEventPromise = new Promise(resolve => {
-    window.addEventListener("scroll", () => {
-      target.style.top = "";
-      eventTimeStamp = document.timeline.currentTime;
-      resolve();
-    }, { once: true });
-  });
-  await promiseScrollAndEvent();
-  is(eventTimeStamp, document.timeline.currentTime,
-     `We are in same time frame where we got a scroll event at ${eventTimeStamp}`);
-  ok(!SpecialPowers.DOMWindowUtils.hasScrollLinkedEffect,
-     "No scroll-linked effect found this time");
-}
-
-waitUntilApzStable()
-.then(test)
-.then(subtestDone, subtestFailed);
-</script>

gfx/layers/apz/test/mochitest/test_group_scroll_linked_effect.html

@@ -8,13 +8,9 @@
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <script type="application/javascript">
 
-var prefs = [
-  ["apz.test.mac.synth_wheel_input", true]
-];
-
 var subtests = [
-  {"file": "helper_scroll_linked_effect_by_wheel.html", prefs},
-  {"file": "helper_scroll_linked_effect_detector.html", prefs}
+  {"file": "helper_scroll_linked_effect_by_wheel.html",
+   "prefs": [["apz.test.mac.synth_wheel_input", true]]},
 ];
 
 if (isApzEnabled()) {

gfx/layers/apz/util/ScrollLinkedEffectDetector.cpp

@@ -25,9 +25,8 @@ void ScrollLinkedEffectDetector::PositioningPropertyMutated() {
   }
 }
 
-ScrollLinkedEffectDetector::ScrollLinkedEffectDetector(
-    dom::Document* aDoc, const TimeStamp& aTimeStamp)
-    : mDocument(aDoc), mTimeStamp(aTimeStamp) {
+ScrollLinkedEffectDetector::ScrollLinkedEffectDetector(dom::Document* aDoc)
+    : mDocument(aDoc) {
   MOZ_ASSERT(NS_IsMainThread());
   sDepth++;
 }
@@ -38,7 +37,7 @@ ScrollLinkedEffectDetector::~ScrollLinkedEffectDetector() {
     // We have exited all (possibly-nested) scroll event dispatches,
     // record whether or not we found an effect, and reset state
     if (sFoundScrollLinkedEffect) {
-      mDocument->ReportHasScrollLinkedEffect(mTimeStamp);
+      mDocument->ReportHasScrollLinkedEffect();
       sFoundScrollLinkedEffect = false;
     }
   }

gfx/layers/apz/util/ScrollLinkedEffectDetector.h

@@ -8,7 +8,6 @@
 #define mozilla_layers_ScrollLinkedEffectDetector_h
 
 #include "mozilla/RefPtr.h"
-#include "mozilla/TimeStamp.h"
 
 namespace mozilla {
 
@@ -34,12 +33,11 @@ class MOZ_STACK_CLASS ScrollLinkedEffectDetector final {
  public:
   static void PositioningPropertyMutated();
 
-  ScrollLinkedEffectDetector(dom::Document*, const TimeStamp& aTimeStamp);
+  explicit ScrollLinkedEffectDetector(dom::Document*);
   ~ScrollLinkedEffectDetector();
 
  private:
   RefPtr<dom::Document> mDocument;
-  TimeStamp mTimeStamp;
 };
 
 }  // namespace layers

gfx/layers/wr/ClipManager.cpp

@@ -13,7 +13,6 @@
 #include "mozilla/layers/WebRenderLayerManager.h"
 #include "mozilla/webrender/WebRenderAPI.h"
 #include "nsDisplayList.h"
-#include "nsRefreshDriver.h"
 #include "nsStyleStructInlines.h"
 #include "UnitTransforms.h"
 
@@ -337,9 +336,8 @@ Maybe<wr::WrSpatialId> ClipManager::DefineScrollLayers(
   // Currently we track scroll-linked effects at the granularity of documents,
   // not scroll frames, so we consider a scroll frame to have a scroll-linked
   // effect whenever its containing document does.
-  nsPresContext* presContext = aItem->Frame()->PresContext();
   const bool hasScrollLinkedEffect =
-      presContext->Document()->HasScrollLinkedEffect();
+      aItem->Frame()->PresContext()->Document()->HasScrollLinkedEffect();
 
   return Some(mBuilder->DefineScrollLayer(
       viewId, parent, wr::ToLayoutRect(contentRect),

layout/generic/nsGfxScrollFrame.cpp

@@ -5864,8 +5864,7 @@ void ScrollFrameHelper::FireScrollEvent() {
   // Fire viewport scroll events at the document (where they
   // will bubble to the window)
   mozilla::layers::ScrollLinkedEffectDetector detector(
-      content->GetComposedDoc(),
-      presContext->RefreshDriver()->MostRecentRefresh());
+      content->GetComposedDoc());
   if (mIsRoot) {
     if (RefPtr<Document> doc = content->GetUncomposedDoc()) {
       // TODO: Bug 1506441

layout/painting/ActiveLayerTracker.cpp

@@ -266,6 +266,23 @@ void ActiveLayerTracker::NotifyRestyle(nsIFrame* aFrame,
   }
 }
 
+/* static */
+void ActiveLayerTracker::NotifyAnimated(nsIFrame* aFrame,
+                                        nsCSSPropertyID aProperty,
+                                        const nsACString& aNewValue,
+                                        nsDOMCSSDeclaration* aDOMCSSDecl) {
+  LayerActivity* layerActivity = GetLayerActivityForUpdate(aFrame);
+  uint8_t& mutationCount = layerActivity->RestyleCountForProperty(aProperty);
+  if (mutationCount != 0xFF) {
+    nsAutoCString oldValue;
+    aDOMCSSDecl->GetPropertyValue(aProperty, oldValue);
+    if (oldValue != aNewValue) {
+      // We know this is animated, so just hack the mutation count.
+      mutationCount = 0xFF;
+    }
+  }
+}
+
 static bool IsPresContextInScriptAnimationCallback(
     nsPresContext* aPresContext) {
   if (aPresContext->RefreshDriver()->IsInRefresh()) {
@@ -279,11 +296,10 @@ static bool IsPresContextInScriptAnimationCallback(
 
 /* static */
 void ActiveLayerTracker::NotifyInlineStyleRuleModified(
-    nsIFrame* aFrame, nsCSSPropertyID aProperty) {
+    nsIFrame* aFrame, nsCSSPropertyID aProperty, const nsACString& aNewValue,
+    nsDOMCSSDeclaration* aDOMCSSDecl) {
   if (IsPresContextInScriptAnimationCallback(aFrame->PresContext())) {
-    LayerActivity* layerActivity = GetLayerActivityForUpdate(aFrame);
-    // We know this is animated, so just hack the mutation count.
-    layerActivity->RestyleCountForProperty(aProperty) = 0xff;
+    NotifyAnimated(aFrame, aProperty, aNewValue, aDOMCSSDecl);
   }
 }
 

layout/painting/ActiveLayerTracker.h

@@ -45,13 +45,27 @@ class ActiveLayerTracker {
    * @param aProperty the property that has changed
    */
   static void NotifyRestyle(nsIFrame* aFrame, nsCSSPropertyID aProperty);
-
+  /**
+   * Mark aFrame as being known to have an animation of aProperty.
+   * Any such marking will time out after a short period.
+   * aNewValue and aDOMCSSDecl are used to determine whether the property's
+   * value has changed.
+   */
+  static void NotifyAnimated(nsIFrame* aFrame, nsCSSPropertyID aProperty,
+                             const nsACString& aNewValue,
+                             nsDOMCSSDeclaration* aDOMCSSDecl);
   /**
    * Notify that a property in the inline style rule of aFrame's element
    * has been modified.
+   * This notification is incomplete --- not all modifications to inline
+   * style will trigger this.
+   * aNewValue and aDOMCSSDecl are used to determine whether the property's
+   * value has changed.
    */
   static void NotifyInlineStyleRuleModified(nsIFrame* aFrame,
-                                            nsCSSPropertyID aProperty);
+                                            nsCSSPropertyID aProperty,
+                                            const nsACString& aNewValue,
+                                            nsDOMCSSDeclaration* aDOMCSSDecl);
   /**
    * Notify that a frame needs to be repainted. This is important for layering
    * decisions where, say, aFrame's transform is updated from JS, but we need

layout/style/ServoTypes.h

@@ -11,7 +11,6 @@
 
 #include "mozilla/RefPtr.h"
 #include "mozilla/TypedEnumBits.h"
-#include "nsCSSPropertyID.h"
 #include "nsCoord.h"
 #include "X11UndefineNone.h"
 
@@ -141,9 +140,8 @@ class ServoStyleSetSizes {
 // A callback that can be sent via FFI which will be invoked _right before_
 // being mutated, and at most once.
 struct DeclarationBlockMutationClosure {
-  // The callback function. The first argument is `data`, the second is the
-  // property id that changed.
-  void (*function)(void*, nsCSSPropertyID) = nullptr;
+  // The callback function. The argument is `data`.
+  void (*function)(void*) = nullptr;
   void* data = nullptr;
 };
 

layout/style/nsDOMCSSAttrDeclaration.cpp

@@ -12,7 +12,6 @@
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/SVGElement.h"
 #include "mozilla/dom/MutationEventBinding.h"
-#include "mozilla/layers/ScrollLinkedEffectDetector.h"
 #include "mozilla/DeclarationBlock.h"
 #include "mozilla/InternalMutationEvent.h"
 #include "mozilla/SMILCSSValueType.h"
@@ -188,67 +187,32 @@ nsresult nsDOMCSSAttributeDeclaration::SetSMILValue(
   });
 }
 
-// Scripted modifications to style.opacity or style.transform (or other
-// transform-like properties, e.g. style.translate, style.rotate, style.scale)
-// could immediately force us into the animated state if heuristics suggest
-// this is a scripted animation.
-//
-// FIXME: This is missing the margin shorthand and the logical versions of
-// the margin properties, see bug 1266287.
-static bool IsActiveLayerProperty(nsCSSPropertyID aPropID) {
-  switch (aPropID) {
-    case eCSSProperty_opacity:
-    case eCSSProperty_transform:
-    case eCSSProperty_translate:
-    case eCSSProperty_rotate:
-    case eCSSProperty_scale:
-    case eCSSProperty_offset_path:
-    case eCSSProperty_offset_distance:
-    case eCSSProperty_offset_rotate:
-    case eCSSProperty_offset_anchor:
-      return true;
-    default:
-      return false;
-  }
-}
-
 void nsDOMCSSAttributeDeclaration::SetPropertyValue(
     const nsCSSPropertyID aPropID, const nsACString& aValue,
     nsIPrincipal* aSubjectPrincipal, ErrorResult& aRv) {
+  // Scripted modifications to style.opacity or style.transform (or other
+  // transform-like properties, e.g. style.translate, style.rotate, style.scale)
+  // could immediately force us into the animated state if heuristics suggest
+  // this is scripted animation.
+  // FIXME: This is missing the margin shorthand and the logical versions of
+  // the margin properties, see bug 1266287.
+  if (aPropID == eCSSProperty_opacity || aPropID == eCSSProperty_transform ||
+      aPropID == eCSSProperty_translate || aPropID == eCSSProperty_rotate ||
+      aPropID == eCSSProperty_scale || aPropID == eCSSProperty_offset_path ||
+      aPropID == eCSSProperty_offset_distance ||
+      aPropID == eCSSProperty_offset_rotate ||
+      aPropID == eCSSProperty_offset_anchor) {
+    nsIFrame* frame = mElement->GetPrimaryFrame();
+    if (frame) {
+      ActiveLayerTracker::NotifyInlineStyleRuleModified(frame, aPropID, aValue,
+                                                        this);
+    }
+  }
   nsDOMCSSDeclaration::SetPropertyValue(aPropID, aValue, aSubjectPrincipal,
                                         aRv);
 }
 
-static bool IsScrollLinkedEffectiveProperty(const nsCSSPropertyID aPropID) {
-  switch (aPropID) {
-    case eCSSProperty_background_position:
-    case eCSSProperty_background_position_x:
-    case eCSSProperty_background_position_y:
-    case eCSSProperty_transform:
-    case eCSSProperty_translate:
-    case eCSSProperty_rotate:
-    case eCSSProperty_scale:
-    case eCSSProperty_top:
-    case eCSSProperty_left:
-    case eCSSProperty_bottom:
-    case eCSSProperty_right:
-    case eCSSProperty_margin:
-    case eCSSProperty_margin_top:
-    case eCSSProperty_margin_left:
-    case eCSSProperty_margin_bottom:
-    case eCSSProperty_margin_right:
-    case eCSSProperty_margin_inline_start:
-    case eCSSProperty_margin_inline_end:
-    case eCSSProperty_margin_block_start:
-    case eCSSProperty_margin_block_end:
-      return true;
-    default:
-      return false;
-  }
-}
-
-void nsDOMCSSAttributeDeclaration::MutationClosureFunction(
-    void* aData, nsCSSPropertyID aPropID) {
+void nsDOMCSSAttributeDeclaration::MutationClosureFunction(void* aData) {
   auto* data = static_cast<MutationClosureData*>(aData);
   MOZ_ASSERT(
       data->mShouldBeCalled,
@@ -256,15 +220,6 @@ void nsDOMCSSAttributeDeclaration::MutationClosureFunction(
   if (data->mWasCalled) {
     return;
   }
-  if (IsScrollLinkedEffectiveProperty(aPropID)) {
-    mozilla::layers::ScrollLinkedEffectDetector::PositioningPropertyMutated();
-  }
-  if (IsActiveLayerProperty(aPropID)) {
-    if (nsIFrame* frame = data->mElement->GetPrimaryFrame()) {
-      ActiveLayerTracker::NotifyInlineStyleRuleModified(frame, aPropID);
-    }
-  }
-
   data->mWasCalled = true;
   data->mElement->InlineStyleDeclarationWillChange(*data);
 }

layout/style/nsDOMCSSAttrDeclaration.h

@@ -60,7 +60,7 @@ class nsDOMCSSAttributeDeclaration final : public nsDOMCSSDeclaration {
                         nsIPrincipal* aSubjectPrincipal,
                         mozilla::ErrorResult& aRv) override;
 
-  static void MutationClosureFunction(void* aData, nsCSSPropertyID);
+  static void MutationClosureFunction(void* aData);
 
   void GetPropertyChangeClosure(
       mozilla::DeclarationBlockMutationClosure* aClosure,

layout/style/nsDOMCSSDeclaration.cpp

@@ -19,6 +19,7 @@
 #include "mozAutoDocUpdate.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "nsQueryObject.h"
+#include "mozilla/layers/ScrollLinkedEffectDetector.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -54,6 +55,33 @@ void nsDOMCSSDeclaration::SetPropertyValue(const nsCSSPropertyID aPropID,
     return;
   }
 
+  switch (aPropID) {
+    case eCSSProperty_background_position:
+    case eCSSProperty_background_position_x:
+    case eCSSProperty_background_position_y:
+    case eCSSProperty_transform:
+    case eCSSProperty_translate:
+    case eCSSProperty_rotate:
+    case eCSSProperty_scale:
+    case eCSSProperty_top:
+    case eCSSProperty_left:
+    case eCSSProperty_bottom:
+    case eCSSProperty_right:
+    case eCSSProperty_margin:
+    case eCSSProperty_margin_top:
+    case eCSSProperty_margin_left:
+    case eCSSProperty_margin_bottom:
+    case eCSSProperty_margin_right:
+    case eCSSProperty_margin_inline_start:
+    case eCSSProperty_margin_inline_end:
+    case eCSSProperty_margin_block_start:
+    case eCSSProperty_margin_block_end:
+      mozilla::layers::ScrollLinkedEffectDetector::PositioningPropertyMutated();
+      break;
+    default:
+      break;
+  }
+
   if (aValue.IsEmpty()) {
     // If the new value of the property is an empty string we remove the
     // property.
@@ -108,7 +136,7 @@ void nsDOMCSSDeclaration::SetCssText(const nsACString& aCssText,
   // doesn't modify any existing declaration and that is why the callback isn't
   // called implicitly.
   if (closure.function && !closureData.mWasCalled) {
-    closure.function(&closureData, eCSSProperty_UNKNOWN);
+    closure.function(&closureData);
   }
 
   RefPtr<DeclarationBlock> newdecl = DeclarationBlock::FromCssText(

servo/components/style/properties/properties.mako.rs

@@ -474,10 +474,9 @@ impl NonCustomPropertyId {
         self.0
     }
 
-    /// Convert a `NonCustomPropertyId` into a `nsCSSPropertyID`.
     #[cfg(feature = "gecko")]
     #[inline]
-    pub fn to_nscsspropertyid(self) -> nsCSSPropertyID {
+    fn to_nscsspropertyid(self) -> nsCSSPropertyID {
         // unsafe: guaranteed by static_assert_nscsspropertyid above.
         unsafe { std::mem::transmute(self.0 as i32) }
     }

servo/ports/geckolib/glue.rs

@@ -146,18 +146,14 @@ use style_traits::{CssWriter, ParsingMode, StyleParseErrorKind, ToCss};
 use to_shmem::SharedMemoryBuilder;
 
 trait ClosureHelper {
-    fn invoke(&self, property_id: Option<NonCustomPropertyId>);
+    fn invoke(&self);
 }
 
 impl ClosureHelper for DeclarationBlockMutationClosure {
     #[inline]
-    fn invoke(&self, property_id: Option<NonCustomPropertyId>) {
+    fn invoke(&self) {
         if let Some(function) = self.function.as_ref() {
-            let gecko_prop_id = match property_id {
-                Some(p) => p.to_nscsspropertyid(),
-                None => nsCSSPropertyID::eCSSPropertyExtra_variable,
-            };
-            unsafe { function(self.data, gecko_prop_id) }
+            unsafe { function(self.data) };
         }
     }
 }
@@ -4661,7 +4657,6 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_GetPropertyIsImportant(
 
 #[inline(always)]
 fn set_property_to_declarations(
-    non_custom_property_id: Option<NonCustomPropertyId>,
     block: &RawServoDeclarationBlock,
     parsed_declarations: &mut SourcePropertyDeclaration,
     before_change_closure: DeclarationBlockMutationClosure,
@@ -4675,7 +4670,7 @@ fn set_property_to_declarations(
         return false;
     }
 
-    before_change_closure.invoke(non_custom_property_id);
+    before_change_closure.invoke();
     write_locked_arc(block, |decls: &mut PropertyDeclarationBlock| {
         decls.update(parsed_declarations.drain(), importance, &mut updates)
     });
@@ -4696,7 +4691,6 @@ fn set_property(
 ) -> bool {
     let mut source_declarations = SourcePropertyDeclaration::new();
     let reporter = ErrorReporter::new(ptr::null_mut(), loader, data.ptr());
-    let non_custom_property_id = property_id.non_custom_id();
     let result = parse_property_into(
         &mut source_declarations,
         property_id,
@@ -4720,7 +4714,6 @@ fn set_property(
     };
 
     set_property_to_declarations(
-        non_custom_property_id,
         declarations,
         &mut source_declarations,
         before_change_closure,
@@ -4761,13 +4754,10 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyToAnimationValue(
     animation_value: &RawServoAnimationValue,
     before_change_closure: DeclarationBlockMutationClosure,
 ) -> bool {
-    let animation_value = AnimationValue::as_arc(&animation_value);
-    let non_custom_property_id = animation_value.id().into();
     let mut source_declarations =
-        SourcePropertyDeclaration::with_one(animation_value.uncompute());
+        SourcePropertyDeclaration::with_one(AnimationValue::as_arc(&animation_value).uncompute());
 
     set_property_to_declarations(
-        Some(non_custom_property_id),
         declarations,
         &mut source_declarations,
         before_change_closure,
@@ -4816,7 +4806,7 @@ fn remove_property(
         None => return false,
     };
 
-    before_change_closure.invoke(property_id.non_custom_id());
+    before_change_closure.invoke();
     write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
         decls.remove_property(&property_id, first_declaration)
     });