Bug 697917. Avoid atomizing the token for nsDOMTokenList containment tests. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 28 Oct 2011 13:06:39 -0400
changeset 80022 0371519c061e745a27665ce8e6396b0376c2a088
parent 80021 4d44e0defe3877d5d77d99ca0d94f9cde3b34cbb
child 80023 f395b5acb6118fa2cbfd1fae6ff3f76c80760b7f
push id506
push userclegnitto@mozilla.com
push dateWed, 09 Nov 2011 02:03:18 +0000
treeherdermozilla-aurora@63587fc7bb93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs697917
milestone10.0a1
Bug 697917. Avoid atomizing the token for nsDOMTokenList containment tests. r=smaug
content/base/src/nsAttrValue.cpp
content/base/src/nsAttrValue.h
content/base/src/nsDOMTokenList.cpp
content/base/src/nsDOMTokenList.h
xpcom/glue/nsTArray.h
--- a/content/base/src/nsAttrValue.cpp
+++ b/content/base/src/nsAttrValue.cpp
@@ -769,17 +769,17 @@ nsAttrValue::Contains(nsIAtom* aValue, n
         nsContentUtils::EqualsIgnoreASCIICase(nsDependentAtomString(aValue),
                                               nsDependentAtomString(atom));
     }
     default:
     {
       if (Type() == eAtomArray) {
         AtomArray* array = GetAtomArrayValue();
         if (aCaseSensitive == eCaseMatters) {
-          return array->IndexOf(aValue) != AtomArray::NoIndex;
+          return array->Contains(aValue);
         }
 
         nsDependentAtomString val1(aValue);
 
         for (nsCOMPtr<nsIAtom> *cur = array->Elements(),
                                *end = cur + array->Length();
              cur != end; ++cur) {
           // For performance reasons, don't do a full on unicode case
@@ -792,16 +792,43 @@ nsAttrValue::Contains(nsIAtom* aValue, n
         }
       }
     }
   }
 
   return false;
 }
 
