Bug 1188721 - Part 13: Use style struct refcount rather than mBits to determine whether a cached struct is safe to return from GetUniqueStyleData. r?dbaron draft
authorCameron McCormack <cam@mcc.id.au>
Wed, 23 Mar 2016 17:35:59 +1100
changeset 343763 af51dc1e4396225a81ac8f9696d5538bc80f81af
parent 343762 d07c4a75f2ae39e30b578161c3fdcb1af6be143f
child 343764 459325bd1a2befed2801fb96c68fb03b89c579d8
push id13680
push usercmccormack@mozilla.com
push dateWed, 23 Mar 2016 06:36:18 +0000
reviewersdbaron
bugs1188721
milestone48.0a1
Bug 1188721 - Part 13: Use style struct refcount rather than mBits to determine whether a cached struct is safe to return from GetUniqueStyleData. r?dbaron MozReview-Commit-ID: LjDJgGKdDVH
layout/style/nsStyleContext.cpp
layout/style/nsStyleStruct.h
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -431,26 +431,25 @@ nsStyleContext::StyleData(nsStyleStructI
 }
 
 // This is an evil evil function, since it forces you to alloc your own separate copy of
 // style data!  Do not use this function unless you absolutely have to!  You should avoid
 // this at all costs! -dwh
 nsStyleStruct*
 nsStyleContext::GetUniqueStyleData(const nsStyleStructID& aSID)
 {
-  // If we already own the struct and no kids could depend on it, then
-  // just return it.  (We leak in this case if there are kids -- and this
-  // function really shouldn't be called for style contexts that could
-  // have kids depending on the data.  ClearStyleData would be OK, but
-  // this test for no mChild or mEmptyChild doesn't catch that case.)
+  // If we already have a cached struct and no kids could depend on it, then
+  // just return it.
   auto current = const_cast<nsStyleStruct*>(StyleData(aSID));
-  if (!mChild && !mEmptyChild &&
-      !(mBits & nsCachedStyleData::GetBitForSID(aSID)) &&
-      GetCachedStyleData(aSID))
-    return current;
+  if (!mChild && !mEmptyChild) {
+    const nsStyleStruct* data = GetCachedStyleData(aSID);
+    if (data && data->HasSingleReference()) {
+      return current;
+    }
+  }
 
   nsStyleStruct* newData;
   nsPresContext *presContext = PresContext();
   switch (aSID) {
 
 #define UNIQUE_CASE(c_)                                                       \
   case eStyleStruct_##c_: {                                                   \
     auto currentData = static_cast<nsStyle##c_*>(current);                    \
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -127,16 +127,18 @@ public:
     static DestroyFn sDestroyFn[] = {
 #define STYLE_STRUCT(name_, checkdata_cb_) &Destroy##name_,
 #include "nsStyleStructList.h"
 #undef STYLE_STRUCT
     };
     sDestroyFn[aSID](aData, aPresContext);
   }
 
+  bool HasSingleReference() const { return mRefCnt == 1; }
+
 #ifdef DEBUG
   MozRefCountType GetRefCnt() const { return mRefCnt; }
 #endif
 
 protected:
   nsStyleStruct() : mRefCnt(0) {}
   nsStyleStruct(const nsStyleStruct& aOther) {}
   nsStyleStruct& operator=(const nsStyleStruct& aOther) {