Bug 1292930: stylo: Store the change hint generated by non-elements in their parent element. draft
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Sat, 06 Aug 2016 13:34:53 -0700
changeset 397592 926026dc647c51fc6725f16a79f585128590f494
parent 397591 b151ff1f8a825f66d45017d6dd7a06eb7a257c3b
child 397593 c44d8a7e52854ff0364ab05537d8afb6369e296b
push id25337
push userbmo:ealvarez@mozilla.com
push dateSat, 06 Aug 2016 22:18:29 +0000
bugs1292930
milestone51.0a1
Bug 1292930: stylo: Store the change hint generated by non-elements in their parent element. Otherwise, the parent style context doesn't have some inherited structs in the cache, so it avoids comparing them entirely. For example, the following example: <p>Hey</p> p { color: blue } p:hover { color: red } Wouldn't work as intended, because when calculating the change hint the nsStyleColor struct in the element hasn't been accessed by layout (only the child text frame's has), so it will report a change hint of 0 when hovering over the paragraph, and we would ignore the nsChangeHint_ReconstructFrame generated by the text node. MozReview-Commit-ID: FW7Thhuh7LG
layout/style/ServoBindings.cpp
layout/style/nsStyleContext.h
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -219,22 +219,32 @@ Gecko_CalcStyleDifference(nsStyleContext
 
   return result;
 }
 
 void
 Gecko_StoreStyleDifference(RawGeckoNode* aNode, nsChangeHint aChangeHintToStore)
 {
 #ifdef MOZ_STYLO
-  // XXXEmilio probably storing it in the nearest content parent is a sane thing
-  // to do if this case can ever happen?
   MOZ_ASSERT(aNode->IsContent());
+  MOZ_ASSERT(aNode->IsDirtyForServo(),
+             "Change hint stored in a not-dirty node");
 
-  nsIContent* aContent = aNode->AsContent();
-  nsIFrame* primaryFrame = aContent->GetPrimaryFrame();
+  // For elements, we need to store the change hint in the proper style context.
+  // For nodes, we want to store the change hint in the parent element, since
+  // Gecko's change list only operates on Elements, and we'll fail to compute
+  // the change hint for the element properly because the property won't always
+  // be in the cache of the parent's nsStyleContext.
+  //
+  // For Gecko this is not a problem, presumably because they access the
+  // inherited structs from the parent style context to inherit them?
+  Element* aElement =
+    aNode->IsElement() ? aNode->AsElement() : aNode->GetParentElement();
+
+  nsIFrame* primaryFrame = aElement->GetPrimaryFrame();
   if (!primaryFrame) {
     // TODO: Pick the undisplayed content map from the frame-constructor, and
     // stick it there. For now we're generating ReconstructFrame
     // unconditionally, which is suboptimal.
     return;
   }
 
   primaryFrame->StyleContext()->StoreChangeHint(aChangeHintToStore);
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -529,17 +529,17 @@ public:
   // NOTE: It'd be great to assert here that the previous change hint is always
   // consumed.
   //
   // This is not the case right now, since the changes of childs of frames that
   // go through frame construction are not consumed.
   void StoreChangeHint(nsChangeHint aHint)
   {
     MOZ_ASSERT(!IsShared());
-    mStoredChangeHint = aHint;
+    mStoredChangeHint |= aHint;
 #ifdef DEBUG
     mConsumedChangeHint = false;
 #endif
   }
 
   nsChangeHint ConsumeStoredChangeHint()
   {
     MOZ_ASSERT(!mConsumedChangeHint, "Re-consuming the same change hint!");