Bug 1360508: Allow fixups on text styles to be reflected. rpending=heycam a=orange
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 28 Apr 2017 15:03:05 +0200
changeset 403690 766e80198c61a9001fbf551a200e9ee762c666ba
parent 403689 7ac1b8bcb88f5ce4f2644d195af6608d304daa6d
child 403691 043cdedf4903ad93f6e43105f7455349ab608a44
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersorange
bugs1360508
milestone55.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 1360508: Allow fixups on text styles to be reflected. rpending=heycam a=orange MozReview-Commit-ID: Eh6shYiv4RC Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
layout/style/nsStyleContext.h
--- 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 =                                      \