Bug 1545177 - If descriptors of a FontFace are modified after creation, update the associated font entry so that face selection will respect the new values, and mark font sets as dirty. r=heycam
authorJonathan Kew <jkew@mozilla.com>
Thu, 09 May 2019 09:43:42 +0000
changeset 532009 3a8c6048c5d0874dd3d32c06e7923625b9383e8d
parent 532008 c24c083425d0e543bd59076d675c2efcb3b7c73c
child 532010 9b9b19e53c3eb97722cf585c4f4fb88f1eef41a8
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1545177
milestone68.0a1
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
Bug 1545177 - If descriptors of a FontFace are modified after creation, update the associated font entry so that face selection will respect the new values, and mark font sets as dirty. r=heycam Differential Revision: https://phabricator.services.mozilla.com/D27980
gfx/thebes/gfxUserFontSet.cpp
gfx/thebes/gfxUserFontSet.h
layout/style/FontFace.cpp
layout/style/FontFace.h
layout/style/FontFaceSet.cpp
layout/style/FontFaceSet.h
servo/ports/geckolib/glue.rs
--- a/gfx/thebes/gfxUserFontSet.cpp
+++ b/gfx/thebes/gfxUserFontSet.cpp
@@ -121,16 +121,37 @@ gfxUserFontEntry::gfxUserFontEntry(
   mStyleRange = aStyle;
   mFeatureSettings.AppendElements(aFeatureSettings);
   mVariationSettings.AppendElements(aVariationSettings);
   mLanguageOverride = aLanguageOverride;
   mCharacterMap = aUnicodeRanges;
   mRangeFlags = aRangeFlags;
 }
 
+void gfxUserFontEntry::UpdateAttributes(
+    WeightRange aWeight, StretchRange aStretch, SlantStyleRange aStyle,
+    const nsTArray<gfxFontFeature>& aFeatureSettings,
+    const nsTArray<gfxFontVariation>& aVariationSettings,
+    uint32_t aLanguageOverride, gfxCharacterMap* aUnicodeRanges,
+    StyleFontDisplay aFontDisplay, RangeFlags aRangeFlags) {
+  // Remove the entry from the user font cache, if present there, as the cache
+  // key may no longer be correct with the new attributes.
+  gfxUserFontSet::UserFontCache::ForgetFont(this);
+
+  mFontDisplay = aFontDisplay;
+  mWeightRange = aWeight;
+  mStretchRange = aStretch;
+  mStyleRange = aStyle;
+  mFeatureSettings = aFeatureSettings;
+  mVariationSettings = aVariationSettings;
+  mLanguageOverride = aLanguageOverride;
+  mCharacterMap = aUnicodeRanges;
+  mRangeFlags = aRangeFlags;
+}
+
 gfxUserFontEntry::~gfxUserFontEntry() {
   // Assert that we don't drop any gfxUserFontEntry objects during a Servo
   // traversal, since PostTraversalTask objects can hold raw pointers to
   // gfxUserFontEntry objects.
   MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal());
 }
 
 bool gfxUserFontEntry::Matches(
--- a/gfx/thebes/gfxUserFontSet.h
+++ b/gfx/thebes/gfxUserFontSet.h
@@ -142,16 +142,17 @@ class gfxUserFontFamily : public gfxFont
   friend class gfxUserFontSet;
 
   explicit gfxUserFontFamily(const nsACString& aName) : gfxFontFamily(aName) {}
 
   virtual ~gfxUserFontFamily();
 
   // add the given font entry to the end of the family's list
   void AddFontEntry(gfxFontEntry* aFontEntry) {
+    MOZ_ASSERT(!mIsSimpleFamily, "not valid for user-font families");
     // keep ref while removing existing entry
     RefPtr<gfxFontEntry> fe = aFontEntry;
     // remove existing entry, if already present
     mAvailableFonts.RemoveElement(aFontEntry);
     // insert at the beginning so that the last-defined font is the first
     // one in the fontlist used for matching, as per CSS Fonts spec
     mAvailableFonts.InsertElementAt(0, aFontEntry);
 
@@ -164,16 +165,21 @@ class gfxUserFontFamily : public gfxFont
       ToLowerCase(thisName);
       ToLowerCase(entryName);
       MOZ_ASSERT(thisName.Equals(entryName));
 #endif
     }
     ResetCharacterMap();
   }
 
