Bug 1296556: Reach the parent to compute inherited style structs. r=heycam
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Fri, 19 Aug 2016 14:32:51 -0700
changeset 312450 1bb4fec092645bd00840ec15605444921ffa3749
parent 312449 adc017f6ddf8e6bd8604b3cef97d513e95322a64
child 312451 774608c1757086d7abebea577ac6e561a22b65c7
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1296556
milestone51.0a1
Bug 1296556: Reach the parent to compute inherited style structs. r=heycam It's a shame that assertion can be made a crashing one, but unfortunately it was hitting me regularly. MozReview-Commit-ID: 4iZf1rm1fO2
layout/style/nsStyleContext.h
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -6,16 +6,17 @@
 /* the interface (to internal code) for retrieving computed style data */
 
 #ifndef _nsStyleContext_h_
 #define _nsStyleContext_h_
 
 #include "mozilla/Assertions.h"
 #include "mozilla/RestyleLogging.h"
 #include "mozilla/StyleContextSource.h"
+#include "nsCSSAnonBoxes.h"
 #include "nsStyleSet.h"
 
 class nsIAtom;
 class nsPresContext;
 
 namespace mozilla {
 enum class CSSPseudoElementType : uint8_t;
 } // namespace mozilla
@@ -650,18 +651,61 @@ private:
       }                                                                 \
       /* Have the rulenode deal */                                      \
       AUTO_CHECK_DEPENDENCY(eStyleStruct_##name_);                      \
       const nsStyle##name_ * newData;                                   \
       if (mSource.IsGeckoRuleNode()) {                                  \
         newData = mSource.AsGeckoRuleNode()->                           \
           GetStyle##name_<aComputeData>(this, mBits);                   \
       } else {                                                          \
-        newData =                                                       \
-          Servo_GetStyle##name_(mSource.AsServoComputedValues());       \
+        /**                                                             \
+         * Reach the parent to grab the inherited style struct if       \
+         * we're a text node.                                           \
+         *                                                              \
+         * This causes the parent element's style context to cache any  \
+         * inherited structs we request for a text node, which means we \
+         * don't have to compute change hints for the text node, as     \
+         * handling the change on the parent element is sufficient.     \
+         *                                                              \
+         * Note that adding the inherit bit is ok, because the struct   \
+         * pointer returned by the parent and the child is owned by     \
+         * Servo. This is fine if the pointers are the same (as it      \
+         * should, read below), because both style context sources will \
+         * hold it.                                                     \
+         *                                                              \
+         * In the case of a mishandled frame, we could end up with the  \
+         * pointer to and old parent style, but that's fine too, since  \
+         * the parent style context will remain alive until we reframe, \
+         * in which case we'll discard both style contexts. Also, we    \
+         * hold a strong reference to the parent style context, which   \
+         * makes it a non-issue.                                        \
+         *                                                              \
+         * Also, note that the assertion below should be true, except   \
+         * for those frames we still don't handle correctly, like       \
+         * anonymous table wrappers, in which case the pointers will    \
+         * differ.                                                      \
+         *                                                              \
+         * That means we're not going to restyle correctly text frames  \
+         * of anonymous table wrappers, for example. It's kind of       \
+         * embarrassing, but I think it's not worth it to add more      \
+         * logic here unconditionally, given that's going to be fixed.  \
+         *                                                              \
+         * TODO(emilio): Convert to a strong assertion once we support  \
+         * all kinds of random frames. In fact, this can be a great     \
+         * assertion to debug them.                                     \
+         */                                                             \
+        if (mPseudoTag == nsCSSAnonBoxes::mozText) {                    \
+          MOZ_ASSERT(mParent);                                          \
+          newData = mParent->DoGetStyle##name_<true>();                 \
+          NS_WARN_IF(newData !=                                         \
+            Servo_GetStyle##name_(mSource.AsServoComputedValues()));    \
+        } else {                                                        \
+          newData =                                                     \
+            Servo_GetStyle##name_(mSource.AsServoComputedValues());     \
+        }                                                               \
         /* the Servo-backed StyleContextSource owns the struct */       \
         AddStyleBit(NS_STYLE_INHERIT_BIT(name_));                       \
       }                                                                 \
       /* always cache inherited data on the style context; the rule */  \
       /* node set the bit in mBits for us if needed. */                 \
       mCachedInheritedData.mStyleStructs[eStyleStruct_##name_] =        \
         const_cast<nsStyle##name_ *>(newData);                          \
       return newData;                                                   \