Bug 1315155 - Part 1: Encapsulate nsStyleContentData. r?xidorn draft
authorCameron McCormack <cam@mcc.id.au>
Thu, 20 Oct 2016 10:22:46 +0800
changeset 433594 48b73ee0206428294d104ab302a407c944dce628
parent 433588 30359e047e9b24b073a9e996b4b3f843f2c5ff8f
child 433595 57f9adee6202fb8730b24ed1ee61a4956c73caa4
push id34625
push userbmo:cam@mcc.id.au
push dateFri, 04 Nov 2016 03:04:28 +0000
reviewersxidorn
bugs1315155
milestone52.0a1
Bug 1315155 - Part 1: Encapsulate nsStyleContentData. r?xidorn MozReview-Commit-ID: LfEMxyM5meF
layout/base/nsCSSFrameConstructor.cpp
layout/style/nsComputedDOMStyle.cpp
layout/style/nsRuleNode.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -1644,50 +1644,50 @@ already_AddRefed<nsIContent>
 nsCSSFrameConstructor::CreateGeneratedContent(nsFrameConstructorState& aState,
                                               nsIContent*     aParentContent,
                                               nsStyleContext* aStyleContext,
                                               uint32_t        aContentIndex)
 {
   // Get the content value
   const nsStyleContentData &data =
     aStyleContext->StyleContent()->ContentAt(aContentIndex);
-  nsStyleContentType type = data.mType;
-
-  if (eStyleContentType_Image == type) {
-    if (!data.mContent.mImage) {
-      // CSS had something specified that couldn't be converted to an
-      // image object
-      return nullptr;
-    }
-
-    // Create an image content object and pass it the image request.
-    // XXX Check if it's an image type we can handle...
-
-    RefPtr<NodeInfo> nodeInfo;
-    nodeInfo = mDocument->NodeInfoManager()->
-      GetNodeInfo(nsGkAtoms::mozgeneratedcontentimage, nullptr,
-                  kNameSpaceID_XHTML, nsIDOMNode::ELEMENT_NODE);
-
-    nsCOMPtr<nsIContent> content;
-    NS_NewGenConImageContent(getter_AddRefs(content), nodeInfo.forget(),
-                             data.mContent.mImage);
-    return content.forget();
-  }
+  nsStyleContentType type = data.GetType();
 
   switch (type) {
-  case eStyleContentType_String:
-    return CreateGenConTextNode(aState,
-                                nsDependentString(data.mContent.mString),
-                                nullptr, nullptr);
-
-  case eStyleContentType_Attr:
-    {
+    case eStyleContentType_Image: {
+      imgRequestProxy* image = data.GetImage();
+      if (!image) {
+        // CSS had something specified that couldn't be converted to an
+        // image object
+        return nullptr;
+      }
+
+      // Create an image content object and pass it the image request.
+      // XXX Check if it's an image type we can handle...
+
+      RefPtr<NodeInfo> nodeInfo;
+      nodeInfo = mDocument->NodeInfoManager()->
+        GetNodeInfo(nsGkAtoms::mozgeneratedcontentimage, nullptr,
+                    kNameSpaceID_XHTML, nsIDOMNode::ELEMENT_NODE);
+
+      nsCOMPtr<nsIContent> content;
+      NS_NewGenConImageContent(getter_AddRefs(content), nodeInfo.forget(),
+                               image);
+      return content.forget();
+    }
+
+    case eStyleContentType_String:
+      return CreateGenConTextNode(aState,
+                                  nsDependentString(data.GetString()),
+                                  nullptr, nullptr);
+
+    case eStyleContentType_Attr: {
       nsCOMPtr<nsIAtom> attrName;
       int32_t attrNameSpace = kNameSpaceID_None;
-      nsAutoString contentString(data.mContent.mString);
+      nsAutoString contentString(data.GetString());
 
       int32_t barIndex = contentString.FindChar('|'); // CSS namespace delimiter
       if (-1 != barIndex) {
         nsAutoString  nameSpaceVal;
         contentString.Left(nameSpaceVal, barIndex);
         nsresult error;
         attrNameSpace = nameSpaceVal.ToInteger(&error);
         contentString.Cut(0, barIndex + 1);
@@ -1710,56 +1710,49 @@ nsCSSFrameConstructor::CreateGeneratedCo
       }
 
       nsCOMPtr<nsIContent> content;
       NS_NewAttributeContent(mDocument->NodeInfoManager(),
                              attrNameSpace, attrName, getter_AddRefs(content));
       return content.forget();
     }
 
-  case eStyleContentType_Counter:
-  case eStyleContentType_Counters:
-    {
-      nsCSSValue::Array* counters = data.mContent.mCounters;
+    case eStyleContentType_Counter:
+    case eStyleContentType_Counters: {
+      nsCSSValue::Array* counters = data.GetCounters();
       nsCounterList* counterList = mCounterManager.CounterListFor(
           nsDependentString(counters->Item(0).GetStringBufferValue()));
 
       nsCounterUseNode* node =
         new nsCounterUseNode(mPresShell->GetPresContext(),
                              counters, aContentIndex,
                              type == eStyleContentType_Counters);
 
       nsGenConInitializer* initializer =
         new nsGenConInitializer(node, counterList,
                                 &nsCSSFrameConstructor::CountersDirty);
       return CreateGenConTextNode(aState, EmptyString(), &node->mText,
                                   initializer);
     }
 
-  case eStyleContentType_Image:
-    NS_NOTREACHED("handled by if above");
-    return nullptr;
-
-  case eStyleContentType_OpenQuote:
-  case eStyleContentType_CloseQuote:
-  case eStyleContentType_NoOpenQuote:
-  case eStyleContentType_NoCloseQuote:
-    {
+    case eStyleContentType_OpenQuote:
+    case eStyleContentType_CloseQuote:
+    case eStyleContentType_NoOpenQuote:
+    case eStyleContentType_NoCloseQuote: {
       nsQuoteNode* node =
         new nsQuoteNode(type, aContentIndex);
 
       nsGenConInitializer* initializer =
         new nsGenConInitializer(node, &mQuoteList,
                                 &nsCSSFrameConstructor::QuotesDirty);
       return CreateGenConTextNode(aState, EmptyString(), &node->mText,
                                   initializer);
     }
 
-  case eStyleContentType_AltContent:
-    {
+    case eStyleContentType_AltContent: {
       // Use the "alt" attribute; if that fails and the node is an HTML
       // <input>, try the value attribute and then fall back to some default
       // localized text we have.
       // XXX what if the 'alt' attribute is added later, how will we
       // detect that and do the right thing here?
       if (aParentContent->HasAttr(kNameSpaceID_None, nsGkAtoms::alt)) {
         nsCOMPtr<nsIContent> content;
         NS_NewAttributeContent(mDocument->NodeInfoManager(),
@@ -1780,20 +1773,20 @@ nsCSSFrameConstructor::CreateGeneratedCo
         nsContentUtils::GetLocalizedString(nsContentUtils::eFORMS_PROPERTIES,
                                            "Submit", temp);
         return CreateGenConTextNode(aState, temp, nullptr, nullptr);
       }
 
       break;
     }
 
-  case eStyleContentType_Uninitialized:
-    NS_NOTREACHED("uninitialized content type");
-    return nullptr;
-  } // switch
+    case eStyleContentType_Uninitialized:
+      NS_NOTREACHED("uninitialized content type");
+      return nullptr;
+  }
 
   return nullptr;
 }
 
 /*
  * aParentFrame - the frame that should be the parent of the generated
  *   content.  This is the frame for the corresponding content node,
  *   which must not be a leaf frame.
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -1075,91 +1075,88 @@ nsComputedDOMStyle::DoGetContent()
 
   if (content->ContentCount() == 0) {
     RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
     val->SetIdent(eCSSKeyword_none);
     return val.forget();
   }
 
   if (content->ContentCount() == 1 &&
-      content->ContentAt(0).mType == eStyleContentType_AltContent) {
+      content->ContentAt(0).GetType() == eStyleContentType_AltContent) {
     RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
     val->SetIdent(eCSSKeyword__moz_alt_content);
     return val.forget();
   }
 
   RefPtr<nsDOMCSSValueList> valueList = GetROCSSValueList(false);
 
   for (uint32_t i = 0, i_end = content->ContentCount(); i < i_end; ++i) {
     RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
 
     const nsStyleContentData &data = content->ContentAt(i);
-    switch (data.mType) {
-      case eStyleContentType_String:
-        {
-          nsAutoString str;
-          nsStyleUtil::AppendEscapedCSSString(
-            nsDependentString(data.mContent.mString), str);
-          val->SetString(str);
-        }
+    nsStyleContentType type = data.GetType();
+    switch (type) {
+      case eStyleContentType_String: {
+        nsAutoString str;
+        nsStyleUtil::AppendEscapedCSSString(
+          nsDependentString(data.GetString()), str);
+        val->SetString(str);
         break;
-      case eStyleContentType_Image:
-        {
-          nsCOMPtr<nsIURI> uri;
-          if (data.mContent.mImage) {
-            data.mContent.mImage->GetURI(getter_AddRefs(uri));
-          }
-          val->SetURI(uri);
+      }
+      case eStyleContentType_Image: {
+        nsCOMPtr<nsIURI> uri;
+        if (imgRequestProxy* image = data.GetImage()) {
+          image->GetURI(getter_AddRefs(uri));
         }
+        val->SetURI(uri);
         break;
-      case eStyleContentType_Attr:
-        {
-          nsAutoString str;
-          nsStyleUtil::AppendEscapedCSSIdent(
-            nsDependentString(data.mContent.mString), str);
-          val->SetString(str, nsIDOMCSSPrimitiveValue::CSS_ATTR);
-        }
+      }
+      case eStyleContentType_Attr: {
+        nsAutoString str;
+        nsStyleUtil::AppendEscapedCSSIdent(
+          nsDependentString(data.GetString()), str);
+        val->SetString(str, nsIDOMCSSPrimitiveValue::CSS_ATTR);
         break;
+      }
       case eStyleContentType_Counter:
-      case eStyleContentType_Counters:
-        {
-          /* FIXME: counters should really use an object */
-          nsAutoString str;
-          if (data.mType == eStyleContentType_Counter) {
-            str.AppendLiteral("counter(");
-          }
-          else {
-            str.AppendLiteral("counters(");
-          }
-          // WRITE ME
-          nsCSSValue::Array *a = data.mContent.mCounters;
-
-          nsStyleUtil::AppendEscapedCSSIdent(
-            nsDependentString(a->Item(0).GetStringBufferValue()), str);
-          int32_t typeItem = 1;
-          if (data.mType == eStyleContentType_Counters) {
-            typeItem = 2;
-            str.AppendLiteral(", ");
-            nsStyleUtil::AppendEscapedCSSString(
-              nsDependentString(a->Item(1).GetStringBufferValue()), str);
-          }
-          MOZ_ASSERT(eCSSUnit_None != a->Item(typeItem).GetUnit(),
-                     "'none' should be handled as identifier value");
-          nsString type;
-          a->Item(typeItem).AppendToString(eCSSProperty_list_style_type,
-                                           type, nsCSSValue::eNormalized);
-          if (!type.LowerCaseEqualsLiteral("decimal")) {
-            str.AppendLiteral(", ");
-            str.Append(type);
-          }
-
-          str.Append(char16_t(')'));
-          val->SetString(str, nsIDOMCSSPrimitiveValue::CSS_COUNTER);
+      case eStyleContentType_Counters: {
+        /* FIXME: counters should really use an object */
+        nsAutoString str;
+        if (type == eStyleContentType_Counter) {
+          str.AppendLiteral("counter(");
+        }
+        else {
+          str.AppendLiteral("counters(");
         }
+        // WRITE ME
+        nsCSSValue::Array *a = data.GetCounters();
+
+        nsStyleUtil::AppendEscapedCSSIdent(
+          nsDependentString(a->Item(0).GetStringBufferValue()), str);
+        int32_t typeItem = 1;
+        if (type == eStyleContentType_Counters) {
+          typeItem = 2;
+          str.AppendLiteral(", ");
+          nsStyleUtil::AppendEscapedCSSString(
+            nsDependentString(a->Item(1).GetStringBufferValue()), str);
+        }
+        MOZ_ASSERT(eCSSUnit_None != a->Item(typeItem).GetUnit(),
+                   "'none' should be handled as identifier value");
+        nsString type;
+        a->Item(typeItem).AppendToString(eCSSProperty_list_style_type,
+                                         type, nsCSSValue::eNormalized);
+        if (!type.LowerCaseEqualsLiteral("decimal")) {
+          str.AppendLiteral(", ");
+          str.Append(type);
+        }
+
+        str.Append(char16_t(')'));
+        val->SetString(str, nsIDOMCSSPrimitiveValue::CSS_COUNTER);
         break;
+      }
       case eStyleContentType_OpenQuote:
         val->SetIdent(eCSSKeyword_open_quote);
         break;
       case eStyleContentType_CloseQuote:
         val->SetIdent(eCSSKeyword_close_quote);
         break;
       case eStyleContentType_NoOpenQuote:
         val->SetIdent(eCSSKeyword_no_open_quote);
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -8817,19 +8817,17 @@ nsRuleNode::ComputeContentData(void* aSt
       content->ContentAt(count) = parentContent->ContentAt(count);
     }
     break;
 
   case eCSSUnit_Enumerated: {
     MOZ_ASSERT(contentValue->GetIntValue() == NS_STYLE_CONTENT_ALT_CONTENT,
                "unrecognized solitary content keyword");
     content->AllocateContents(1);
-    nsStyleContentData& data = content->ContentAt(0);
-    data.mType = eStyleContentType_AltContent;
-    data.mContent.mString = nullptr;
+    content->ContentAt(0).SetKeyword(eStyleContentType_AltContent);
     break;
   }
 
   case eCSSUnit_List:
   case eCSSUnit_ListDep: {
     const nsCSSValueList* contentValueList = contentValue->GetListValue();
     count = 0;
     while (contentValueList) {
@@ -8838,56 +8836,62 @@ nsRuleNode::ComputeContentData(void* aSt
     }
     content->AllocateContents(count);
     const nsAutoString nullStr;
     count = 0;
     contentValueList = contentValue->GetListValue();
     while (contentValueList) {
       const nsCSSValue& value = contentValueList->mValue;
       nsCSSUnit unit = value.GetUnit();
-      nsStyleContentType type;
-      nsStyleContentData &data = content->ContentAt(count++);
+      nsStyleContentData& data = content->ContentAt(count++);
       switch (unit) {
-      case eCSSUnit_String:   type = eStyleContentType_String;    break;
-      case eCSSUnit_Image:    type = eStyleContentType_Image;     break;
-      case eCSSUnit_Attr:     type = eStyleContentType_Attr;      break;
-      case eCSSUnit_Counter:  type = eStyleContentType_Counter;   break;
-      case eCSSUnit_Counters: type = eStyleContentType_Counters;  break;
-      case eCSSUnit_Enumerated:
-        switch (value.GetIntValue()) {
-        case NS_STYLE_CONTENT_OPEN_QUOTE:
-          type = eStyleContentType_OpenQuote;     break;
-        case NS_STYLE_CONTENT_CLOSE_QUOTE:
-          type = eStyleContentType_CloseQuote;    break;
-        case NS_STYLE_CONTENT_NO_OPEN_QUOTE:
-          type = eStyleContentType_NoOpenQuote;   break;
-        case NS_STYLE_CONTENT_NO_CLOSE_QUOTE:
-          type = eStyleContentType_NoCloseQuote;  break;
+        case eCSSUnit_Image:
+          SetImageRequest([&](imgRequestProxy* req) {
+            data.SetImage(req);
+          }, mPresContext, value);
+          break;
+        case eCSSUnit_String:
+        case eCSSUnit_Attr: {
+          nsStyleContentType type =
+            unit == eCSSUnit_String ? eStyleContentType_String
+                                    : eStyleContentType_Attr;
+          value.GetStringValue(buffer);
+          data.SetString(type, buffer.get());
+          break;
+        }
+        case eCSSUnit_Counter:
+        case eCSSUnit_Counters: {
+          nsStyleContentType type =
+            unit == eCSSUnit_Counter ? eStyleContentType_Counter
+                                     : eStyleContentType_Counters;
+          data.SetCounters(type, value.GetArrayValue());
+          break;
+        }
+        case eCSSUnit_Enumerated:
+          switch (value.GetIntValue()) {
+            case NS_STYLE_CONTENT_OPEN_QUOTE:
+              data.SetKeyword(eStyleContentType_OpenQuote);
+              break;
+            case NS_STYLE_CONTENT_CLOSE_QUOTE:
+              data.SetKeyword(eStyleContentType_CloseQuote);
+              break;
+            case NS_STYLE_CONTENT_NO_OPEN_QUOTE:
+              data.SetKeyword(eStyleContentType_NoOpenQuote);
+              break;
+            case NS_STYLE_CONTENT_NO_CLOSE_QUOTE:
+              data.SetKeyword(eStyleContentType_NoCloseQuote);
+              break;
+            default:
+              NS_ERROR("bad content value");
+              break;
+          }
+          break;
         default:
-          NS_ERROR("bad content value");
-          type = eStyleContentType_Uninitialized;
-        }
-        break;
-      default:
-        NS_ERROR("bad content type");
-        type = eStyleContentType_Uninitialized;
-      }
-      data.mType = type;
-      if (type == eStyleContentType_Image) {
-        SetImageRequest([&](imgRequestProxy* req) {
-          data.SetImage(req);
-        }, mPresContext, value);
-      } else if (type <= eStyleContentType_Attr) {
-        value.GetStringValue(buffer);
-        data.mContent.mString = NS_strdup(buffer.get());
-      } else if (type <= eStyleContentType_Counters) {
-        data.mContent.mCounters = value.GetArrayValue();
-        data.mContent.mCounters->AddRef();
-      } else {
-        data.mContent.mString = nullptr;
+          NS_ERROR("bad content type");
+          break;
       }
       contentValueList = contentValueList->mNext;
     }
     break;
   }
 
   default:
     MOZ_ASSERT(false, "unrecognized content unit");
@@ -8988,18 +8992,18 @@ nsRuleNode::ComputeContentData(void* aSt
   }
 
   default:
     MOZ_ASSERT(false, "unexpected value unit");
   }
 
   // If we ended up with an image, track it.
   for (uint32_t i = 0; i < content->ContentCount(); ++i) {
-    if ((content->ContentAt(i).mType == eStyleContentType_Image) &&
-        content->ContentAt(i).mContent.mImage) {
+    if (content->ContentAt(i).GetType() == eStyleContentType_Image &&
+        content->ContentAt(i).GetImage()) {
       content->ContentAt(i).TrackImage(
           aContext->PresContext()->Document()->ImageTracker());
     }
   }
 
   COMPUTE_END_RESET(Content, content)
 }
 
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3626,17 +3626,18 @@ nsStyleContent::~nsStyleContent()
   MOZ_COUNT_DTOR(nsStyleContent);
 }
 
 void
 nsStyleContent::Destroy(nsPresContext* aContext)
 {
   // Unregister any images we might have with the document.
   for (auto& content : mContents) {
-    if (content.mType == eStyleContentType_Image && content.mContent.mImage) {
+    if (content.GetType() == eStyleContentType_Image &&
+        content.GetImage()) {
       content.UntrackImage(aContext->Document()->ImageTracker());
     }
   }
 
   this->~nsStyleContent();
   aContext->PresShell()->FreeByObjectID(eArenaObjectID_nsStyleContent, this);
 }
 
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -3148,28 +3148,19 @@ enum nsStyleContentType {
   eStyleContentType_OpenQuote     = 40,
   eStyleContentType_CloseQuote    = 41,
   eStyleContentType_NoOpenQuote   = 42,
   eStyleContentType_NoCloseQuote  = 43,
   eStyleContentType_AltContent    = 50,
   eStyleContentType_Uninitialized
 };
 
-struct nsStyleContentData
+class nsStyleContentData
 {
-  nsStyleContentType  mType;
-  union {
-    char16_t *mString;
-    imgRequestProxy *mImage;
-    nsCSSValue::Array* mCounters;
-  } mContent;
-#ifdef DEBUG
-  bool mImageTracked;
-#endif
-
+public:
   nsStyleContentData()
     : mType(eStyleContentType_Uninitialized)
 #ifdef DEBUG
     , mImageTracked(false)
 #endif
   {
     MOZ_COUNT_CTOR(nsStyleContentData);
     mContent.mString = nullptr;
@@ -3182,23 +3173,92 @@ struct nsStyleContentData
 
   bool operator!=(const nsStyleContentData& aOther) const {
     return !(*this == aOther);
   }
 
   void TrackImage(mozilla::dom::ImageTracker* aImageTracker);
   void UntrackImage(mozilla::dom::ImageTracker* aImageTracker);
 
+  nsStyleContentType GetType() const { return mType; }
+
+  char16_t* GetString() const
+  {
+    MOZ_ASSERT(mType == eStyleContentType_String ||
+               mType == eStyleContentType_Attr);
+    return mContent.mString;
+  }
+
+  nsCSSValue::Array* GetCounters() const
+  {
+    MOZ_ASSERT(mType == eStyleContentType_Counter ||
+               mType == eStyleContentType_Counters);
+    return mContent.mCounters;
+  }
+
+  imgRequestProxy* GetImage() const
+  {
+    MOZ_ASSERT(mType == eStyleContentType_Image);
+    return mContent.mImage;
+  }
+
+  void SetKeyword(nsStyleContentType aType)
+  {
+    MOZ_ASSERT(aType == eStyleContentType_OpenQuote ||
+               aType == eStyleContentType_CloseQuote ||
+               aType == eStyleContentType_NoOpenQuote ||
+               aType == eStyleContentType_NoCloseQuote ||
+               aType == eStyleContentType_AltContent);
+    MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
+               "should only initialize nsStyleContentData once");
+    mType = aType;
+  }
+
+  void SetString(nsStyleContentType aType, const char16_t* aString)
+  {
+    MOZ_ASSERT(aType == eStyleContentType_String ||
+               aType == eStyleContentType_Attr);
+    MOZ_ASSERT(aString);
+    MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
+               "should only initialize nsStyleContentData once");
+    mType = aType;
+    mContent.mString = NS_strdup(aString);
+  }
+
+  void SetCounters(nsStyleContentType aType, nsCSSValue::Array* aCounters)
+  {
+    MOZ_ASSERT(aType == eStyleContentType_Counter ||
+               aType == eStyleContentType_Counters);
+    MOZ_ASSERT(aCounters);
+    MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
+               "should only initialize nsStyleContentData once");
+    mType = aType;
+    mContent.mCounters = aCounters;
+    mContent.mCounters->AddRef();
+  }
+
   void SetImage(imgRequestProxy* aRequest)
   {
+    MOZ_ASSERT(mType == eStyleContentType_Uninitialized,
+               "should only initialize nsStyleContentData once");
     MOZ_ASSERT(!mImageTracked,
                "Setting a new image without untracking the old one!");
-    MOZ_ASSERT(mType == eStyleContentType_Image, "Wrong type!");
     NS_IF_ADDREF(mContent.mImage = aRequest);
   }
+
+private:
+  nsStyleContentType mType;
+  union {
+    char16_t *mString;
+    imgRequestProxy *mImage;
+    nsCSSValue::Array* mCounters;
+  } mContent;
+#ifdef DEBUG
+  bool mImageTracked;
+#endif
 };
 
 struct nsStyleCounterData
 {
   nsString  mCounter;
   int32_t   mValue;
 
   bool operator==(const nsStyleCounterData& aOther) const {