Bug 1437244: Try to assert a bit more about stylist accesses. r?hiro draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 09 Feb 2018 22:09:59 +0100
changeset 753684 e0ee08ed5ff2c95b7a304bc89280c698c66a299b
parent 753683 33a310b0eb1c60ae2d26387aa85bcc509ede3af4
child 753685 032d354722a5ef1560e451c5554d29f659f83aea
push id98647
push userbmo:emilio@crisal.io
push dateSun, 11 Feb 2018 20:22:05 +0000
reviewershiro
bugs1437244
milestone60.0a1
Bug 1437244: Try to assert a bit more about stylist accesses. r?hiro The keyframe stuff runs from animation building, which needs clean styles already. Same for counter styles, they run from a well-defined point in time where rules should be up-to-date already. The canvas stuff needs no stylist access mostly, since it's only used to compute a couple font-related things. MozReview-Commit-ID: 3Vh1wzeaYl3
layout/style/ServoStyleSet.cpp
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -451,17 +451,17 @@ ServoStyleSet::PreTraverseSync()
       gfxUserFontSet::UserFontCache::ClearAllowedFontSets(userFontSet);
     }
     if (cacheGeneration != mUserFontCacheUpdateGeneration || principalChanged) {
       gfxUserFontSet::UserFontCache::UpdateAllowedFontSets(userFontSet);
       mUserFontCacheUpdateGeneration = cacheGeneration;
     }
   }
 
-  UpdateStylistIfNeeded();
+  MOZ_ASSERT(!StylistNeedsUpdate());
   presContext->CacheAllLangs();
 }
 
 void
 ServoStyleSet::PreTraverse(ServoTraversalFlags aFlags, Element* aRoot)
 {
   PreTraverseSync();
 
@@ -567,18 +567,19 @@ LazyPseudoIsCacheable(CSSPseudoElementTy
 }
 
 already_AddRefed<ServoStyleContext>
 ServoStyleSet::ResolvePseudoElementStyle(Element* aOriginatingElement,
                                          CSSPseudoElementType aType,
                                          ServoStyleContext* aParentContext,
                                          Element* aPseudoElement)
 {
+  // Runs from frame construction, this should have clean styles already, except
+  // with non-lazy FC...
   UpdateStylistIfNeeded();
-
   MOZ_ASSERT(aType < CSSPseudoElementType::Count);
 
   RefPtr<ServoStyleContext> computedValues;
 
   if (aPseudoElement) {
     MOZ_ASSERT(aType == aPseudoElement->GetPseudoElementType());
     computedValues =
       Servo_ResolveStyle(aPseudoElement, mRawSet.get()).Consume();
@@ -924,16 +925,18 @@ ServoStyleSet::AddDocStyleSheet(ServoSty
   return NS_OK;
 }
 
 already_AddRefed<ServoStyleContext>
 ServoStyleSet::ProbePseudoElementStyle(Element* aOriginatingElement,
                                        CSSPseudoElementType aType,
                                        ServoStyleContext* aParentContext)
 {
+  // Runs from frame construction, this should have clean styles already, except
+  // with non-lazy FC...
   UpdateStylistIfNeeded();
 
   // NB: We ignore aParentContext, because in some cases
   // (first-line/first-letter on anonymous box blocks) Gecko passes something
   // nonsensical there.  In all other cases we want to inherit directly from
   // aOriginatingElement's styles anyway.
   MOZ_ASSERT(aType < CSSPseudoElementType::Count);
 
@@ -1171,18 +1174,19 @@ ServoStyleSet::AssertTreeIsClean()
 }
 #endif
 
 bool
 ServoStyleSet::GetKeyframesForName(nsAtom* aName,
                                    const nsTimingFunction& aTimingFunction,
                                    nsTArray<Keyframe>& aKeyframes)
 {
-  UpdateStylistIfNeeded();
-
+  // TODO(emilio): This may need to look at the element itself for handling
+  // @keyframes properly in Shadow DOM.
+  MOZ_ASSERT(!StylistNeedsUpdate());
   return Servo_StyleSet_GetKeyframesForName(mRawSet.get(),
                                             aName,
                                             &aTimingFunction,
                                             &aKeyframes);
 }
 
 nsTArray<ComputedKeyframeValues>
 ServoStyleSet::GetComputedKeyframeValuesFor(
@@ -1399,36 +1403,37 @@ ServoStyleSet::AppendFontFaceRules(nsTAr
   UpdateStylistIfNeeded();
   Servo_StyleSet_GetFontFaceRules(mRawSet.get(), &aArray);
   return true;
 }
 
 nsCSSCounterStyleRule*
 ServoStyleSet::CounterStyleRuleForName(nsAtom* aName)
 {
-  // FIXME(emilio): This should probably call UpdateStylistIfNeeded, or
-  // otherwise assert?
+  MOZ_ASSERT(!StylistNeedsUpdate());
   return Servo_StyleSet_GetCounterStyleRule(mRawSet.get(), aName);
 }
 
 already_AddRefed<gfxFontFeatureValueSet>
 ServoStyleSet::BuildFontFeatureValueSet()
 {
+  // FIXME(emilio): This should assert once we update the stylist from
+  // FlushPendingNotifications explicitly.
   UpdateStylistIfNeeded();
   RefPtr<gfxFontFeatureValueSet> set =
     Servo_StyleSet_BuildFontFeatureValueSet(mRawSet.get());
   return set.forget();
 }
 
 already_AddRefed<ServoStyleContext>
 ServoStyleSet::ResolveForDeclarations(
   const ServoStyleContext* aParentOrNull,
   RawServoDeclarationBlockBorrowed aDeclarations)
 {
-  UpdateStylistIfNeeded();
+  // No need to update the stylist, we're only cascading aDeclarations.
   return Servo_StyleSet_ResolveForDeclarations(mRawSet.get(),
                                                aParentOrNull,
                                                aDeclarations).Consume();
 }
 
 void
 ServoStyleSet::UpdateStylist()
 {