Bug 1380133 - Part 4: Make CalcStyleDifferenceInternal not cache any new structs on ServoStyleContexts when in a traversal. r=emilio draft
authorCameron McCormack <cam@mcc.id.au>
Fri, 21 Jul 2017 11:42:43 +0800
changeset 612843 b77d33b5823eca4a57e568881c00959ddecf9089
parent 612842 63e1e4fba69c587f7eb403efd7d8dd98214c86ac
child 612844 88576e17cd3867acdf8862dab42fd4b1e73bc755
push id69614
push userbmo:cam@mcc.id.au
push dateFri, 21 Jul 2017 03:44:21 +0000
reviewersemilio
bugs1380133
milestone56.0a1
Bug 1380133 - Part 4: Make CalcStyleDifferenceInternal not cache any new structs on ServoStyleContexts when in a traversal. r=emilio MozReview-Commit-ID: Eu4MvdQUBor --- layout/generic/nsFrame.cpp | 8 ++++++++ layout/style/nsStyleContext.cpp | 11 +++++++---- layout/tables/nsTableFrame.cpp | 8 ++++++++ 3 files changed, 23 insertions(+), 4 deletions(-)
layout/generic/nsFrame.cpp
layout/style/nsStyleContext.cpp
layout/tables/nsTableFrame.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -10267,16 +10267,24 @@ nsIFrame::UpdateStyleOfOwnedChildFrame(
   // 2) Content can change stylesheets that change the styles of pseudos, and
   //    extensions can add/remove stylesheets that change the styles of
   //    anonymous boxes directly.
   uint32_t equalStructs, samePointerStructs; // Not used, actually.
   nsChangeHint childHint = aChildFrame->StyleContext()->CalcStyleDifference(
     aNewStyleContext,
     &equalStructs,
     &samePointerStructs);
+
+  // CalcStyleDifference will handle caching structs on the new style context,
+  // but only if we're not on a style worker thread.
+  MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal(),
+             "if we can get in here from style worker threads, then we need "
+             "a ResolveSameStructsAs call to ensure structs are cached on "
+             "aNewStyleContext");
+
   // If aChildFrame is out of flow, then aRestyleState's "changes handled by the
   // parent" doesn't apply to it, because it may have some other parent in the
   // frame tree.
   if (!aChildFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)) {
     childHint = NS_RemoveSubsumedHints(
       childHint, aRestyleState.ChangesHandledFor(*aChildFrame));
   }
   if (childHint) {
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -253,17 +253,18 @@ nsStyleContext::CalcStyleDifferenceInter
     } else if (checkUnrequestedServoStructs) {                                \
       this##struct_ =                                                         \
         AsServo()->ComputedValues()->GetStyle##struct_();                     \
       unrequestedStruct = true;                                               \
     } else {                                                                  \
       unrequestedStruct = false;                                              \
     }                                                                         \
     if (this##struct_) {                                                      \
-      const nsStyle##struct_* other##struct_ = aNewContext->Style##struct_(); \
+      const nsStyle##struct_* other##struct_ =                                \
+        aNewContext->ThreadsafeStyle##struct_();                              \
       if (this##struct_ == other##struct_) {                                  \
         /* The very same struct, so we know that there will be no */          \
         /* differences.                                           */          \
         *aEqualStructs |= NS_STYLE_INHERIT_BIT(struct_);                      \
       } else {                                                                \
         nsChangeHint difference =                                             \
           this##struct_->CalcDifference(*other##struct_ EXPAND extra_args_);  \
         if (!unrequestedStruct) {                                             \
@@ -333,17 +334,17 @@ nsStyleContext::CalcStyleDifferenceInter
   // struct.)  This is important for callers in RestyleManager that
   // need to know the equality or not of the final set of cached struct
   // pointers.
   *aSamePointerStructs = 0;
 
 #define STYLE_STRUCT(name_, callback_)                                        \
   {                                                                           \
     const nsStyle##name_* data = PeekStyle##name_();                          \
-    if (!data || data == aNewContext->Style##name_()) {                       \
+    if (!data || data == aNewContext->ThreadsafeStyle##name_()) {             \
       *aSamePointerStructs |= NS_STYLE_INHERIT_BIT(name_);                    \
     }                                                                         \
   }
 #include "nsStyleStructList.h"
 #undef STYLE_STRUCT
 
   // Note that we do not check whether this->RelevantLinkVisited() !=
   // aNewContext->RelevantLinkVisited(); we don't need to since
@@ -382,18 +383,20 @@ nsStyleContext::CalcStyleDifferenceInter
     // NB: Calling Peek on |this|, not |thisVis|, since callers may look
     // at a struct on |this| without looking at the same struct on
     // |thisVis| (including this function if we skip one of these checks
     // due to change being true already or due to the old style context
     // not having a style-if-visited), but not the other way around.
 #define STYLE_FIELD(name_) thisVisStruct->name_ != otherVisStruct->name_
 #define STYLE_STRUCT(name_, fields_)                                    \
     if (!change && PeekStyle##name_()) {                                \
-      const nsStyle##name_* thisVisStruct = thisVis->Style##name_();    \
-      const nsStyle##name_* otherVisStruct = otherVis->Style##name_();  \
+      const nsStyle##name_* thisVisStruct =                             \
+        thisVis->ThreadsafeStyle##name_();                              \
+      const nsStyle##name_* otherVisStruct =                            \
+        otherVis->ThreadsafeStyle##name_();                             \
       if (MOZ_FOR_EACH_SEPARATED(STYLE_FIELD, (||), (), fields_)) {     \
         change = true;                                                  \
       }                                                                 \
     }
 #include "nsCSSVisitedDependentPropList.h"
 #undef STYLE_STRUCT
 #undef STYLE_FIELD
 
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -8062,16 +8062,24 @@ nsTableFrame::UpdateStyleOfOwnedAnonBoxe
   // NOTE(emilio): We can't use the ChangesHandledFor optimization (and we
   // assert against that), because the table wrapper is up in the frame tree
   // compared to the owner frame.
   uint32_t equalStructs, samePointerStructs; // Not used, actually.
   nsChangeHint wrapperHint = aWrapperFrame->StyleContext()->CalcStyleDifference(
     newContext,
     &equalStructs,
     &samePointerStructs);
+
+  // CalcStyleDifference will handle caching structs on the new style context,
+  // but only if we're not on a style worker thread.
+  MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal(),
+             "if we can get in here from style worker threads, then we need "
+             "a ResolveSameStructsAs call to ensure structs are cached on "
+             "aNewStyleContext");
+
   if (wrapperHint) {
     aRestyleState.ChangeList().AppendChange(
       aWrapperFrame, aWrapperFrame->GetContent(), wrapperHint);
   }
 
   for (nsIFrame* cur = aWrapperFrame; cur; cur = cur->GetNextContinuation()) {
     cur->SetStyleContext(newContext);
   }