Bug 1360508: Allow fixups on text styles to be reflected. r?heycam
MozReview-Commit-ID: Eh6shYiv4RC
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -626,59 +626,41 @@ private:
GetStyle##name_<aComputeData>(this, mBits); \
/* 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; \
} \
/** \
- * Always forward to the parent to grab the inherited style struct\
- * we're a text node. \
+ * Also (conservatively) set the owning bit in the parent style \
+ * context 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. \
+ * Note, however, that we still need to request the style struct \
+ * of the text node itself, since we may run some fixups on it, \
+ * like for text-combine. \
* \
- * 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. \
+ * This model is sound because for the fixed-up values to change, \
+ * other properties on the parent need to change too, and we'll \
+ * handle those change hints correctly. \
* \
- * 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. \
+ * TODO(emilio): Perhaps we should remove those fixups and handle \
+ * those in layout instead. Those fixups are kind of expensive \
+ * for style sharing, and computed style of text nodes is not \
+ * observable. If we do that, we could assert here that the \
+ * inherited structs of both are the same. \
*/ \
- if (mPseudoTag == nsCSSAnonBoxes::mozText) { \
+ if (mPseudoTag == nsCSSAnonBoxes::mozText && aComputeData) { \
MOZ_ASSERT(mParent); \
- const nsStyle##name_* data = \
- mParent->DoGetStyle##name_<aComputeData>(); \
- NS_WARNING_ASSERTION(!data || \
- data == Servo_GetStyle##name_(mSource.AsServoComputedValues()), \
- "bad data"); \
- return data; \
+ mParent->AddStyleBit(NS_STYLE_INHERIT_BIT(name_)); \
} \
\
const bool needToCompute = !(mBits & NS_STYLE_INHERIT_BIT(name_));\
if (!aComputeData && needToCompute) { \
return nullptr; \
} \
\
const nsStyle##name_* data = \