Bug 1261754 - Part 2: Make quotes computed values shareable between different structs. r?dholbert draft
authorCameron McCormack <cam@mcc.id.au>
Tue, 12 Apr 2016 11:35:26 +1000
changeset 349671 67e34ee48f8398804bb850b88a24464a9be90b8c
parent 349670 8a0c1afec0a4ea5c378c00d844f9995b2a3619ab
child 349672 782bc72d52df922c5527fa67abf02cca247807ab
push id15154
push usercmccormack@mozilla.com
push dateTue, 12 Apr 2016 01:46:39 +0000
reviewersdholbert
bugs1261754
milestone48.0a1
Bug 1261754 - Part 2: Make quotes computed values shareable between different structs. r?dholbert MozReview-Commit-ID: 4uzh1jilABa
layout/base/nsLayoutUtils.cpp
layout/base/nsQuoteList.cpp
layout/style/nsComputedDOMStyle.cpp
layout/style/nsRuleNode.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -7656,16 +7656,19 @@ nsLayoutUtils::Shutdown()
     sContentMap = nullptr;
   }
 
   Preferences::UnregisterCallback(GridEnabledPrefChangeCallback,
                                   GRID_ENABLED_PREF_NAME);
   Preferences::UnregisterCallback(WebkitPrefixEnabledPrefChangeCallback,
                                   WEBKIT_PREFIXES_ENABLED_PREF_NAME);
   nsComputedDOMStyle::UnregisterPrefChangeCallbacks();