+  void RemoveFontEntry(gfxFontEntry* aFontEntry) {
+    MOZ_ASSERT(!mIsSimpleFamily, "not valid for user-font families");
+    mAvailableFonts.RemoveElement(aFontEntry);
+  }
+
   // Remove all font entries from the family
   void DetachFontEntries() { mAvailableFonts.Clear(); }
 };
 
 class gfxUserFontEntry;
 class gfxOTSContext;
 
 class gfxUserFontSet {
@@ -536,16 +542,27 @@ class gfxUserFontEntry : public gfxFontE
                    const nsTArray<gfxFontFeature>& aFeatureSettings,
                    const nsTArray<gfxFontVariation>& aVariationSettings,
                    uint32_t aLanguageOverride, gfxCharacterMap* aUnicodeRanges,
                    mozilla::StyleFontDisplay aFontDisplay,
                    RangeFlags aRangeFlags);
 
   virtual ~gfxUserFontEntry();
 
+  // Update the attributes of the entry to the given values, without disturbing
+  // the associated platform font entry or in-progress downloads.
+  void UpdateAttributes(WeightRange aWeight, StretchRange aStretch,
+                        SlantStyleRange aStyle,
+                        const nsTArray<gfxFontFeature>& aFeatureSettings,
+                        const nsTArray<gfxFontVariation>& aVariationSettings,
+                        uint32_t aLanguageOverride,
+                        gfxCharacterMap* aUnicodeRanges,
+                        mozilla::StyleFontDisplay aFontDisplay,
+                        RangeFlags aRangeFlags);
+
   // Return whether the entry matches the given list of attributes
   bool Matches(const nsTArray<gfxFontFaceSrc>& aFontFaceSrcList,
                WeightRange aWeight, StretchRange aStretch,
                SlantStyleRange aStyle,
                const nsTArray<gfxFontFeature>& aFeatureSettings,
                const nsTArray<gfxFontVariation>& aVariationSettings,
                uint32_t aLanguageOverride, gfxCharacterMap* aUnicodeRanges,
                mozilla::StyleFontDisplay aFontDisplay, RangeFlags aRangeFlags);
@@ -585,19 +602,19 @@ class gfxUserFontEntry : public gfxFontE
 
   // load the font - starts the loading of sources which continues until
   // a valid font resource is found or all sources fail
   void Load();
 
   // methods to expose some information to FontFaceSet::UserFontSet
   // since we can't make that class a friend
   void SetLoader(nsFontFaceLoader* aLoader) { mLoader = aLoader; }
-  nsFontFaceLoader* GetLoader() { return mLoader; }
-  gfxFontSrcPrincipal* GetPrincipal() { return mPrincipal; }
-  uint32_t GetSrcIndex() { return mSrcIndex; }
+  nsFontFaceLoader* GetLoader() const { return mLoader; }
+  gfxFontSrcPrincipal* GetPrincipal() const { return mPrincipal; }
+  uint32_t GetSrcIndex() const { return mSrcIndex; }
   void GetFamilyNameAndURIForLogging(nsACString& aFamilyName, nsACString& aURI);
 
   gfxFontEntry* Clone() const override {
     MOZ_ASSERT_UNREACHABLE("cannot Clone user fonts");
     return nullptr;
   }
 
 #ifdef DEBUG
