Bug 1352306 - Part 1: stylo: Only snapshot attributes if there is some rule that depends on that attribute. r=emilio
authorCameron McCormack <cam@mcc.id.au>
Mon, 08 May 2017 16:04:31 +0800
changeset 364579 cde7eed4820808907b2102cb177e7744d95bb804
parent 364578 e658e0254df8adb0b6b4a3371b7dab8f0aad2b00
child 364580 064975f358bc7efe7a6da2fcb1583e0ba1f65698
push id32049
push usercbook@mozilla.com
push dateMon, 19 Jun 2017 11:36:23 +0000
treeherdermozilla-central@26d62a1ac0e3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1352306
milestone56.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 1352306 - Part 1: stylo: Only snapshot attributes if there is some rule that depends on that attribute. r=emilio MozReview-Commit-ID: Emey96ovc2a
layout/base/ServoRestyleManager.cpp
layout/reftests/bugs/1352306-1-ref.html
layout/reftests/bugs/1352306-1.html
layout/reftests/bugs/reftest.list
layout/style/ServoBindingList.h
layout/style/ServoStyleSet.cpp
layout/style/ServoStyleSet.h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -810,35 +810,46 @@ ServoRestyleManager::AttributeWillChange
                                          const nsAttrValue* aNewValue)
 {
   MOZ_ASSERT(!mInStyleRefresh);
 
   if (!aElement->HasServoData()) {
     return;
   }
 
+  bool influencesOtherPseudoClassState =
+    AttributeInfluencesOtherPseudoClassState(aElement, aAttribute);
+
+  if (!influencesOtherPseudoClassState &&
+      !((aNameSpaceID == kNameSpaceID_None &&
+         (aAttribute == nsGkAtoms::id ||
+          aAttribute == nsGkAtoms::_class)) ||
+        aAttribute == nsGkAtoms::lang ||
+        StyleSet()->MightHaveAttributeDependency(aAttribute))) {
+    return;
+  }
+
   ServoElementSnapshot& snapshot = SnapshotFor(aElement);
   snapshot.AddAttrs(aElement, aNameSpaceID, aAttribute);
 
-  if (AttributeInfluencesOtherPseudoClassState(aElement, aAttribute)) {
+  if (influencesOtherPseudoClassState) {
     snapshot.AddOtherPseudoClassState(aElement);
   }
 
   if (Element* parent = aElement->GetFlattenedTreeParentElementForStyle()) {
     parent->NoteDirtyDescendantsForServo();
   }
 }
 
 void
 ServoRestyleManager::AttributeChanged(Element* aElement, int32_t aNameSpaceID,
                                       nsIAtom* aAttribute, int32_t aModType,
                                       const nsAttrValue* aOldValue)
 {
   MOZ_ASSERT(!mInStyleRefresh);
-  MOZ_ASSERT(!mSnapshots.Get(aElement) || mSnapshots.Get(aElement)->HasAttrs());
 
   nsIFrame* primaryFrame = aElement->GetPrimaryFrame();
   if (primaryFrame) {
     primaryFrame->AttributeChanged(aNameSpaceID, aAttribute, aModType);
   }
 
   nsChangeHint hint = aElement->GetAttributeChangeHint(aAttribute, aModType);
   if (hint) {
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1352306-1-ref.html
@@ -0,0 +1,4 @@
+<!doctype html>
+<div id="container" ng-cloak>
+  Should show up
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1352306-1.html
@@ -0,0 +1,20 @@
+<!doctype html>
+<div id="container" ng-cloak>
+  Should show up
+</div>
+<style>
+[ng-cloak] {
+ display:none !important;
+}
+</style>
+<script>
+document.body.offsetTop;
+</script>
+<style>
+.ng-foo {
+  color: red;
+}
+</style>
+<script>
+container.removeAttribute("ng-cloak");
+</script>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -2001,14 +2001,15 @@ fails-if(styloVsGecko) == 1322512-1.html
 == 1364280-1.html 1364280-1-ref.html
 == 1364280-2a.html 1364280-2-ref.html
 == 1364280-2b.html 1364280-2-ref.html
 == 1364280-2c.html 1364280-2-ref.html
 == 1364335.html 1364335-ref.html
 == 1364360-1.html 1364360-1-ref.html
 == 1365159-1.html 1365159-1-ref.html
 fails-if(!stylo||styloVsGecko) == 1365162-1.html 1365162-1-ref.html
+== 1352306-1.html 1352306-1-ref.html
 == 1366144.html 1366144-ref.html
 == 1367592-1.html 1367592-1-ref.html
 == 1368113-1.html 1368113-1-ref.html
 == 1369584-1a.html 1369584-1-ref.html
 == 1369584-1b.html 1369584-1-ref.html
 == 1369954-1.xhtml 1369954-1-ref.xhtml
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -91,16 +91,19 @@ SERVO_BINDING_FUNC(Servo_StyleSet_GetFon
                    RawGeckoFontFaceRuleListBorrowedMut list)
 SERVO_BINDING_FUNC(Servo_StyleSet_GetCounterStyleRule, nsCSSCounterStyleRule*,
                    RawServoStyleSetBorrowed set, nsIAtom* name)
 SERVO_BINDING_FUNC(Servo_StyleSet_ResolveForDeclarations,
                    ServoComputedValuesStrong,
                    RawServoStyleSetBorrowed set,
                    ServoComputedValuesBorrowedOrNull parent_style,
                    RawServoDeclarationBlockBorrowed declarations)
+SERVO_BINDING_FUNC(Servo_StyleSet_MightHaveAttributeDependency, bool,
+                   RawServoStyleSetBorrowed set,
+                   nsIAtom* local_name)
 
 // CSSRuleList
 SERVO_BINDING_FUNC(Servo_CssRules_ListTypes, void,
                    ServoCssRulesBorrowed rules,
                    nsTArrayBorrowed_uintptr_t result)
 SERVO_BINDING_FUNC(Servo_CssRules_InsertRule, nsresult,
                    ServoCssRulesBorrowed rules,
                    RawServoStyleSheetBorrowed sheet, const nsACString* rule,
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -1351,9 +1351,15 @@ ServoStyleSet::RunPostTraversalTasks()
   nsTArray<PostTraversalTask> tasks;
   tasks.SwapElements(mPostTraversalTasks);
 
   for (auto& task : tasks) {
     task.Run();
   }
 }
 
+bool
+ServoStyleSet::MightHaveAttributeDependency(nsIAtom* aAttribute)
+{
+  return Servo_StyleSet_MightHaveAttributeDependency(mRawSet.get(), aAttribute);
+}
+
 ServoStyleSet* ServoStyleSet::sInServoTraversal = nullptr;
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -427,16 +427,26 @@ public:
   bool EnsureUniqueInnerOnCSSSheets();
 
   // Called by StyleSheet::EnsureUniqueInner to let us know it cloned
   // its inner.
   void SetNeedsRestyleAfterEnsureUniqueInner() {
     mNeedsRestyleAfterEnsureUniqueInner = true;
   }
 
+  /**
+   * Returns true if a modification to an an attribute with the specified
+   * local name might require us to restyle the element.
+   *
+   * This function allows us to skip taking a an attribute snapshot when
+   * the modified attribute doesn't appear in an attribute selector in
+   * a style sheet.
+   */
+  bool MightHaveAttributeDependency(nsIAtom* aAttribute);
+
 private:
   // On construction, sets sInServoTraversal to the given ServoStyleSet.
   // On destruction, clears sInServoTraversal and calls RunPostTraversalTasks.
   class MOZ_STACK_CLASS AutoSetInServoTraversal
   {
   public:
     explicit AutoSetInServoTraversal(ServoStyleSet* aSet)
       : mSet(aSet)