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
--- 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; \