--- a/layout/style/FontFace.cpp
+++ b/layout/style/FontFace.cpp
@@ -173,17 +173,18 @@ already_AddRefed<FontFace> FontFace::Con
   obj->InitializeSource(aSource);
   return obj.forget();
 }
 
 void FontFace::InitializeSource(
     const StringOrArrayBufferOrArrayBufferView& aSource) {
   if (aSource.IsString()) {
     IgnoredErrorResult rv;
-    if (!SetDescriptor(eCSSFontDesc_Src, aSource.GetAsString(), rv)) {
+    SetDescriptor(eCSSFontDesc_Src, aSource.GetAsString(), rv);
+    if (rv.Failed()) {
       Reject(NS_ERROR_DOM_SYNTAX_ERR);
 
       SetStatus(FontFaceLoadStatus::Error);
       return;
     }
 
     mSourceType = eSourceType_URLs;
     return;
@@ -205,57 +206,67 @@ void FontFace::InitializeSource(
 
 void FontFace::GetFamily(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_Family, aResult);
 }
 
 void FontFace::SetFamily(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_Family, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_Family, aValue, aRv)) {
+    DescriptorUpdated();
+  }
 }
 
 void FontFace::GetStyle(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_Style, aResult);
 }
 
 void FontFace::SetStyle(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_Style, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_Style, aValue, aRv)) {
+    DescriptorUpdated();
+  }
 }
 
 void FontFace::GetWeight(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_Weight, aResult);
 }
 
 void FontFace::SetWeight(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_Weight, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_Weight, aValue, aRv)) {
+    DescriptorUpdated();
+  }
 }
 
 void FontFace::GetStretch(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_Stretch, aResult);
 }
 
 void FontFace::SetStretch(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_Stretch, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_Stretch, aValue, aRv)) {
+    DescriptorUpdated();
+  }
 }
 
 void FontFace::GetUnicodeRange(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_UnicodeRange, aResult);
 }
 
 void FontFace::SetUnicodeRange(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_UnicodeRange, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_UnicodeRange, aValue, aRv)) {
+    DescriptorUpdated();
+  }
 }
 
 void FontFace::GetVariant(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
 
   // XXX Just expose the font-variant descriptor as "normal" until we
   // support it properly (bug 1055385).
   aResult.AssignLiteral("normal");
@@ -270,37 +281,65 @@ void FontFace::SetVariant(const nsAStrin
 
 void FontFace::GetFeatureSettings(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_FontFeatureSettings, aResult);
 }
 
 void FontFace::SetFeatureSettings(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_FontFeatureSettings, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_FontFeatureSettings, aValue, aRv)) {
+    DescriptorUpdated();
+  }
 }
 
 void FontFace::GetVariationSettings(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_FontVariationSettings, aResult);
 }
 
 void FontFace::SetVariationSettings(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_FontVariationSettings, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_FontVariationSettings, aValue, aRv)) {
+    DescriptorUpdated();
+  }
 }
 
 void FontFace::GetDisplay(nsString& aResult) {
   mFontFaceSet->FlushUserFontSet();
   GetDesc(eCSSFontDesc_Display, aResult);
 }
 
 void FontFace::SetDisplay(const nsAString& aValue, ErrorResult& aRv) {
   mFontFaceSet->FlushUserFontSet();
-  SetDescriptor(eCSSFontDesc_Display, aValue, aRv);
+  if (SetDescriptor(eCSSFontDesc_Display, aValue, aRv)) {
+    DescriptorUpdated();
+  }
+}
+
+void FontFace::DescriptorUpdated()
+{
+  // If we haven't yet initialized mUserFontEntry, no need to do anything here;
+  // we'll respect the updated descriptor when the time comes to create it.
+  if (!mUserFontEntry) {
+    return;
+  }
+
+  // Behind the scenes, this will actually update the existing entry and return
+  // it, rather than create a new one.
+  RefPtr<gfxUserFontEntry> newEntry =
+    mFontFaceSet->FindOrCreateUserFontEntryFromFontFace(this);
+  SetUserFontEntry(newEntry);
+
+  if (mInFontFaceSet) {
+    mFontFaceSet->MarkUserFontSetDirty();
+  }
+  for (auto& set : mOtherFontFaceSets) {
+    set->MarkUserFontSetDirty();
+  }
 }
 
 FontFaceLoadStatus FontFace::Status() { return mStatus; }
 
 Promise* FontFace::Load(ErrorResult& aRv) {
   MOZ_ASSERT(NS_IsMainThread());
 
   mFontFaceSet->FlushUserFontSet();
@@ -442,66 +481,77 @@ already_AddRefed<URLExtraData> FontFace:
 
   // We pass RP_Unset when creating URLExtraData object here because it's not
   // going to result to change referer policy in a resource request.
   RefPtr<URLExtraData> url =
       new URLExtraData(base, docURI, principal, net::RP_Unset);
   return url.forget();
 }
 