+
+  // so the cached initial quotes array doesn't appear to be a leak
+  nsStyleQuotes::Shutdown();
 }
 
 /* static */
 void
 nsLayoutUtils::RegisterImageRequest(nsPresContext* aPresContext,
                                     imgIRequest* aRequest,
                                     bool* aRequestRegistered)
 {
--- a/layout/base/nsQuoteList.cpp
+++ b/layout/base/nsQuoteList.cpp
@@ -32,35 +32,36 @@ nsQuoteNode::InitTextFrame(nsGenConList*
 }
 
 const nsString*
 nsQuoteNode::Text()
 {
   NS_ASSERTION(mType == eStyleContentType_OpenQuote ||
                mType == eStyleContentType_CloseQuote,
                "should only be called when mText should be non-null");
-  const nsStyleQuotes* styleQuotes = mPseudoFrame->StyleQuotes();
-  int32_t quotesCount = styleQuotes->QuotesCount(); // 0 if 'quotes:none'
+  const nsStyleQuoteValues::Array& quotePairs =
+    mPseudoFrame->StyleQuotes()->GetQuotePairs();
+  int32_t quotesCount = quotePairs.Length(); // 0 if 'quotes:none'
   int32_t quoteDepth = Depth();
 
   // Reuse the last pair when the depth is greater than the number of
   // pairs of quotes.  (Also make 'quotes: none' and close-quote from
   // a depth of 0 equivalent for the next test.)
   if (quoteDepth >= quotesCount)
     quoteDepth = quotesCount - 1;
 
-  const nsString *result;
+  const nsString* result;
   if (quoteDepth == -1) {
     // close-quote from a depth of 0 or 'quotes: none' (we want a node
     // with the empty string so dynamic changes are easier to handle)
-    result = & EmptyString();
+    result = &EmptyString();
   } else {
     result = eStyleContentType_OpenQuote == mType
-               ? styleQuotes->OpenQuoteAt(quoteDepth)
-               : styleQuotes->CloseQuoteAt(quoteDepth);
+               ? &quotePairs[quoteDepth].first
+               : &quotePairs[quoteDepth].second;
   }
   return result;
 }
 
 void
 nsQuoteList::Calc(nsQuoteNode* aNode)
 {
   if (aNode == FirstNode()) {
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -1466,35 +1466,35 @@ nsComputedDOMStyle::DoGetCounterReset()
   }
 
   return valueList.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetQuotes()
 {
-  const nsStyleQuotes *quotes = StyleQuotes();
-
-  if (quotes->QuotesCount() == 0) {
+  const auto& quotePairs = StyleQuotes()->GetQuotePairs();
+
+  if (quotePairs.IsEmpty()) {
     RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
     val->SetIdent(eCSSKeyword_none);
     return val.forget();
   }
 
   RefPtr<nsDOMCSSValueList> valueList = GetROCSSValueList(false);
 
-  for (uint32_t i = 0, i_end = quotes->QuotesCount(); i < i_end; ++i) {
+  for (const auto& quotePair : quotePairs) {
     RefPtr<nsROCSSPrimitiveValue> openVal = new nsROCSSPrimitiveValue;
     RefPtr<nsROCSSPrimitiveValue> closeVal = new nsROCSSPrimitiveValue;
 
     nsString s;
-    nsStyleUtil::AppendEscapedCSSString(*quotes->OpenQuoteAt(i), s);
+    nsStyleUtil::AppendEscapedCSSString(quotePair.first, s);
     openVal->SetString(s);
     s.Truncate();
-    nsStyleUtil::AppendEscapedCSSString(*quotes->CloseQuoteAt(i), s);
+    nsStyleUtil::AppendEscapedCSSString(quotePair.second, s);
     closeVal->SetString(s);
 
     valueList->AppendCSSValue(openVal.forget());
     valueList->AppendCSSValue(closeVal.forget());
   }
 
   return valueList.forget();
 }
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -8855,44 +8855,43 @@ nsRuleNode::ComputeQuotesData(void* aSta
   // quotes: inherit, initial, none, [string string]+
   const nsCSSValue* quotesValue = aRuleData->ValueForQuotes();
   switch (quotesValue->GetUnit()) {
   case eCSSUnit_Null:
     break;
   case eCSSUnit_Inherit:
   case eCSSUnit_Unset:
     conditions.SetUncacheable();
-    quotes->CopyFrom(*parentQuotes);
+    quotes->SetQuotesInherit(parentQuotes);
     break;
   case eCSSUnit_Initial:
-    quotes->SetInitial();
+    quotes->SetQuotesInitial();
     break;
   case eCSSUnit_None:
-    quotes->AllocateQuotes(0);
+    quotes->SetQuotesNone();
     break;
   case eCSSUnit_PairList:
   case eCSSUnit_PairListDep: {
-    const nsCSSValuePairList* ourQuotes
-      = quotesValue->GetPairListValue();
+    const nsCSSValuePairList* ourQuotes = quotesValue->GetPairListValue();
+
+    nsStyleQuoteValues::Array quotePairs;
+    quotePairs.SetLength(ListLength(ourQuotes));
+
+    size_t index = 0;
     nsAutoString buffer;
-    nsAutoString closeBuffer;
-    uint32_t count = ListLength(ourQuotes);
-    if (NS_FAILED(quotes->AllocateQuotes(count))) {
-      break;
-    }
-    count = 0;
     while (ourQuotes) {
       MOZ_ASSERT(ourQuotes->mXValue.GetUnit() == eCSSUnit_String &&
                  ourQuotes->mYValue.GetUnit() == eCSSUnit_String,
                  "improper list contents for quotes");
-      ourQuotes->mXValue.GetStringValue(buffer);
-      ourQuotes->mYValue.GetStringValue(closeBuffer);
-      quotes->SetQuotesAt(count++, buffer, closeBuffer);
+      quotePairs[index].first  = ourQuotes->mXValue.GetStringValue(buffer);
+      quotePairs[index].second = ourQuotes->mYValue.GetStringValue(buffer);
+      ++index;
       ourQuotes = ourQuotes->mNext;
     }
+    quotes->SetQuotes(Move(quotePairs));
     break;
   }
   default:
     MOZ_ASSERT(false, "unexpected value unit");
   }
 
   COMPUTE_END_INHERITED(Quotes, quotes)
 }
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3492,85 +3492,103 @@ nsresult nsStyleContent::AllocateContent
   return NS_OK;
 }
 
 // ---------------------
 // nsStyleQuotes
 //
 
 nsStyleQuotes::nsStyleQuotes(StyleStructContext aContext)
-  : mQuotesCount(0),
-    mQuotes(nullptr)
 {
   MOZ_COUNT_CTOR(nsStyleQuotes);
-  SetInitial();
+  SetQuotesInitial();
 }
 
-nsStyleQuotes::~nsStyleQuotes(void)
+nsStyleQuotes::~nsStyleQuotes()
 {
   MOZ_COUNT_DTOR(nsStyleQuotes);
-  DELETE_ARRAY_IF(mQuotes);
 }
 
 nsStyleQuotes::nsStyleQuotes(const nsStyleQuotes& aSource)
-  : mQuotesCount(0),
-    mQuotes(nullptr)
+  : mQuotes(aSource.mQuotes)
 {
   MOZ_COUNT_CTOR(nsStyleQuotes);
-  CopyFrom(aSource);
+}
+
+void
+nsStyleQuotes::SetQuotesInherit(const nsStyleQuotes* aOther)
+{
+  mQuotes = aOther->mQuotes;
 }
 
 void
-nsStyleQuotes::SetInitial()
+nsStyleQuotes::SetQuotesInitial()
 {
-  // The initial value for quotes is the en-US typographic convention:
-  // outermost are LEFT and RIGHT DOUBLE QUOTATION MARK, alternating
-  // with LEFT and RIGHT SINGLE QUOTATION MARK.
-  static const char16_t initialQuotes[8] = {
-    0x201C, 0, 0x201D, 0, 0x2018, 0, 0x2019, 0
-  };
-  
-  if (NS_SUCCEEDED(AllocateQuotes(2))) {
-    SetQuotesAt(0,
-                nsDependentString(&initialQuotes[0], 1),
-                nsDependentString(&initialQuotes[2], 1));
-    SetQuotesAt(1,
-                nsDependentString(&initialQuotes[4], 1),
-                nsDependentString(&initialQuotes[6], 1));
+  if (!sInitialQuotes) {
+    // The initial value for quotes is the en-US typographic convention:
+    // outermost are LEFT and RIGHT DOUBLE QUOTATION MARK, alternating
+    // with LEFT and RIGHT SINGLE QUOTATION MARK.
+    static const char16_t initialQuotes[8] = {
+      0x201C, 0, 0x201D, 0, 0x2018, 0, 0x2019, 0
+    };
+
+    sInitialQuotes = new nsStyleQuoteValues;
+    sInitialQuotes->mQuotePairs.AppendElement(
+        std::make_pair(nsDependentString(&initialQuotes[0], 1),
+                       nsDependentString(&initialQuotes[2], 1)));
+    sInitialQuotes->mQuotePairs.AppendElement(
+        std::make_pair(nsDependentString(&initialQuotes[4], 1),
+                       nsDependentString(&initialQuotes[6], 1)));
   }
+
+  mQuotes = sInitialQuotes;
+}
+
+void
+nsStyleQuotes::SetQuotesNone()
+{
+  if (!sNoneQuotes) {
+    sNoneQuotes = new nsStyleQuoteValues;
+  }
+  mQuotes = sNoneQuotes;
 }
 
 void
-nsStyleQuotes::CopyFrom(const nsStyleQuotes& aSource)
+nsStyleQuotes::SetQuotes(nsStyleQuoteValues::Array&& aValues)
 {
-  if (NS_SUCCEEDED(AllocateQuotes(aSource.QuotesCount()))) {
-    uint32_t count = (mQuotesCount * 2);
-    for (uint32_t index = 0; index < count; index += 2) {
-      aSource.GetQuotesAt(index, mQuotes[index], mQuotes[index + 1]);
-    }
-  }
+  mQuotes = new nsStyleQuoteValues;
+  mQuotes->mQuotePairs = Move(aValues);
 }
 
-nsChangeHint nsStyleQuotes::CalcDifference(const nsStyleQuotes& aOther) const
+const nsStyleQuoteValues::Array&
+nsStyleQuotes::GetQuotePairs() const
+{
+  return mQuotes->mQuotePairs;
+}
+
+nsChangeHint
+nsStyleQuotes::CalcDifference(const nsStyleQuotes& aOther) const
 {
   // If the quotes implementation is ever going to change we might not need
   // a framechange here and a reflow should be sufficient.  See bug 35768.
-  if (mQuotesCount == aOther.mQuotesCount) {
-    uint32_t ix = (mQuotesCount * 2);
-    while (0 < ix--) {
-      if (mQuotes[ix] != aOther.mQuotes[ix]) {
-        return NS_STYLE_HINT_FRAMECHANGE;
-      }
-    }
-
-    return NS_STYLE_HINT_NONE;
+  if (mQuotes != aOther.mQuotes &&
+      mQuotes->mQuotePairs != aOther.mQuotes->mQuotePairs) {
+    return NS_STYLE_HINT_FRAMECHANGE;
   }
-  return NS_STYLE_HINT_FRAMECHANGE;
+
+  return NS_STYLE_HINT_NONE;
 }
 
+StaticRefPtr<nsStyleQuoteValues>
+nsStyleQuotes::sInitialQuotes;
+
+StaticRefPtr<nsStyleQuoteValues>
+nsStyleQuotes::sNoneQuotes;
+
+
 // --------------------
 // nsStyleTextReset
 //
 
 nsStyleTextReset::nsStyleTextReset(StyleStructContext aContext)
 { 
   MOZ_COUNT_CTOR(nsStyleTextReset);
   mVerticalAlign.SetIntValue(NS_STYLE_VERTICAL_ALIGN_BASELINE, eStyleUnit_Enumerated);
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -11,16 +11,17 @@
 
 #ifndef nsStyleStruct_h___
 #define nsStyleStruct_h___
 
 #include "mozilla/ArenaObjectID.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/CSSVariableValues.h"
 #include "mozilla/SheetType.h"
+#include "mozilla/StaticPtr.h"
 #include "mozilla/StyleStructContext.h"
 #include "nsColor.h"
 #include "nsCoord.h"
 #include "nsMargin.h"
 #include "nsFont.h"
 #include "nsStyleCoord.h"
 #include "nsStyleConsts.h"
 #include "nsChangeHint.h"
@@ -28,16 +29,17 @@
 #include "nsCOMPtr.h"
 #include "nsCOMArray.h"
 #include "nsTArray.h"
 #include "nsCSSValue.h"
 #include "imgRequestProxy.h"
 #include "Orientation.h"
 #include "CounterStyleManager.h"
 #include <cstddef> // offsetof()
+#include <utility>
 
 class nsIFrame;
 class nsIURI;
 class nsStyleContext;
 class nsTextFrame;
 class imgIContainer;
 struct nsStyleVisibility;
 
@@ -2791,98 +2793,77 @@ struct nsStyleCounterData
 {
   nsString  mCounter;
   int32_t   mValue;
 };
 
 
 #define DELETE_ARRAY_IF(array)  if (array) { delete[] array; array = nullptr; }
 
+/**
+ * An object that allows sharing of arrays that store 'quotes' property
+ * values.  This is particularly important for inheritance, where we want
+ * to share the same 'quotes' value with a parent style context.
+ */
+class nsStyleQuoteValues
+{
+public:
+  typedef nsTArray<std::pair<nsString, nsString>> Array;
+  NS_INLINE_DECL_REFCOUNTING(nsStyleQuoteValues);
+  Array mQuotePairs;
+
+private:
+  ~nsStyleQuoteValues() {}
+};
+
 struct nsStyleQuotes
 {
   explicit nsStyleQuotes(StyleStructContext aContext);
   nsStyleQuotes(const nsStyleQuotes& aQuotes);
   ~nsStyleQuotes();
 
   void* operator new(size_t sz, nsStyleQuotes* aSelf) CPP_THROW_NEW { return aSelf; }
   void* operator new(size_t sz, nsPresContext* aContext) CPP_THROW_NEW {
     return aContext->PresShell()->
       AllocateByObjectID(mozilla::eArenaObjectID_nsStyleQuotes, sz);
   }
   void Destroy(nsPresContext* aContext) {
     this->~nsStyleQuotes();
     aContext->PresShell()->
       FreeByObjectID(mozilla::eArenaObjectID_nsStyleQuotes, this);
   }
-
-  void SetInitial();
-  void CopyFrom(const nsStyleQuotes& aSource);
-
   nsChangeHint CalcDifference(const nsStyleQuotes& aOther) const;
   static nsChangeHint MaxDifference() {
     return NS_STYLE_HINT_FRAMECHANGE;
   }
   static nsChangeHint DifferenceAlwaysHandledForDescendants() {
     // CalcDifference never returns the reflow hints that are sometimes
     // handled for descendants as hints not handled for descendants.
     return nsChangeHint_NeedReflow |
            nsChangeHint_ReflowChangesSizeOrPosition |
            nsChangeHint_ClearAncestorIntrinsics;
   }
 
-  uint32_t  QuotesCount(void) const { return mQuotesCount; } // [inherited]
-
-  const nsString* OpenQuoteAt(uint32_t aIndex) const
-  {
-    NS_ASSERTION(aIndex < mQuotesCount, "out of range");
-    return mQuotes + (aIndex * 2);
-  }
-  const nsString* CloseQuoteAt(uint32_t aIndex) const
-  {
-    NS_ASSERTION(aIndex < mQuotesCount, "out of range");
-    return mQuotes + (aIndex * 2 + 1);
-  }
-  nsresult  GetQuotesAt(uint32_t aIndex, nsString& aOpen, nsString& aClose) const {
-    if (aIndex < mQuotesCount) {
-      aIndex *= 2;
-      aOpen = mQuotes[aIndex];
-      aClose = mQuotes[++aIndex];
-      return NS_OK;
-    }
-    return NS_ERROR_ILLEGAL_VALUE;
+  static void Shutdown() {
+    sInitialQuotes = nullptr;
+    sNoneQuotes = nullptr;
   }
 
-  nsresult  AllocateQuotes(uint32_t aCount) {
-    if (aCount != mQuotesCount) {
-      DELETE_ARRAY_IF(mQuotes);
-      if (aCount) {
-        mQuotes = new nsString[aCount * 2];
-        if (! mQuotes) {
-          mQuotesCount = 0;
-          return NS_ERROR_OUT_OF_MEMORY;
-        }
-      }
-      mQuotesCount = aCount;
-    }
-    return NS_OK;
-  }
-
-  nsresult  SetQuotesAt(uint32_t aIndex, const nsString& aOpen, const nsString& aClose) {
-    if (aIndex < mQuotesCount) {
-      aIndex *= 2;
-      mQuotes[aIndex] = aOpen;
-      mQuotes[++aIndex] = aClose;
-      return NS_OK;
-    }
-    return NS_ERROR_ILLEGAL_VALUE;
-  }
-
-protected:
-  uint32_t            mQuotesCount;
-  nsString*           mQuotes;
+  const nsStyleQuoteValues::Array& GetQuotePairs() const;
+
+  void SetQuotesInherit(const nsStyleQuotes* aOther);
+  void SetQuotesInitial();
+  void SetQuotesNone();
+  void SetQuotes(nsStyleQuoteValues::Array&& aValues);
+
+private:
+  RefPtr<nsStyleQuoteValues> mQuotes;  // [inherited]
+
+  static mozilla::StaticRefPtr<nsStyleQuoteValues> sInitialQuotes;
+  static mozilla::StaticRefPtr<nsStyleQuoteValues> sNoneQuotes;
 };
 
 struct nsStyleContent
 {
   explicit nsStyleContent(StyleStructContext aContext);
   nsStyleContent(const nsStyleContent& aContent);
   ~nsStyleContent(void);