+struct AtomArrayStringComparator {
+  bool Equals(nsIAtom* atom, const nsAString& string) const {
+    return atom->Equals(string);
+  }
+};
+
+bool
+nsAttrValue::Contains(const nsAString& aValue) const
+{
+  switch (BaseType()) {
+    case eAtomBase:
+    {
+      nsIAtom* atom = GetAtomValue();
+      return atom->Equals(aValue);
+    }
+    default:
+    {
+      if (Type() == eAtomArray) {
+        AtomArray* array = GetAtomArrayValue();
+        return array->Contains(aValue, AtomArrayStringComparator());
+      }
+    }
+  }
+
+  return false;
+}
+
 void
 nsAttrValue::ParseAtom(const nsAString& aValue)
 {
   ResetIfSet();
 
   nsIAtom* atom = NS_NewAtom(aValue);
   if (atom) {
     SetPtrValueAndType(atom, eAtomBase);
--- a/content/base/src/nsAttrValue.h
+++ b/content/base/src/nsAttrValue.h
@@ -175,16 +175,22 @@ public:
   bool Equals(const nsAString& aValue, nsCaseTreatment aCaseSensitive) const;
   bool Equals(nsIAtom* aValue, nsCaseTreatment aCaseSensitive) const;
 
   /**
    * Returns true if this AttrValue is equal to the given atom, or is an
    * array which contains the given atom.
    */
   bool Contains(nsIAtom* aValue, nsCaseTreatment aCaseSensitive) const;
+  /**
+   * Returns true if this AttrValue is an atom equal to the given
+   * string, or is an array of atoms which contains the given string.
+   * This always does a case-sensitive comparison.
+   */
+  bool Contains(const nsAString& aValue) const;
 
   void ParseAtom(const nsAString& aValue);
   void ParseAtomArray(const nsAString& aValue);
   void ParseStringOrAtom(const nsAString& aValue);
 
   /**
    * Structure for a mapping from int (enum) values to strings.  When you use
    * it you generally create an array of them.
--- a/content/base/src/nsDOMTokenList.cpp
+++ b/content/base/src/nsDOMTokenList.cpp
@@ -118,39 +118,29 @@ nsDOMTokenList::CheckToken(const nsAStri
     if (nsContentUtils::IsHTMLWhitespace(*iter))
       return NS_ERROR_DOM_INVALID_CHARACTER_ERR;
     ++iter;
   }
 
   return NS_OK;
 }
 
-bool
-nsDOMTokenList::ContainsInternal(const nsAttrValue* aAttr,
-                                 const nsAString& aToken)
-{
-  NS_ABORT_IF_FALSE(aAttr, "Need an attribute");
-
-  nsCOMPtr<nsIAtom> atom = do_GetAtom(aToken);
-  return aAttr->Contains(atom, eCaseMatters);
-}
-
 NS_IMETHODIMP
 nsDOMTokenList::Contains(const nsAString& aToken, bool* aResult)
 {
   nsresult rv = CheckToken(aToken);
   NS_ENSURE_SUCCESS(rv, rv);
 
   const nsAttrValue* attr = GetParsedAttr();
   if (!attr) {
     *aResult = false;
     return NS_OK;
   }
 
-  *aResult = ContainsInternal(attr, aToken);
+  *aResult = attr->Contains(aToken);
 
   return NS_OK;
 }
 
 void
 nsDOMTokenList::AddInternal(const nsAttrValue* aAttr,
                             const nsAString& aToken)
 {
@@ -177,17 +167,17 @@ nsDOMTokenList::AddInternal(const nsAttr
 NS_IMETHODIMP
 nsDOMTokenList::Add(const nsAString& aToken)
 {
   nsresult rv = CheckToken(aToken);
   NS_ENSURE_SUCCESS(rv, rv);
 
   const nsAttrValue* attr = GetParsedAttr();
 
-  if (attr && ContainsInternal(attr, aToken)) {
+  if (attr && attr->Contains(aToken)) {
     return NS_OK;
   }
 
   AddInternal(attr, aToken);
 
   return NS_OK;
 }
 
@@ -259,34 +249,34 @@ nsDOMTokenList::Remove(const nsAString& 
   nsresult rv = CheckToken(aToken);
   NS_ENSURE_SUCCESS(rv, rv);
 
   const nsAttrValue* attr = GetParsedAttr();
   if (!attr) {
     return NS_OK;
   }
 
-  if (!ContainsInternal(attr, aToken)) {
+  if (!attr->Contains(aToken)) {
     return NS_OK;
   }
 
   RemoveInternal(attr, aToken);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMTokenList::Toggle(const nsAString& aToken, bool* aResult)
 {
   nsresult rv = CheckToken(aToken);
   NS_ENSURE_SUCCESS(rv, rv);
 
   const nsAttrValue* attr = GetParsedAttr();
 
-  if (attr && ContainsInternal(attr, aToken)) {
+  if (attr && attr->Contains(aToken)) {
     RemoveInternal(attr, aToken);
     *aResult = false;
   } else {
     AddInternal(attr, aToken);
     *aResult = true;
   }
 
   return NS_OK;
--- a/content/base/src/nsDOMTokenList.h
+++ b/content/base/src/nsDOMTokenList.h
@@ -62,17 +62,16 @@ protected:
   const nsAttrValue* GetParsedAttr() {
     if (!mElement) {
       return nsnull;
     }
     return mElement->GetAttrInfo(kNameSpaceID_None, mAttrAtom).mValue;
   }
 
   nsresult CheckToken(const nsAString& aStr);
-  bool ContainsInternal(const nsAttrValue* aAttr, const nsAString& aToken);
   void AddInternal(const nsAttrValue* aAttr, const nsAString& aToken);
   void RemoveInternal(const nsAttrValue* aAttr, const nsAString& aToken);
 
   nsGenericElement* mElement;
   nsCOMPtr<nsIAtom> mAttrAtom;
 };
 
 #endif // nsDOMTokenList_h___
--- a/xpcom/glue/nsTArray.h
+++ b/xpcom/glue/nsTArray.h
@@ -405,24 +405,25 @@ public:
 //   elem_type MAY define operator== for searching.
 //
 // For methods taking a Comparator instance, the Comparator must be a class
 // defining the following methods:
 //
 //   class Comparator {
 //     public:
 //       /** @return True if the elements are equals; false otherwise. */
-//       bool Equals(const elem_type& a, const elem_type& b) const;
+//       bool Equals(const elem_type& a, const Item& b) const;
 //
 //       /** @return True if (a < b); false otherwise. */
-//       bool LessThan(const elem_type& a, const elem_type& b) const;
+//       bool LessThan(const elem_type& a, const Item& b) const;
 //   };
 //
 // The Equals method is used for searching, and the LessThan method is used
-// for sorting.
+// for sorting.  The |Item| type above can be arbitrary, but must match the
+// Item type passed to the sort or search function.
 //
 // The Alloc template parameter can be used to choose between
 // "fallible" and "infallible" nsTArray (if available), defaulting to
 // fallible.  If the *fallible* allocator is used, the return value of
 // methods that might allocate needs to be checked; Append() is
 // one such method.  These return values don't need to be checked if
 // the *in*fallible allocator is chosen.  When in doubt, choose the
 // infallible allocator.