+// Boolean result indicates whether the value of the descriptor was actually
+// changed.
 bool FontFace::SetDescriptor(nsCSSFontDesc aFontDesc, const nsAString& aValue,
                              ErrorResult& aRv) {
   // FIXME We probably don't need to distinguish between this anymore
   // since we have common backend now.
   NS_ASSERTION(!HasRule(), "we don't handle rule backed FontFace objects yet");
   if (HasRule()) {
     return false;
   }
 
   // FIXME(heycam): Should not allow modification of FontFaces that are
   // CSS-connected and whose rule is read only.
 
   NS_ConvertUTF16toUTF8 value(aValue);
   RefPtr<URLExtraData> url = GetURLExtraData();
-  if (!Servo_FontFaceRule_SetDescriptor(GetData(), aFontDesc, &value, url)) {
+  bool changed;
+  if (!Servo_FontFaceRule_SetDescriptor(GetData(), aFontDesc, &value, url,
+                                        &changed)) {
     aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
     return false;
   }
 
+  if (!changed) {
+    return false;
+  }
+
   if (aFontDesc == eCSSFontDesc_UnicodeRange) {
     mUnicodeRangeDirty = true;
   }
 
-  // XXX Setting descriptors doesn't actually have any effect on FontFace
-  // objects that have started loading or have already been loaded.
   return true;
 }
 
 bool FontFace::SetDescriptors(const nsAString& aFamily,
                               const FontFaceDescriptors& aDescriptors) {
   MOZ_ASSERT(!HasRule());
   MOZ_ASSERT(!mDescriptors);
 
-  IgnoredErrorResult rv;
   mDescriptors = Servo_FontFaceRule_CreateEmpty().Consume();
 
+  // Helper to call SetDescriptor and return true on success, false on failure.
+  auto setDesc = [=](nsCSSFontDesc aDesc, const nsAString& aVal) -> bool {
+    IgnoredErrorResult rv;
+    SetDescriptor(aDesc, aVal, rv);
+    return !rv.Failed();
+  };
+
   // Parse all of the mDescriptors in aInitializer, which are the values
   // we got from the JS constructor.
-  if (!SetDescriptor(eCSSFontDesc_Family, aFamily, rv) ||
-      !SetDescriptor(eCSSFontDesc_Style, aDescriptors.mStyle, rv) ||
-      !SetDescriptor(eCSSFontDesc_Weight, aDescriptors.mWeight, rv) ||
-      !SetDescriptor(eCSSFontDesc_Stretch, aDescriptors.mStretch, rv) ||
-      !SetDescriptor(eCSSFontDesc_UnicodeRange, aDescriptors.mUnicodeRange,
-                     rv) ||
-      !SetDescriptor(eCSSFontDesc_FontFeatureSettings,
-                     aDescriptors.mFeatureSettings, rv) ||
+  if (!setDesc(eCSSFontDesc_Family, aFamily) ||
+      !setDesc(eCSSFontDesc_Style, aDescriptors.mStyle) ||
+      !setDesc(eCSSFontDesc_Weight, aDescriptors.mWeight) ||
+      !setDesc(eCSSFontDesc_Stretch, aDescriptors.mStretch) ||
+      !setDesc(eCSSFontDesc_UnicodeRange, aDescriptors.mUnicodeRange) ||
+      !setDesc(eCSSFontDesc_FontFeatureSettings,
+               aDescriptors.mFeatureSettings) ||
       (StaticPrefs::layout_css_font_variations_enabled() &&
-       !SetDescriptor(eCSSFontDesc_FontVariationSettings,
-                      aDescriptors.mVariationSettings, rv)) ||
-      !SetDescriptor(eCSSFontDesc_Display, aDescriptors.mDisplay, rv)) {
+       !setDesc(eCSSFontDesc_FontVariationSettings,
+                aDescriptors.mVariationSettings)) ||
+      !setDesc(eCSSFontDesc_Display, aDescriptors.mDisplay)) {
     // XXX Handle font-variant once we support it (bug 1055385).
 
     // If any of the descriptors failed to parse, none of them should be set
     // on the FontFace.
     mDescriptors = Servo_FontFaceRule_CreateEmpty().Consume();
 
     Reject(NS_ERROR_DOM_SYNTAX_ERR);
 
--- a/layout/style/FontFace.h
+++ b/layout/style/FontFace.h
@@ -183,28 +183,36 @@ class FontFace final : public nsISupport
   ~FontFace();
 
   void InitializeSource(const StringOrArrayBufferOrArrayBufferView& aSource);
 
   // Helper function for Load.
   void DoLoad();
 
   // Helper function for the descriptor setter methods.
-  // Returns whether it successfully sets the descriptor.
+  // Returns true if the descriptor was modified, false if descriptor is
+  // unchanged (which may not be an error: check aRv for actual failure).
   bool SetDescriptor(nsCSSFontDesc aFontDesc, const nsAString& aValue,
                      mozilla::ErrorResult& aRv);
 
   /**
    * Sets all of the descriptor values in mDescriptors using values passed
    * to the JS constructor.
+   * Returns true on success, false if parsing any descriptor failed.
    */
   bool SetDescriptors(const nsAString& aFamily,
                       const FontFaceDescriptors& aDescriptors);
 
   /**
+   * Called when a descriptor has been modified, so font-face sets can
+   * be told to refresh.
+   */
+  void DescriptorUpdated();
+
+  /**
    * Sets the current loading status.
    */
   void SetStatus(mozilla::dom::FontFaceLoadStatus aStatus);
 
   void GetDesc(nsCSSFontDesc aDescID, nsString& aResult) const;
 
   already_AddRefed<URLExtraData> GetURLExtraData() const;
 
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -1025,22 +1025,47 @@ FontFaceSet::FindOrCreateUserFontEntryFr
   if (Maybe<StyleFontLanguageOverride> descriptor =
           aFontFace->GetFontLanguageOverride()) {
     languageOverride = descriptor->_0;
   }
 
   // set up unicode-range
   gfxCharacterMap* unicodeRanges = aFontFace->GetUnicodeRangeAsCharacterMap();
 
+  RefPtr<gfxUserFontEntry> existingEntry = aFontFace->GetUserFontEntry();
+  if (existingEntry) {
+    // aFontFace already has a user font entry, so we update its attributes
+    // rather than creating a new one.
+    existingEntry->UpdateAttributes(weight, stretch, italicStyle,
+                                    featureSettings, variationSettings,
+                                    languageOverride, unicodeRanges,
+                                    fontDisplay, rangeFlags);
+    // If the family name has changed, remove the entry from its current family
+    // and clear the mFamilyName field so it can be reset when added to a new
+    // family.
+    if (!existingEntry->mFamilyName.IsEmpty() &&
+        existingEntry->mFamilyName != aFamilyName) {
+      gfxUserFontFamily* family =
+        set->GetUserFontSet()->LookupFamily(existingEntry->mFamilyName);
+      if (family) {
+        family->RemoveFontEntry(existingEntry);
+      }
+      existingEntry->mFamilyName.Truncate(0);
+    }
+    return existingEntry.forget();
+  }
+
   // set up src array
   nsTArray<gfxFontFaceSrc> srcArray;
 
   if (aFontFace->HasFontData()) {
     gfxFontFaceSrc* face = srcArray.AppendElement();
-    if (!face) return nullptr;
+    if (!face) {
+      return nullptr;
+    }
 
     face->mSourceType = gfxFontFaceSrc::eSourceType_Buffer;
     face->mBuffer = aFontFace->CreateBufferSource();
     face->mReferrerPolicy = mozilla::net::RP_Unset;
   } else {
     AutoTArray<StyleFontFaceSourceListComponent, 8> sourceListComponents;
     aFontFace->GetSources(sourceListComponents);
     size_t len = sourceListComponents.Length();
--- a/layout/style/FontFaceSet.h
+++ b/layout/style/FontFaceSet.h
@@ -194,16 +194,18 @@ class FontFaceSet final : public DOMEven
   already_AddRefed<mozilla::dom::FontFaceSetIterator> Values();
   MOZ_CAN_RUN_SCRIPT
   void ForEach(JSContext* aCx, FontFaceSetForEachCallback& aCallback,
                JS::Handle<JS::Value> aThisArg, mozilla::ErrorResult& aRv);
 
   // For ServoStyleSet to know ahead of time whether a font is loadable.
   void CacheFontLoadability();
 
+  void MarkUserFontSetDirty();
+
  private:
   ~FontFaceSet();
 
   /**
    * Returns whether the given FontFace is currently "in" the FontFaceSet.
    */
   bool HasAvailableFontFace(FontFace* aFontFace);
 
@@ -273,17 +275,16 @@ class FontFaceSet final : public DOMEven
   bool IsFontLoadAllowed(const gfxFontFaceSrc& aSrc);
 
   void DispatchFontLoadViolations(nsTArray<nsCOMPtr<nsIRunnable>>& aViolations);
   nsresult SyncLoadFontData(gfxUserFontEntry* aFontToLoad,
                             const gfxFontFaceSrc* aFontFaceSrc,
                             uint8_t*& aBuffer, uint32_t& aBufferLength);
   nsresult LogMessage(gfxUserFontEntry* aUserFontEntry, const char* aMessage,
                       uint32_t aFlags, nsresult aStatus);
-  void MarkUserFontSetDirty();
 
   void InsertRuleFontFace(FontFace* aFontFace, StyleOrigin aOrigin,
                           nsTArray<FontFaceRecord>& aOldRecords,
                           bool& aFontSetModified);
   void InsertNonRuleFontFace(FontFace* aFontFace, bool& aFontSetModified);
 
 #ifdef DEBUG
   bool HasRuleFontFace(FontFace* aFontFace);
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -2870,16 +2870,17 @@ pub unsafe extern "C" fn Servo_FontFaceR
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_FontFaceRule_SetDescriptor(
     rule: &RawServoFontFaceRule,
     desc: nsCSSFontDesc,
     value: *const nsACString,
     data: *mut URLExtraData,
+    out_changed: *mut bool,
 ) -> bool {
     let value = value.as_ref().unwrap().as_str_unchecked();
     let mut input = ParserInput::new(&value);
     let mut parser = Parser::new(&mut input);
     let url_data = UrlExtraData::from_ptr_ref(&data);
     let context = ParserContext::new(
         Origin::Author,
         url_data,
@@ -2895,17 +2896,19 @@ pub unsafe extern "C" fn Servo_FontFaceR
             (
                 valid: [$($v_enum_name:ident => $field:ident,)*]
                 invalid: [$($i_enum_name:ident,)*]
             ) => {
                 match desc {
                     $(
                         nsCSSFontDesc::$v_enum_name => {
                             if let Ok(value) = parser.parse_entirely(|i| Parse::parse(&context, i)) {
-                                rule.$field = Some(value);
+                                let result = Some(value);
+                                *out_changed = result != rule.$field;
+                                rule.$field = result;
                                 true
                             } else {
                                 false
                             }
                         }
                     )*
                     $(
                         nsCSSFontDesc::$i_enum_name => {