Bug 661746 - Part 5: Avoid creating atoms during selector matching. r=bzbarsky
☠☠ backed out by add05f884131 ☠ ☠
authorDavid Zbarsky <dzbarsky@gmail.com>
Mon, 14 Nov 2011 16:30:28 +1300
changeset 80221 15cf68a3c027d91c1ada77a35fbcb587eaa80b9f
parent 80220 2f403e4c42c88edc2bd831db44107f927b8475be
child 80222 dc82c3485d1a66af9e2eff30a9bbdf70d6fd7f31
push id323
push userrcampbell@mozilla.com
push dateTue, 15 Nov 2011 21:58:36 +0000
treeherderfx-team@3ea216303184 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs661746
milestone11.0a1
Bug 661746 - Part 5: Avoid creating atoms during selector matching. r=bzbarsky
layout/style/StyleRule.cpp
layout/style/StyleRule.h
layout/style/nsCSSParser.cpp
layout/style/nsCSSRuleProcessor.cpp
--- a/layout/style/StyleRule.cpp
+++ b/layout/style/StyleRule.cpp
@@ -138,25 +138,26 @@ nsPseudoClassList::nsPseudoClassList(nsC
   NS_ASSERTION(!nsCSSPseudoClasses::HasStringArg(aType) &&
                !nsCSSPseudoClasses::HasNthPairArg(aType),
                "unexpected pseudo-class");
   MOZ_COUNT_CTOR(nsPseudoClassList);
   u.mMemory = nsnull;
 }
 
 nsPseudoClassList::nsPseudoClassList(nsCSSPseudoClasses::Type aType,
-                                     const PRUnichar* aString)
+                                     nsIAtom* aAtom)
   : mType(aType),
     mNext(nsnull)
 {
   NS_ASSERTION(nsCSSPseudoClasses::HasStringArg(aType),
                "unexpected pseudo-class");
-  NS_ASSERTION(aString, "string expected");
+  NS_ASSERTION(aAtom, "atom expected");
   MOZ_COUNT_CTOR(nsPseudoClassList);
-  u.mString = NS_strdup(aString);
+  NS_ADDREF(aAtom);
+  u.mAtom = aAtom;
 }
 
 nsPseudoClassList::nsPseudoClassList(nsCSSPseudoClasses::Type aType,
                                      const PRInt32* aIntPair)
   : mType(aType),
     mNext(nsnull)
 {
   NS_ASSERTION(nsCSSPseudoClasses::HasNthPairArg(aType),
@@ -182,17 +183,17 @@ nsPseudoClassList::nsPseudoClassList(nsC
 
 nsPseudoClassList*
 nsPseudoClassList::Clone(bool aDeep) const
 {
   nsPseudoClassList *result;
   if (!u.mMemory) {
     result = new nsPseudoClassList(mType);
   } else if (nsCSSPseudoClasses::HasStringArg(mType)) {
-    result = new nsPseudoClassList(mType, u.mString);
+    result = new nsPseudoClassList(mType, u.mAtom);
   } else if (nsCSSPseudoClasses::HasNthPairArg(mType)) {
     result = new nsPseudoClassList(mType, u.mNumbers);
   } else {
     NS_ASSERTION(nsCSSPseudoClasses::HasSelectorListArg(mType),
                  "unexpected pseudo-class");
     // This constructor adopts its selector list argument.
     result = new nsPseudoClassList(mType, u.mSelectors->Clone());
   }
@@ -205,17 +206,21 @@ nsPseudoClassList::Clone(bool aDeep) con
 }
 
 nsPseudoClassList::~nsPseudoClassList(void)
 {
   MOZ_COUNT_DTOR(nsPseudoClassList);
   if (nsCSSPseudoClasses::HasSelectorListArg(mType)) {
     delete u.mSelectors;
   } else if (u.mMemory) {
-    NS_Free(u.mMemory);
+    if (nsCSSPseudoClasses::HasStringArg(mType)) {
+      NS_RELEASE(u.mAtom);
+    } else {
+      NS_Free(u.mMemory);
+    }
   }
   NS_CSS_DELETE_LIST_MEMBER(nsPseudoClassList, this, mNext);
 }
 
 nsAttrSelector::nsAttrSelector(PRInt32 aNameSpace, const nsString& aAttr)
   : mValue(),
     mNext(nsnull),
     mLowercaseAttr(nsnull),
@@ -409,19 +414,19 @@ void nsCSSSelector::AddClass(const nsStr
 }
 
 void nsCSSSelector::AddPseudoClass(nsCSSPseudoClasses::Type aType)
 {
   AddPseudoClassInternal(new nsPseudoClassList(aType));
 }
 
 void nsCSSSelector::AddPseudoClass(nsCSSPseudoClasses::Type aType,
-                                   const PRUnichar* aString)
+                                   nsIAtom* aAtom)
 {
-  AddPseudoClassInternal(new nsPseudoClassList(aType, aString));
+  AddPseudoClassInternal(new nsPseudoClassList(aType, aAtom));
 }
 
 void nsCSSSelector::AddPseudoClass(nsCSSPseudoClasses::Type aType,
                                    const PRInt32* aIntPair)
 {
   AddPseudoClassInternal(new nsPseudoClassList(aType, aIntPair));
 }
 
@@ -767,17 +772,17 @@ nsCSSSelector::AppendToStringWithoutComb
     // has a ":" that can't be escaped and (b) all pseudo-classes at
     // this point are known, and therefore we know they don't need
     // escaping.
     aString.Append(temp);
     if (list->u.mMemory) {
       aString.Append(PRUnichar('('));
       if (nsCSSPseudoClasses::HasStringArg(list->mType)) {
         nsStyleUtil::AppendEscapedCSSIdent(
-          nsDependentString(list->u.mString), aString);
+          nsDependentAtomString(list->u.mAtom), aString);
       } else if (nsCSSPseudoClasses::HasNthPairArg(list->mType)) {
         PRInt32 a = list->u.mNumbers[0],
                 b = list->u.mNumbers[1];
         temp.Truncate();
         if (a != 0) {
           if (a == -1) {
             temp.Append(PRUnichar('-'));
           } else if (a != 1) {
--- a/layout/style/StyleRule.h
+++ b/layout/style/StyleRule.h
@@ -75,17 +75,17 @@ private:
   // These are not supported and are not implemented! 
   nsAtomList(const nsAtomList& aCopy);
   nsAtomList& operator=(const nsAtomList& aCopy); 
 };
 
 struct nsPseudoClassList {
 public:
   nsPseudoClassList(nsCSSPseudoClasses::Type aType);
-  nsPseudoClassList(nsCSSPseudoClasses::Type aType, const PRUnichar *aString);
+  nsPseudoClassList(nsCSSPseudoClasses::Type aType, nsIAtom *aAtom);
   nsPseudoClassList(nsCSSPseudoClasses::Type aType, const PRInt32 *aIntPair);
   nsPseudoClassList(nsCSSPseudoClasses::Type aType,
                     nsCSSSelectorList *aSelectorList /* takes ownership */);
   ~nsPseudoClassList(void);
 
   /** Do a deep clone.  Should be used only on the first in the linked list. */
   nsPseudoClassList* Clone() const { return Clone(true); }
 
@@ -94,18 +94,18 @@ public:
     //   a. no value, which means mMemory is always null
     //      (if none of the conditions for (b), (c), or (d) is true)
     //   b. a string value, which means mString/mMemory is non-null
     //      (if nsCSSPseudoClasses::HasStringArg(mType))
     //   c. an integer pair value, which means mNumbers/mMemory is non-null
     //      (if nsCSSPseudoClasses::HasNthPairArg(mType))
     //   d. a selector list, which means mSelectors is non-null
     //      (if nsCSSPseudoClasses::HasSelectorListArg(mType))
-    void*           mMemory; // mString and mNumbers use NS_Alloc/NS_Free
-    PRUnichar*      mString;
+    void*           mMemory; // mNumbers uses NS_Alloc/NS_Free
+    nsIAtom*        mAtom; // STRONG REF
     PRInt32*        mNumbers;
     nsCSSSelectorList* mSelectors;
   } u;
   nsCSSPseudoClasses::Type mType;
   nsPseudoClassList* mNext;
 private: 
   nsPseudoClassList* Clone(bool aDeep) const;
 
@@ -160,17 +160,17 @@ public:
   nsCSSSelector* Clone() const { return Clone(true, true); }
 
   void Reset(void);
   void SetNameSpace(PRInt32 aNameSpace);
   void SetTag(const nsString& aTag);
   void AddID(const nsString& aID);
   void AddClass(const nsString& aClass);
   void AddPseudoClass(nsCSSPseudoClasses::Type aType);
-  void AddPseudoClass(nsCSSPseudoClasses::Type aType, const PRUnichar* aString);
+  void AddPseudoClass(nsCSSPseudoClasses::Type aType, nsIAtom* aAtom);
   void AddPseudoClass(nsCSSPseudoClasses::Type aType, const PRInt32* aIntPair);
   // takes ownership of aSelectorList
   void AddPseudoClass(nsCSSPseudoClasses::Type aType,
                       nsCSSSelectorList* aSelectorList);
   void AddAttribute(PRInt32 aNameSpace, const nsString& aAttr);
   void AddAttribute(PRInt32 aNameSpace, const nsString& aAttr, PRUint8 aFunc, 
                     const nsString& aValue, bool aCaseSensitive);
   void SetOperator(PRUnichar aOperator);
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -3366,26 +3366,26 @@ CSSParserImpl::ParsePseudoClassWithIdent
   }
   // We expect an identifier with a language abbreviation
   if (eCSSToken_Ident != mToken.mType) {
     REPORT_UNEXPECTED_TOKEN(PEPseudoClassArgNotIdent);
     UngetToken();
     return eSelectorParsingStatus_Error; // our caller calls SkipUntil(')')
   }
 
+  nsCOMPtr<nsIAtom> atom = do_GetAtom(mToken.mIdent);
   // -moz-locale-dir can only have values of 'ltr' or 'rtl'.
   if (aType == nsCSSPseudoClasses::ePseudoClass_mozLocaleDir) {
-    if (!mToken.mIdent.EqualsLiteral("ltr") &&
-        !mToken.mIdent.EqualsLiteral("rtl")) {
+    if (atom != nsGkAtoms::ltr && atom != nsGkAtoms::rtl) {
       return eSelectorParsingStatus_Error; // our caller calls SkipUntil(')')
     }
   }
 
   // Add the pseudo with the language parameter
-  aSelector.AddPseudoClass(aType, mToken.mIdent.get());
+  aSelector.AddPseudoClass(aType, atom);
 
   // close the parenthesis
   if (!ExpectSymbol(')', true)) {
     REPORT_UNEXPECTED_TOKEN(PEPseudoClassNoClose);
     return eSelectorParsingStatus_Error; // our caller calls SkipUntil(')')
   }
 
   return eSelectorParsingStatus_Continue;
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1681,71 +1681,68 @@ static bool SelectorMatches(const Elemen
       case nsCSSPseudoClasses::ePseudoClass_mozOnlyWhitespace:
         if (!checkGenericEmptyMatches(aElement, aTreeMatchContext, false)) {
           return false;
         }
         break;
 
       case nsCSSPseudoClasses::ePseudoClass_mozEmptyExceptChildrenWithLocalname:
         {
-          NS_ASSERTION(pseudoClass->u.mString, "Must have string!");
+          NS_ASSERTION(pseudoClass->u.mAtom, "Must have atom!");
           const nsIContent *child = nsnull;
           PRInt32 index = -1;
 
           if (aTreeMatchContext.mForStyling)
             // FIXME:  This isn't sufficient to handle:
             //   :-moz-empty-except-children-with-localname() + E
             //   :-moz-empty-except-children-with-localname() ~ E
             // because we don't know to restyle the grandparent of the
             // inserted/removed element (as in bug 534804 for :empty).
             aTreeMatchContext.RecordFlagsToAdd(aElement, NODE_HAS_SLOW_SELECTOR);
           do {
             child = aElement->GetChildAt(++index);
           } while (child &&
                    (!IsSignificantChild(child, true, false) ||
                     (child->GetNameSpaceID() == aElement->GetNameSpaceID() &&
-                     child->Tag()->Equals(nsDependentString(pseudoClass->u.mString)))));
+                     child->Tag() == pseudoClass->u.mAtom)));
           if (child != nsnull) {
             return false;
           }
         }
         break;
 
       case nsCSSPseudoClasses::ePseudoClass_lang:
         {
-          NS_ASSERTION(nsnull != pseudoClass->u.mString, "null lang parameter");
-          if (!pseudoClass->u.mString || !*pseudoClass->u.mString) {
-            return false;
-          }
+          NS_ASSERTION(pseudoClass->u.mAtom, "Must have atom!");
 
           // We have to determine the language of the current element.  Since
           // this is currently no property and since the language is inherited
           // from the parent we have to be prepared to look at all parent
           // nodes.  The language itself is encoded in the LANG attribute.
           nsAutoString language;
           aElement->GetLang(language);
+          nsDependentAtomString langString(pseudoClass->u.mAtom);
           if (!language.IsEmpty()) {
             if (!nsStyleUtil::DashMatchCompare(language,
-                                               nsDependentString(pseudoClass->u.mString),
+                                               langString,
                                                nsASCIICaseInsensitiveStringComparator())) {
               return false;
             }
             // This pseudo-class matched; move on to the next thing
             break;
           }
 
           const nsIDocument* doc = aTreeMatchContext.mDocument;
           if (doc) {
             // Try to get the language from the HTTP header or if this
             // is missing as well from the preferences.
             // The content language can be a comma-separated list of
             // language codes.
             doc->GetContentLanguage(language);
 
-            nsDependentString langString(pseudoClass->u.mString);
             language.StripWhitespace();
             PRInt32 begin = 0;
             PRInt32 len = language.Length();
             while (begin < len) {
               PRInt32 end = language.FindChar(PRUnichar(','), begin);
               if (end == kNotFound) {
                 end = len;
               }
@@ -1928,34 +1925,32 @@ static bool SelectorMatches(const Elemen
       case nsCSSPseudoClasses::ePseudoClass_mozIsHTML:
         if (!aTreeMatchContext.mIsHTMLDocument || !aElement->IsHTML()) {
           return false;
         }
         break;
 
       case nsCSSPseudoClasses::ePseudoClass_mozSystemMetric:
         {
-          nsCOMPtr<nsIAtom> metric = do_GetAtom(pseudoClass->u.mString);
-          if (!nsCSSRuleProcessor::HasSystemMetric(metric)) {
+          if (!nsCSSRuleProcessor::HasSystemMetric(pseudoClass->u.mAtom)) {
             return false;
           }
         }
         break;
 
       case nsCSSPseudoClasses::ePseudoClass_mozLocaleDir:
         {
           bool docIsRTL =
             aTreeMatchContext.mDocStates.HasState(NS_DOCUMENT_STATE_RTL_LOCALE);
 
-          nsDependentString dirString(pseudoClass->u.mString);
-          NS_ASSERTION(dirString.EqualsLiteral("ltr") ||
-                       dirString.EqualsLiteral("rtl"),
+          NS_ASSERTION(pseudoClass->u.mAtom == nsGkAtoms::ltr ||
+                       pseudoClass->u.mAtom == nsGkAtoms::rtl,
                        "invalid value for -moz-locale-dir");
 
-          if (dirString.EqualsLiteral("rtl") != docIsRTL) {
+          if ((pseudoClass->u.mAtom == nsGkAtoms::rtl) != docIsRTL) {
             return false;
           }
         }
         break;
 
       case nsCSSPseudoClasses::ePseudoClass_mozLWTheme:
         {
           if (aTreeMatchContext.mDocTheme <= nsIDocument::Doc_Theme_None) {