Bug 1166598 (part 6) - Clean up nsStaticCaseInsensitiveNameTable. r=froydnj.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 05 May 2015 21:13:53 -0700
changeset 277792 d2b79df6cb618d50c80e4c6b5a577e8c171ed1f7
parent 277791 5a9cc200b499fa96275d95386f226e67e2756a72
child 277793 b9eaaca25256cdb68df930f5938e16b7a10a573e
push id897
push userjlund@mozilla.com
push dateMon, 14 Sep 2015 18:56:12 +0000
treeherdermozilla-release@9411e2d2b214 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1166598
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1166598 (part 6) - Clean up nsStaticCaseInsensitiveNameTable. r=froydnj. This patch simplifies nsStaticCaseInsensitiveNameTable greatly by taking advantage of the following observations. - |new| is infallible, so |new nsStaticCaseInsensitiveNameTable()| calls don't need their return value checked. - nsStaticCaseInsensitiveNameTable::Init() checks if any of the added entries differ only in case, so the callers of that function don't need to do the same check. - nsStaticCaseInsensitiveNameTable::Init() never returns false because moz_xmalloc() is infallible. (Its callers never check the return value anyway.) So it can be merged into the constructor. And ~nsStaticCaseInsensitiveNameTable() need not null-check |mNameArray|. - PLDHashTable now has an initializing constructor and destructor, so these can be used in nsStaticCaseInsensitiveNameTable, thus avoiding manual PLD_HashTable{Init,Finish}() calls.
gfx/src/nsColor.cpp
layout/style/nsCSSKeywords.cpp
layout/style/nsCSSProps.cpp
xpcom/ds/nsStaticNameTable.cpp
xpcom/ds/nsStaticNameTable.h
--- a/gfx/src/nsColor.cpp
+++ b/gfx/src/nsColor.cpp
@@ -34,31 +34,18 @@ static const nscolor kColors[] = {
 #define eColorName_UNKNOWN (-1)
 
 static nsStaticCaseInsensitiveNameTable* gColorTable = nullptr;
 
 void nsColorNames::AddRefTable(void) 
 {
   NS_ASSERTION(!gColorTable, "pre existing array!");
   if (!gColorTable) {
-    gColorTable = new nsStaticCaseInsensitiveNameTable();
-    if (gColorTable) {
-#ifdef DEBUG
-    {
-      // let's verify the table...
-      for (uint32_t index = 0; index < eColorName_COUNT; ++index) {
-        nsAutoCString temp1(kColorNames[index]);
-        nsAutoCString temp2(kColorNames[index]);
-        ToLowerCase(temp1);
-        NS_ASSERTION(temp1.Equals(temp2), "upper case char in table");
-      }
-    }
-#endif      
-      gColorTable->Init(kColorNames, eColorName_COUNT); 
-    }
+    gColorTable =
+      new nsStaticCaseInsensitiveNameTable(kColorNames, eColorName_COUNT);
   }
 }
 
 void nsColorNames::ReleaseTable(void)
 {
   if (gColorTable) {
     delete gColorTable;
     gColorTable = nullptr;
--- a/layout/style/nsCSSKeywords.cpp
+++ b/layout/style/nsCSSKeywords.cpp
@@ -22,34 +22,27 @@ const char* const kCSSRawKeywords[] = {
 static int32_t gKeywordTableRefCount;
 static nsStaticCaseInsensitiveNameTable* gKeywordTable;
 
 void
 nsCSSKeywords::AddRefTable(void) 
 {
   if (0 == gKeywordTableRefCount++) {
     NS_ASSERTION(!gKeywordTable, "pre existing array!");
-    gKeywordTable = new nsStaticCaseInsensitiveNameTable();
-    if (gKeywordTable) {
+    gKeywordTable =
+      new nsStaticCaseInsensitiveNameTable(kCSSRawKeywords, eCSSKeyword_COUNT);
 #ifdef DEBUG
-    {
-      // let's verify the table...
-      int32_t index = 0;
-      for (; index < eCSSKeyword_COUNT && kCSSRawKeywords[index]; ++index) {
-        nsAutoCString temp1(kCSSRawKeywords[index]);
-        nsAutoCString temp2(kCSSRawKeywords[index]);
-        ToLowerCase(temp1);
-        NS_ASSERTION(temp1.Equals(temp2), "upper case char in table");
-        NS_ASSERTION(-1 == temp1.FindChar('_'), "underscore char in table");
-      }
-      NS_ASSERTION(index == eCSSKeyword_COUNT, "kCSSRawKeywords and eCSSKeyword_COUNT are out of sync");
+    // Partially verify the entries.
+    int32_t index = 0;
+    for (; index < eCSSKeyword_COUNT && kCSSRawKeywords[index]; ++index) {
+      nsAutoCString temp(kCSSRawKeywords[index]);
+      NS_ASSERTION(-1 == temp.FindChar('_'), "underscore char in table");
     }
-#endif      
-      gKeywordTable->Init(kCSSRawKeywords, eCSSKeyword_COUNT); 
-    }
+    NS_ASSERTION(index == eCSSKeyword_COUNT, "kCSSRawKeywords and eCSSKeyword_COUNT are out of sync");
+#endif
   }
 }
 
 void
 nsCSSKeywords::ReleaseTable(void) 
 {
   if (0 == --gKeywordTableRefCount) {
     if (gKeywordTable) {
--- a/layout/style/nsCSSProps.cpp
+++ b/layout/style/nsCSSProps.cpp
@@ -129,32 +129,25 @@ static nsCSSProperty gAliases[eCSSAliasC
   eCSSProperty_##propid_ ,
 #include "nsCSSPropAliasList.h"
 #undef CSS_PROP_ALIAS
 };
 
 nsStaticCaseInsensitiveNameTable*
 CreateStaticTable(const char* const aRawTable[], int32_t aLength)
 {
-  auto table = new nsStaticCaseInsensitiveNameTable();
-  if (table) {
+  auto table = new nsStaticCaseInsensitiveNameTable(aRawTable, aLength);
 #ifdef DEBUG
-    // let's verify the table...
-    for (int32_t index = 0; index < aLength; ++index) {
-      nsAutoCString temp1(aRawTable[index]);
-      nsAutoCString temp2(aRawTable[index]);
-      ToLowerCase(temp1);
-      MOZ_ASSERT(temp1.Equals(temp2),
-                 "upper case char in case insensitive name table");
-      MOZ_ASSERT(-1 == temp1.FindChar('_'),
-                 "underscore char in case insensitive name table");
-    }
+  // Partially verify the entries.
+  for (int32_t index = 0; index < aLength; ++index) {
+    nsAutoCString temp(aRawTable[index]);
+    MOZ_ASSERT(-1 == temp.FindChar('_'),
+               "underscore char in case insensitive name table");
+  }
 #endif
-    table->Init(aRawTable, aLength);
-  }
   return table;
 }
 
 void
 nsCSSProps::AddRefTable(void)
 {
   if (0 == gPropertyTableRefCount++) {
     MOZ_ASSERT(!gPropertyTable, "pre existing array!");
--- a/xpcom/ds/nsStaticNameTable.cpp
+++ b/xpcom/ds/nsStaticNameTable.cpp
@@ -96,68 +96,43 @@ caseInsensitiveStringHashKey(PLDHashTabl
 static const struct PLDHashTableOps nametable_CaseInsensitiveHashTableOps = {
   caseInsensitiveStringHashKey,
   matchNameKeysCaseInsensitive,
   PL_DHashMoveEntryStub,
   PL_DHashClearEntryStub,
   nullptr,
 };
 
-nsStaticCaseInsensitiveNameTable::nsStaticCaseInsensitiveNameTable()
+nsStaticCaseInsensitiveNameTable::nsStaticCaseInsensitiveNameTable(
+    const char* const aNames[], int32_t aLength)
   : mNameArray(nullptr)
+  , mNameTable(&nametable_CaseInsensitiveHashTableOps,
+               sizeof(NameTableEntry), aLength)
   , mNullStr("")
 {
   MOZ_COUNT_CTOR(nsStaticCaseInsensitiveNameTable);
-}
 
-nsStaticCaseInsensitiveNameTable::~nsStaticCaseInsensitiveNameTable()
-{
-  if (mNameArray) {
-    // manually call the destructor on placement-new'ed objects
-    for (uint32_t index = 0; index < mNameTable.EntryCount(); index++) {
-      mNameArray[index].~nsDependentCString();
-    }
-    free((void*)mNameArray);
-  }
-  if (mNameTable.IsInitialized()) {
-    PL_DHashTableFinish(&mNameTable);
-  }
-  MOZ_COUNT_DTOR(nsStaticCaseInsensitiveNameTable);
-}
-
-bool
-nsStaticCaseInsensitiveNameTable::Init(const char* const aNames[],
-                                       int32_t aLength)
-{
-  NS_ASSERTION(!mNameArray, "double Init");
-  NS_ASSERTION(!mNameTable.IsInitialized(), "double Init");
-  NS_ASSERTION(aNames, "null name table");
-  NS_ASSERTION(aLength, "0 length");
+  MOZ_ASSERT(aNames, "null name table");
+  MOZ_ASSERT(aLength, "0 length");
 
   mNameArray = (nsDependentCString*)
     moz_xmalloc(aLength * sizeof(nsDependentCString));
-  if (!mNameArray) {
-    return false;
-  }
-
-  PL_DHashTableInit(&mNameTable, &nametable_CaseInsensitiveHashTableOps,
-                    sizeof(NameTableEntry), aLength);
 
   for (int32_t index = 0; index < aLength; ++index) {
     const char* raw = aNames[index];
 #ifdef DEBUG
     {
       // verify invariants of contents
       nsAutoCString temp1(raw);
       nsDependentCString temp2(raw);
       ToLowerCase(temp1);
-      NS_ASSERTION(temp1.Equals(temp2), "upper case char in table");
-      NS_ASSERTION(nsCRT::IsAscii(raw),
-                   "non-ascii string in table -- "
-                   "case-insensitive matching won't work right");
+      MOZ_ASSERT(temp1.Equals(temp2), "upper case char in table");
+      MOZ_ASSERT(nsCRT::IsAscii(raw),
+                 "non-ascii string in table -- "
+                 "case-insensitive matching won't work right");
     }
 #endif
     // use placement-new to initialize the string object
     nsDependentCString* strPtr = &mNameArray[index];
     new (strPtr) nsDependentCString(raw);
 
     NameTableKey key(strPtr);
 
@@ -170,17 +145,26 @@ nsStaticCaseInsensitiveNameTable::Init(c
     NS_ASSERTION(entry->mString == 0, "Entry already exists!");
 
     entry->mString = strPtr;      // not owned!
     entry->mIndex = index;
   }
 #ifdef DEBUG
   PL_DHashMarkTableImmutable(&mNameTable);
 #endif
-  return true;
+}
+
+nsStaticCaseInsensitiveNameTable::~nsStaticCaseInsensitiveNameTable()
+{
+  // manually call the destructor on placement-new'ed objects
+  for (uint32_t index = 0; index < mNameTable.EntryCount(); index++) {
+    mNameArray[index].~nsDependentCString();
+  }
+  free((void*)mNameArray);
+  MOZ_COUNT_DTOR(nsStaticCaseInsensitiveNameTable);
 }
 
 int32_t
 nsStaticCaseInsensitiveNameTable::Lookup(const nsACString& aName)
 {
   NS_ASSERTION(mNameArray, "not inited");
   NS_ASSERTION(mNameTable.IsInitialized(), "not inited");
 
--- a/xpcom/ds/nsStaticNameTable.h
+++ b/xpcom/ds/nsStaticNameTable.h
@@ -28,23 +28,22 @@
  *    as long as this table object - typically a static string array.
  */
 
 class nsStaticCaseInsensitiveNameTable
 {
 public:
   enum { NOT_FOUND = -1 };
 
-  bool             Init(const char* const aNames[], int32_t aLength);
   int32_t          Lookup(const nsACString& aName);
   int32_t          Lookup(const nsAString& aName);
   const nsAFlatCString& GetStringValue(int32_t aIndex);
 
-  nsStaticCaseInsensitiveNameTable();
+  nsStaticCaseInsensitiveNameTable(const char* const aNames[], int32_t aLength);
   ~nsStaticCaseInsensitiveNameTable();
 
 private:
   nsDependentCString*   mNameArray;
-  PLDHashTable mNameTable;
+  PLDHashTable2         mNameTable;
   nsDependentCString    mNullStr;
 };
 
 #endif /* nsStaticNameTable_h___ */