root-atoms
author Benjamin Smedberg <benjamin@smedbergs.us>
Sat, 26 Jul 2008 22:49:39 -0400
changeset 167 a4da40849f5436e629c5732f4368c6c48189637f
parent 126 e40ba022b1fa24d2b032c85f3c64ea5a1c23bdd5
permissions -rw-r--r--
State as of now

* * *
* * *
* * *

diff --git a/xpcom/build/nsXPComInit.cpp b/xpcom/build/nsXPComInit.cpp
--- a/xpcom/build/nsXPComInit.cpp
+++ b/xpcom/build/nsXPComInit.cpp
@@ -373,7 +373,7 @@ static const nsModuleComponentInfo compo
                        nsConsoleService,
                        nsIClassInfo::THREADSAFE | nsIClassInfo::SINGLETON),
     COMPONENT(EXCEPTIONSERVICE, nsExceptionServiceConstructor),
-    COMPONENT(ATOMSERVICE, nsAtomServiceConstructor),
+    COMPONENT(ATOMSERVICE, nsAtomService::GetSingleton),
 #ifdef MOZ_TIMELINE
     COMPONENT(TIMELINESERVICE, nsTimelineServiceConstructor),
 #endif
@@ -502,6 +502,19 @@ NS_InitXPCOM3(nsIServiceManager* *result
 
     NS_LogInit();
 
+    // Get NS_RootUntilShutdown working early
+    NS_ASSERTION(nsComponentManagerImpl::gComponentManager == NULL, "CompMgr not null at init");
+
+    nsComponentManagerImpl::gComponentManager =
+        new nsComponentManagerImpl::Root();
+    if (!nsComponentManagerImpl::gComponentManager)
+        return NS_ERROR_OUT_OF_MEMORY;
+
+    // Root the atomservice/atoms before doing much anything else
+    void *dummy;
+    nsAtomService::GetSingleton(nsnull, NS_GET_IID(nsIAtomService),
+                                &dummy);
+
     // Establish the main thread here.
     rv = nsThreadManager::get()->Init();
     if (NS_FAILED(rv)) return rv;
@@ -556,8 +569,6 @@ NS_InitXPCOM3(nsIServiceManager* *result
         if (NS_FAILED(rv)) return rv;
     }
 
-    NS_ASSERTION(nsComponentManagerImpl::gComponentManager == NULL, "CompMgr not null at init");
-
     // Create the Component/Service Manager
     nsComponentManagerImpl *compMgr = new nsComponentManagerImpl();
     if (compMgr == NULL)
@@ -570,10 +581,7 @@ NS_InitXPCOM3(nsIServiceManager* *result
         return rv;
     }
 
-    nsComponentManagerImpl::gComponentManager =
-        new nsComponentManagerImpl::Root(compMgr);
-    if (!nsComponentManagerImpl::gComponentManager)
-        return NS_ERROR_OUT_OF_MEMORY;
+    nsComponentManagerImpl::gComponentManager->instance = compMgr;
 
     if (result) {
         nsIServiceManager *serviceManager =
diff --git a/xpcom/components/nsComponentManager.h b/xpcom/components/nsComponentManager.h
--- a/xpcom/components/nsComponentManager.h
+++ b/xpcom/components/nsComponentManager.h
@@ -171,9 +171,9 @@ public:
     class Root : public MMgc::GCRoot
     {
     public:
-        Root(nsComponentManagerImpl *compMgr)
+        Root()
             : MMgc::GCRoot(NS_GetGC())
-            , instance(compMgr)
+            , instance(nsnull)
         {
         }
 
diff --git a/xpcom/ds/nsAtomService.cpp b/xpcom/ds/nsAtomService.cpp
--- a/xpcom/ds/nsAtomService.cpp
+++ b/xpcom/ds/nsAtomService.cpp
@@ -42,6 +42,7 @@ NS_IMPL_THREADSAFE_ISUPPORTS1(nsAtomServ
 NS_IMPL_THREADSAFE_ISUPPORTS1(nsAtomService, nsIAtomService)
 
 nsAtomService::nsAtomService()
+  : MMgc::GCCallback(NS_GetGC())
 {
 }
 
@@ -88,3 +89,21 @@ nsAtomService::GetPermanentAtomUTF8(cons
 
     return NS_OK;
 }
+
+NS_METHOD
+nsAtomService::GetSingleton(nsISupports* outer,
+                            const nsIID& aIID,
+                            void **aInstancePtr)
+{
+    NS_ENSURE_NO_AGGREGATION(outer);
+
+    if (!gService) {
+        gService = new nsAtomService();
+        NS_RootUntilShutdown(gService);
+    }
+
+    return gService->QueryInterface(aIID, aInstancePtr);
+}
+
+nsAtomService*
+nsAtomService::gService;
diff --git a/xpcom/ds/nsAtomService.h b/xpcom/ds/nsAtomService.h
--- a/xpcom/ds/nsAtomService.h
+++ b/xpcom/ds/nsAtomService.h
@@ -41,7 +41,9 @@
 
 #include "nsIAtomService.h"
 
-class nsAtomService : public nsIAtomService
+class nsAtomService : public XPCOMGCFinalizedObject,
+                      public nsIAtomService,
+                      public MMgc::GCCallback
 {
  public:
   nsAtomService();
@@ -49,7 +51,15 @@ class nsAtomService : public nsIAtomServ
     
   NS_DECL_NSIATOMSERVICE
 
+  virtual bool CustomMark();
+  virtual void presweep();
+
+  static NS_METHOD GetSingleton(nsISupports* outer,
+                                const nsIID& aIID,
+                                void * *aInstancePtr);
  private:
+  static nsAtomService *gService;
+
   ~nsAtomService() {}
 };
 
diff --git a/xpcom/ds/nsAtomTable.cpp b/xpcom/ds/nsAtomTable.cpp
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -37,6 +37,7 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsAtomTable.h"
+#include "nsAtomService.h"
 #include "nsStaticAtom.h"
 #include "nsString.h"
 #include "nsReadableUtils.h"
@@ -45,11 +46,6 @@
 #include "pldhash.h"
 #include "prenv.h"
 #include "nsThreadUtils.h"
-
-#define PL_ARENA_CONST_ALIGN_MASK 3
-#include "plarena.h"
-
-class nsStaticAtomWrapper;
 
 /**
  * The shared hash table for atom lookups.
@@ -61,49 +57,6 @@ class nsStaticAtomWrapper;
  * If |gAtomTable.ops| is 0, then the table is uninitialized.
  */
 static PLDHashTable gAtomTable;
-
-// this is where we keep the nsStaticAtomWrapper objects
-
-static PLArenaPool* gStaticAtomArena = 0;
-
-class nsStaticAtomWrapper : public nsIAtom
-{
-public:
-  nsStaticAtomWrapper(const nsStaticAtom* aAtom, PRUint32 aLength) :
-    mStaticAtom(aAtom), mLength(aLength)
-  {
-    MOZ_COUNT_CTOR(nsStaticAtomWrapper);
-  }
-  ~nsStaticAtomWrapper() {   // no subclasses -> not virtual
-    // this is arena allocated and won't be called except in debug
-    // builds. If this function ever does anything non-debug, be sure
-    // to get rid of the ifdefs in AtomTableClearEntry!
-    MOZ_COUNT_DTOR(nsStaticAtomWrapper);
-  }
-
-  NS_IMETHOD QueryInterface(REFNSIID aIID,
-                            void** aInstancePtr);
-  NS_IMETHOD_(nsrefcnt) AddRef(void);
-  NS_IMETHOD_(nsrefcnt) Release(void);
-
-  NS_DECL_NSIATOM
-
-  const nsStaticAtom* GetStaticAtom() const {
-    return mStaticAtom;
-  }
-
-  PRUint32 getLength() const {
-    return mLength;
-  }
-
-private:
-  const nsStaticAtom* mStaticAtom;
-
-  // The length of the string in the static atom. The static atom
-  // (nsStaticAtom) doesn't hold a length, so we keep it here in the
-  // wrapper instead.
-  PRUint32 mLength;
-};
 
 // The |key| pointer in the various PLDHashTable callbacks we use is an
 // AtomTableClearEntry*.  These pointers can come from two places: either a
@@ -124,15 +77,12 @@ private:
 //
 // If keyHash is any other value (> 1), the AtomTableEntry is an
 // actual live entry in the table, and then mBits & ~0x1 in the
-// AtomTableEntry points to an AtomImpl or a nsStaticAtomWrapper,
-// indicated by the first bit of PtrBits.
+// AtomTableEntry points to an AtomImpl
 // XXX This whole mess could be vastly simplified now that pldhash
 // no longer has a getKey callback.
 typedef PRUword PtrBits;
 
 struct AtomTableEntry : public PLDHashEntryHdr {
-  // If keyHash > 1, mBits & 0x1 means (mBits & ~0x1) points to an
-  // nsStaticAtomWrapper else it points to an nsAtomImpl
   PtrBits mBits;
   PRUint32 mLength;
 
@@ -146,12 +96,6 @@ struct AtomTableEntry : public PLDHashEn
     : mBits(PtrBits(aString)), mLength(aLength)
   {
     keyHash = 1;
-  }
-
-  inline PRBool IsStaticAtom() const {
-    NS_ASSERTION(keyHash > 1,
-                 "IsStaticAtom() called on non-atom AtomTableEntry!");
-    return (mBits & 0x1) != 0;
   }
 
   inline PRBool IsUTF8String() const {
@@ -170,17 +114,6 @@ struct AtomTableEntry : public PLDHashEn
     mLength = aAtom->mLength;
   }
 
-  inline void SetStaticAtomWrapper(nsStaticAtomWrapper* aAtom) {
-    NS_ASSERTION(keyHash > 1,
-                 "SetStaticAtomWrapper() called on non-atom AtomTableEntry!");
-    NS_ASSERTION(aAtom, "Setting null atom");
-    NS_ASSERTION((PtrBits(aAtom) & ~0x1) == PtrBits(aAtom),
-                 "Pointers must align or this is broken");
-
-    mBits = PtrBits(aAtom) | 0x1;
-    mLength = aAtom->getLength();
-  }
-  
   inline void ClearAtom() {
     mBits = nsnull;
   }
@@ -188,28 +121,15 @@ struct AtomTableEntry : public PLDHashEn
   inline PRBool HasValue() const {
     NS_ASSERTION(keyHash > 1,
                  "HasValue() called on non-atom AtomTableEntry!");
-    return (mBits & ~0x1) != 0;
+    return mBits != 0;
   }
 
   // these accessors assume that you already know the type
   inline AtomImpl *GetAtomImpl() const {
     NS_ASSERTION(keyHash > 1,
                  "GetAtomImpl() called on non-atom AtomTableEntry!");
-    NS_ASSERTION(!IsStaticAtom(), "This is a static atom, not an AtomImpl");
-    return (AtomImpl*) (mBits & ~0x1);
-  }
 
-  inline nsStaticAtomWrapper *GetStaticAtomWrapper() const {
-    NS_ASSERTION(keyHash > 1,
-                 "GetStaticAtomWrapper() called on non-atom AtomTableEntry!");
-    NS_ASSERTION(IsStaticAtom(), "This is an AtomImpl, not a static atom");
-    return (nsStaticAtomWrapper*) (mBits & ~0x1);
-  }
-
-  inline const nsStaticAtom* GetStaticAtom() const {
-    NS_ASSERTION(keyHash > 1,
-                 "GetStaticAtom() called on non-atom AtomTableEntry!");
-    return GetStaticAtomWrapper()->GetStaticAtom();
+    return (AtomImpl*) mBits;
   }
 
   // type-agnostic accessors
@@ -219,7 +139,7 @@ struct AtomTableEntry : public PLDHashEn
     NS_ASSERTION(keyHash > 1,
                  "getAtomString() called on non-atom AtomTableEntry!");
 
-    return IsStaticAtom() ? GetStaticAtom()->mString : GetAtomImpl()->mString;
+    return GetAtomImpl()->mString;
   }
 
   // get the string buffer
@@ -241,24 +161,6 @@ struct AtomTableEntry : public PLDHashEn
   // get the string length
   inline PRUint32 getLength() const {
     return mLength;
-  }
-
-  // get an addreffed nsIAtom - not using already_AddRef'ed atom
-  // because the callers are not (and should not be) using nsCOMPtr
-  inline nsIAtom* GetAtom() const {
-    NS_ASSERTION(keyHash > 1,
-                 "GetAtom() called on non-atom AtomTableEntry!");
-
-    nsIAtom* result;
-    
-    if (IsStaticAtom())
-      result = GetStaticAtomWrapper();
-    else {
-      result = GetAtomImpl();
-      NS_ADDREF(result);
-    }
-    
-    return result;
   }
 };
 
@@ -309,32 +211,6 @@ AtomTableMatchKey(PLDHashTable *table, c
   return memcmp(atomString, str, length * sizeof(char)) == 0;
 }
 
-PR_STATIC_CALLBACK(void)
-AtomTableClearEntry(PLDHashTable *table, PLDHashEntryHdr *entry)
-{
-  AtomTableEntry *he = static_cast<AtomTableEntry*>(entry);
-  
-  if (!he->IsStaticAtom()) {
-    AtomImpl *atom = he->GetAtomImpl();
-    // Normal |AtomImpl| atoms are deleted when their refcount hits 0, and
-    // they then remove themselves from the table.  In other words, they
-    // are owned by the callers who own references to them.
-    // |PermanentAtomImpl| permanent atoms ignore their refcount and are
-    // deleted when they are removed from the table at table destruction.
-    // In other words, they are owned by the atom table.
-    if (atom->IsPermanent()) {
-      he->keyHash = 0;
-
-      delete static_cast<PermanentAtomImpl*>(atom);
-    }
-  }
-  else {
-    he->GetStaticAtomWrapper()->~nsStaticAtomWrapper();
-  }
-  
-  he->ClearAtom();
-}
-
 PR_STATIC_CALLBACK(PRBool)
 AtomTableInitEntry(PLDHashTable *table, PLDHashEntryHdr *entry,
                    const void *key)
@@ -354,7 +230,7 @@ static const PLDHashTableOps AtomTableOp
   AtomTableGetHash,
   AtomTableMatchKey,
   PL_DHashMoveEntryStub,
-  AtomTableClearEntry,
+  PL_DHashClearEntryStub,
   PL_DHashFinalizeStub,
   AtomTableInitEntry
 };
@@ -368,35 +244,16 @@ DumpAtomLeaks(PLDHashTable *table, PLDHa
 {
   AtomTableEntry *entry = static_cast<AtomTableEntry*>(he);
   
-  if (entry->IsStaticAtom())
-    return PL_DHASH_NEXT;
-  
   AtomImpl* atom = entry->GetAtomImpl();
-  if (!atom->IsPermanent()) {
-    ++*static_cast<PRUint32*>(arg);
-    const char *str;
-    atom->GetUTF8String(&str);
-    fputs(str, stdout);
-    fputs("\n", stdout);
-  }
+  ++*static_cast<PRUint32*>(arg);
+  const char *str;
+  atom->GetUTF8String(&str);
+  fputs(str, stdout);
+  fputs("\n", stdout);
   return PL_DHASH_NEXT;
 }
 
 #endif
-
-static inline
-void PromoteToPermanent(AtomImpl* aAtom)
-{
-#ifdef NS_BUILD_REFCNT_LOGGING
-  {
-    nsrefcnt refcount = aAtom->GetRefCount();
-    do {
-      NS_LOG_RELEASE(aAtom, --refcount, "AtomImpl");
-    } while (refcount);
-  }
-#endif
-  aAtom = new (aAtom) PermanentAtomImpl();
-}
 
 void
 NS_PurgeAtomTable()
@@ -414,100 +271,25 @@ NS_PurgeAtomTable()
     PL_DHashTableFinish(&gAtomTable);
     gAtomTable.entryCount = 0;
     gAtomTable.ops = nsnull;
-
-    if (gStaticAtomArena) {
-      PL_FinishArenaPool(gStaticAtomArena);
-      delete gStaticAtomArena;
-      gStaticAtomArena = nsnull;
-    }
   }
 }
 
-AtomImpl::AtomImpl()
+AtomImpl::AtomImpl(const nsACString &aString, PRBool aPermanent)
+  : mPermanent(aPermanent)
+  , mLength(aString.Length())
 {
+  const char *begin = aString.BeginReading();
+
+  memcpy(mString, begin, mLength);
+  mString[mLength] = '\0';
 }
 
-AtomImpl::~AtomImpl()
+NS_IMPL_QUERY_INTERFACE1(AtomImpl,
+                         nsIAtom)
+
+void* AtomImpl::operator new ( size_t size, size_t extra ) CPP_THROW_NEW
 {
-  NS_PRECONDITION(gAtomTable.ops, "uninitialized atom hashtable");
-  // Permanent atoms are removed from the hashtable at shutdown, and we
-  // don't want to remove them twice.  See comment above in
-  // |AtomTableClearEntry|.
-  if (!IsPermanentInDestructor()) {
-    AtomTableEntry key(mString, mLength);
-    PL_DHashTableOperate(&gAtomTable, &key, PL_DHASH_REMOVE);
-    if (gAtomTable.entryCount == 0) {
-      PL_DHashTableFinish(&gAtomTable);
-      NS_ASSERTION(gAtomTable.entryCount == 0,
-                   "PL_DHashTableFinish changed the entry count");
-    }
-  }
-}
-
-NS_IMPL_ISUPPORTS1(AtomImpl, nsIAtom)
-
-PermanentAtomImpl::PermanentAtomImpl()
-  : AtomImpl()
-{
-}
-
-PermanentAtomImpl::~PermanentAtomImpl()
-{
-  // So we can tell if we were permanent while running the base class dtor.
-}
-
-NS_IMETHODIMP_(nsrefcnt) PermanentAtomImpl::AddRef()
-{
-  NS_ASSERTION(NS_IsMainThread(), "wrong thread");
-  return 2;
-}
-
-NS_IMETHODIMP_(nsrefcnt) PermanentAtomImpl::Release()
-{
-  NS_ASSERTION(NS_IsMainThread(), "wrong thread");
-  return 1;
-}
-
-/* virtual */ PRBool
-AtomImpl::IsPermanent()
-{
-  return PR_FALSE;
-}
-
-/* virtual */ PRBool
-PermanentAtomImpl::IsPermanent()
-{
-  return PR_TRUE;
-}
-
-void* AtomImpl::operator new ( size_t size, const nsACString& aString ) CPP_THROW_NEW
-{
-    /*
-      Note: since the |size| will initially also include the |PRUnichar| member
-        |mString|, our size calculation will give us one character too many.
-        We use that extra character for a zero-terminator.
-
-      Note: this construction is not guaranteed to be possible by the C++
-        compiler.  A more reliable scheme is used by |nsShared[C]String|s, see
-        http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsSharedString.h#174
-     */
-  size += aString.Length() * sizeof(char);
-  AtomImpl* ii = static_cast<AtomImpl*>(::operator new(size));
-  NS_ENSURE_TRUE(ii, nsnull);
-
-  char* toBegin = &ii->mString[0];
-  nsACString::const_iterator fromBegin, fromEnd;
-  *copy_string(aString.BeginReading(fromBegin), aString.EndReading(fromEnd), toBegin) = '\0';
-  ii->mLength = aString.Length();
-  return ii;
-}
-
-void* PermanentAtomImpl::operator new ( size_t size, AtomImpl* aAtom ) CPP_THROW_NEW {
-  NS_ASSERTION(!aAtom->IsPermanent(),
-               "converting atom that's already permanent");
-
-  // Just let the constructor overwrite the vtable pointer.
-  return aAtom;
+  return MMgc::GCFinalizedObject::operator new(size, NS_GetGC(), extra);
 }
 
 NS_IMETHODIMP 
@@ -549,86 +331,6 @@ AtomImpl::Equals(const nsAString& aStrin
 
 //----------------------------------------------------------------------
 
-// wrapper class for the nsStaticAtom struct
-
-NS_IMETHODIMP_(nsrefcnt)
-nsStaticAtomWrapper::AddRef()
-{
-  NS_ASSERTION(NS_IsMainThread(), "wrong thread");
-  return 2;
-}
-
-NS_IMETHODIMP_(nsrefcnt)
-nsStaticAtomWrapper::Release()
-{
-  NS_ASSERTION(NS_IsMainThread(), "wrong thread");
-  return 1;
-}
-
-NS_IMPL_QUERY_INTERFACE1(nsStaticAtomWrapper, nsIAtom)
-
-NS_IMETHODIMP
-nsStaticAtomWrapper::GetUTF8String(const char** aResult)
-{
-  *aResult = mStaticAtom->mString;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsStaticAtomWrapper::ToString(nsAString& aBuf)
-{
-  // static should always be always ASCII, to allow tools like gperf
-  // to generate the tables, and to avoid unnecessary conversion
-  NS_ASSERTION(nsCRT::IsAscii(mStaticAtom->mString),
-               "Data loss - atom should be ASCII");
-  CopyASCIItoUTF16(nsDependentCString(mStaticAtom->mString, mLength), aBuf);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsStaticAtomWrapper::ToUTF8String(nsACString& aBuf)
-{
-  aBuf.Assign(mStaticAtom->mString);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsStaticAtomWrapper::EqualsUTF8(const nsACString& aString, PRBool* aResult)
-{
-  *aResult = aString.Equals(nsDependentCString(mStaticAtom->mString, mLength));
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsStaticAtomWrapper::Equals(const nsAString& aString, PRBool* aResult)
-{
-  *aResult = CompareUTF8toUTF16(nsDependentCString(mStaticAtom->mString,
-                                                   mLength),
-                                aString) == 0;
-  return NS_OK;
-}
-//----------------------------------------------------------------------
-
-static nsStaticAtomWrapper*
-WrapStaticAtom(const nsStaticAtom* aAtom, PRUint32 aLength)
-{
-  if (!gStaticAtomArena) {
-    gStaticAtomArena = new PLArenaPool;
-    if (!gStaticAtomArena)
-      return nsnull;
-    
-    PL_INIT_ARENA_POOL(gStaticAtomArena, "nsStaticAtomArena", 4096);
-  }
-
-  void* mem;
-  PL_ARENA_ALLOCATE(mem, gStaticAtomArena, sizeof(nsStaticAtomWrapper));
-
-  nsStaticAtomWrapper* wrapper =
-    new (mem) nsStaticAtomWrapper(aAtom, aLength);
-
-  return wrapper;
-}
-
 #define ATOM_HASHTABLE_INITIAL_SIZE  4096
 
 static inline AtomTableEntry*
@@ -663,58 +365,67 @@ GetAtomHashEntry(const PRUnichar* aStrin
                     (PL_DHashTableOperate(&gAtomTable, &key, PL_DHASH_ADD));
 }
 
+static PLDHashOperator
+MarkPermanentAtoms(PLDHashTable *table, PLDHashEntryHdr *hdr,
+                   PRUint32 number, void *arg)
+{
+  AtomTableEntry *entry = static_cast<AtomTableEntry*>(hdr);
+  AtomImpl *impl = entry->GetAtomImpl();
+
+  if (impl && impl->mPermanent) {
+    NS_GetGC()->SetMark(impl);
+  }
+
+  return PL_DHASH_NEXT;
+}
+
+bool
+nsAtomService::CustomMark()
+{
+  NS_GetGC()->SetMark(this);
+
+  // We don't have any interesting members... we just want to mark permanent
+  // atoms in our table.
+  if (gAtomTable.ops)
+    PL_DHashTableEnumerate(&gAtomTable, MarkPermanentAtoms, NULL);
+
+  return true;
+}
+
+static PLDHashOperator
+SweepDyingAtoms(PLDHashTable *table, PLDHashEntryHdr *hdr,
+                PRUint32 number, void *arg)
+{
+  AtomTableEntry *entry = static_cast<AtomTableEntry*>(hdr);
+  AtomImpl *impl = entry->GetAtomImpl();
+  
+  if (!impl || NS_GetGC()->GetMark(impl)) {
+    return PL_DHASH_NEXT;
+  }
+
+  return PL_DHASH_REMOVE;
+}
+
+void
+nsAtomService::presweep()
+{
+  if (gAtomTable.ops)
+    PL_DHashTableEnumerate(&gAtomTable, SweepDyingAtoms, NULL);
+}
+
 NS_COM nsresult
 NS_RegisterStaticAtoms(const nsStaticAtom* aAtoms, PRUint32 aAtomCount)
 {
-  // this does two things:
-  // 1) wraps each static atom in a wrapper, if necessary
-  // 2) initializes the address pointed to by each mBits slot
-  
   for (PRUint32 i=0; i<aAtomCount; i++) {
-    NS_ASSERTION(nsCRT::IsAscii(aAtoms[i].mString),
-                 "Static atoms must be ASCII!");
-
-    PRUint32 stringLen = strlen(aAtoms[i].mString);
-
-    AtomTableEntry *he =
-      GetAtomHashEntry(aAtoms[i].mString, stringLen);
-
-    if (he->HasValue() && aAtoms[i].mAtom) {
-      // there already is an atom with this name in the table.. but we
-      // still have to update mBits
-      if (!he->IsStaticAtom() && !he->GetAtomImpl()->IsPermanent()) {
-        // since we wanted to create a static atom but there is
-        // already one there, we convert it to a non-refcounting
-        // permanent atom
-        PromoteToPermanent(he->GetAtomImpl());
-      }
-      
-      // and now, if the consumer wants to remember this value in a
-      // slot, we do so
-      if (aAtoms[i].mAtom)
-        *aAtoms[i].mAtom = he->GetAtom();
-    }
-    else {
-      nsStaticAtomWrapper* atom = WrapStaticAtom(&aAtoms[i], stringLen);
-      NS_ASSERTION(atom, "Failed to wrap static atom");
-
-      // but even if atom is null, no real difference in code..
-      he->SetStaticAtomWrapper(atom);
-      if (aAtoms[i].mAtom)
-        *aAtoms[i].mAtom = atom;
+    if (aAtoms[i].mAtom) {
+      *aAtoms[i].mAtom = do_GetPermanentAtom(aAtoms[i].mString);
     }
   }
   return NS_OK;
 }
 
 NS_COM nsIAtom*
-NS_NewAtom(const char* aUTF8String)
-{
-  return NS_NewAtom(nsDependentCString(aUTF8String));
-}
-
-NS_COM nsIAtom*
-NS_NewAtom(const nsACString& aUTF8String)
+NS_NewAtom(const nsACString& aUTF8String, PRBool aPermanent)
 {
   AtomTableEntry *he = GetAtomHashEntry(aUTF8String.Data(),
                                         aUTF8String.Length());
@@ -726,17 +437,16 @@ NS_NewAtom(const nsACString& aUTF8String
   NS_ASSERTION(!he->IsUTF8String() && !he->IsUTF16String(),
                "Atom hash entry is string?  Should be atom!");
 
-  if (he->HasValue())
-    return he->GetAtom();
+  if (he->HasValue()) {
+    AtomImpl *impl = he->GetAtomImpl();
+    if (aPermanent)
+      impl->mPermanent = PR_TRUE;
 
-  AtomImpl* atom = new (aUTF8String) AtomImpl();
-  he->SetAtomImpl(atom);
-  if (!atom) {
-    PL_DHashTableRawRemove(&gAtomTable, he);
-    return nsnull;
+    return impl;
   }
 
-  NS_ADDREF(atom);
+  AtomImpl* atom = new (aUTF8String.Length()) AtomImpl(aUTF8String, aPermanent);
+  he->SetAtomImpl(atom);
   return atom;
 }
 
@@ -747,76 +457,27 @@ NS_NewAtom(const PRUnichar* aUTF16String
 }
 
 NS_COM nsIAtom*
-NS_NewAtom(const nsAString& aUTF16String)
+NS_NewAtom(const nsAString& aUTF16String, PRBool aPermanent)
 {
   AtomTableEntry *he = GetAtomHashEntry(aUTF16String.Data(),
                                         aUTF16String.Length());
 
-  if (he->HasValue())
-    return he->GetAtom();
+  if (he->HasValue()) {
+    AtomImpl *impl = he->GetAtomImpl();
+
+    if (aPermanent)
+      impl->mPermanent = PR_TRUE;
+
+    return impl;
+  }
 
   // MSVC.NET doesn't like passing a temporary NS_ConvertUTF16toUTF8() to
   // operator new, so declare one as a local instead.
   NS_ConvertUTF16toUTF8 str(aUTF16String);
-  AtomImpl* atom = new (str) AtomImpl();
+  AtomImpl* atom = new (str.Length()) AtomImpl(str, aPermanent);
   he->SetAtomImpl(atom);
-  if (!atom) {
-    PL_DHashTableRawRemove(&gAtomTable, he);
-    return nsnull;
-  }
 
-  NS_ADDREF(atom);
   return atom;
-}
-
-NS_COM nsIAtom*
-NS_NewPermanentAtom(const char* aUTF8String)
-{
-  return NS_NewPermanentAtom(nsDependentCString(aUTF8String));
-}
-
-NS_COM nsIAtom*
-NS_NewPermanentAtom(const nsACString& aUTF8String)
-{
-  AtomTableEntry *he = GetAtomHashEntry(aUTF8String.Data(),
-                                        aUTF8String.Length());
-
-  if (he->HasValue() && he->IsStaticAtom())
-    return he->GetStaticAtomWrapper();
-  
-  // either there is no atom and we'll create an AtomImpl,
-  // or there is an existing AtomImpl
-  AtomImpl* atom = he->GetAtomImpl();
-  
-  if (atom) {
-    // ensure that it's permanent
-    if (!atom->IsPermanent()) {
-      PromoteToPermanent(atom);
-    }
-  } else {
-    // otherwise, make a new atom
-    atom = new (aUTF8String) PermanentAtomImpl();
-    he->SetAtomImpl(atom);
-    if ( !atom ) {
-      PL_DHashTableRawRemove(&gAtomTable, he);
-      return nsnull;
-    }
-  }
-
-  NS_ADDREF(atom);
-  return atom;
-}
-
-NS_COM nsIAtom*
-NS_NewPermanentAtom(const nsAString& aUTF16String)
-{
-  return NS_NewPermanentAtom(NS_ConvertUTF16toUTF8(aUTF16String));
-}
-
-NS_COM nsIAtom*
-NS_NewPermanentAtom(const PRUnichar* aUTF16String)
-{
-  return NS_NewPermanentAtom(NS_ConvertUTF16toUTF8(aUTF16String));
 }
 
 NS_COM nsrefcnt
diff --git a/xpcom/ds/nsAtomTable.h b/xpcom/ds/nsAtomTable.h
--- a/xpcom/ds/nsAtomTable.h
+++ b/xpcom/ds/nsAtomTable.h
@@ -46,37 +46,17 @@
  * objects using placement new and just overwriting the vtable pointer.
  */
 
-class AtomImpl : public nsIAtom {
+class AtomImpl : public XPCOMGCObject, public nsIAtom {
 public:
-  AtomImpl();
-
-protected:
-  // We don't need a virtual destructor here because PermanentAtomImpl
-  // deletions aren't handled through Release().
-  ~AtomImpl();
+  AtomImpl(const nsACString &aString, PRBool aPermanent);
 
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIATOM
 
-  enum { REFCNT_PERMANENT_SENTINEL = PR_UINT32_MAX };
+  void* operator new(size_t size, size_t extra) CPP_THROW_NEW;
 
-  virtual PRBool IsPermanent();
-
-  // We can't use the virtual function in the base class destructor.
-  PRBool IsPermanentInDestructor() {
-    // XXXbsmedberg: going away next!
-    return PR_TRUE;
-  }
-
-  void* operator new(size_t size, const nsACString& aString) CPP_THROW_NEW;
-
-  void operator delete(void* ptr) {
-    ::operator delete(ptr);
-  }
-
-  // for |#ifdef NS_BUILD_REFCNT_LOGGING| access to reference count
-  nsrefcnt GetRefCount() { return 1; }
+  PRBool mPermanent;
 
   // The length of the string in the atom.
   PRUint32 mLength;
@@ -86,26 +66,6 @@ public:
   char mString[1];
 };
 
-/**
- * A non-refcounted implementation of nsIAtom.
- */
-
-class PermanentAtomImpl : public AtomImpl {
-public:
-  PermanentAtomImpl();
-  ~PermanentAtomImpl();
-  NS_IMETHOD_(nsrefcnt) AddRef();
-  NS_IMETHOD_(nsrefcnt) Release();
-
-  virtual PRBool IsPermanent();
-
-  void* operator new(size_t size, const nsACString& aString) CPP_THROW_NEW {
-    return AtomImpl::operator new(size, aString);
-  }
-  void* operator new(size_t size, AtomImpl* aAtom) CPP_THROW_NEW;
-
-};
-
 void NS_PurgeAtomTable();
 
 #endif // nsAtomTable_h__
diff --git a/xpcom/ds/nsIAtom.idl b/xpcom/ds/nsIAtom.idl
--- a/xpcom/ds/nsIAtom.idl
+++ b/xpcom/ds/nsIAtom.idl
@@ -105,13 +105,36 @@ interface nsIAtom : nsISupports
  * count, which requires locking and hurts performance.
  */
 
+/**
+ * Find an atom that matches the given UTF-8 string.
+ */
+extern NS_COM nsIAtom* NS_NewAtom(const nsACString& aUTF8String,
+                                  PRBool aPermanent = PR_FALSE);
+
+inline nsIAtom* NS_NewPermanentAtom(const nsACString& aUTF8String)
+{
+  return NS_NewAtom(aUTF8String, PR_TRUE);
+}
+
+inline nsIAtom* do_GetAtom(const nsACString& aUTF8String)
+    { return NS_NewAtom(aUTF8String); }
+inline nsIAtom* do_GetPermanentAtom(const nsACString& aUTF8String)
+    { return NS_NewPermanentAtom(aUTF8String); }
 
 /**
  * Find an atom that matches the given UTF-8 string.
  * The string is assumed to be zero terminated.
  */
-extern NS_COM nsIAtom* NS_NewAtom(const char* aUTF8String);
-extern NS_COM nsIAtom* NS_NewPermanentAtom(const char* aUTF8String);
+inline nsIAtom* NS_NewAtom(const char* aUTF8String,
+                           PRBool aPermanent = PR_FALSE)
+{
+  return NS_NewAtom(nsDependentCString(aUTF8String), aPermanent);
+}
+
+inline nsIAtom* NS_NewPermanentAtom(const char* aUTF8String)
+{
+  return NS_NewAtom(aUTF8String, PR_TRUE);
+}
 
 inline nsIAtom* do_GetAtom(const char* aUTF8String)
     { return NS_NewAtom(aUTF8String); }
@@ -119,37 +142,39 @@ inline nsIAtom* do_GetPermanentAtom(cons
     { return NS_NewPermanentAtom(aUTF8String); }
  
 /**
- * Find an atom that matches the given UTF-8 string.
+ * Find an atom that matches the given UTF-16 string.
  */
-extern NS_COM nsIAtom* NS_NewAtom(const nsACString& aUTF8String);
-extern NS_COM nsIAtom* NS_NewPermanentAtom(const nsACString& aUTF8String);
+extern NS_COM nsIAtom* NS_NewAtom(const nsAString& aUTF16String,
+                                  PRBool aPermanent = PR_FALSE);
 
-inline nsIAtom* do_GetAtom(const nsACString& aUTF8String)
-    { return NS_NewAtom(aUTF8String); }
-inline nsIAtom* do_GetPermanentAtom(const nsACString& aUTF8String)
-    { return NS_NewPermanentAtom(aUTF8String); }
+inline nsIAtom* NS_NewPermanentAtom(const nsAString& aUTF16String)
+{
+  return NS_NewAtom(aUTF16String, PR_TRUE);
+}
+
+inline nsIAtom* do_GetAtom(const nsAString& aUTF16String)
+    { return NS_NewAtom(aUTF16String); }
+inline nsIAtom* do_GetPermanentAtom(const nsAString& aUTF16String)
+    { return NS_NewPermanentAtom(aUTF16String); }
 
 /**
  * Find an atom that matches the given UTF-16 string.
  * The string is assumed to be zero terminated.
  */
-extern NS_COM nsIAtom* NS_NewAtom(const PRUnichar* aUTF16String);
-extern NS_COM nsIAtom* NS_NewPermanentAtom(const PRUnichar* aUTF16String);
+inline nsIAtom* NS_NewAtom(const PRUnichar* aUTF16String,
+                           PRBool aPermanent = PR_FALSE)
+{
+  return NS_NewAtom(nsDependentString(aUTF16String), aPermanent);
+}
+
+inline nsIAtom* NS_NewPermanentAtom(const PRUnichar* aUTF16String)
+{
+  return NS_NewAtom(aUTF16String, PR_TRUE);
+}
 
 inline nsIAtom* do_GetAtom(const PRUnichar* aUTF16String)
     { return NS_NewAtom(aUTF16String); }
 inline nsIAtom* do_GetPermanentAtom(const PRUnichar* aUTF16String)
-    { return NS_NewPermanentAtom(aUTF16String); }
-
-/**
- * Find an atom that matches the given UTF-16 string.
- */
-extern NS_COM nsIAtom* NS_NewAtom(const nsAString& aUTF16String);
-extern NS_COM nsIAtom* NS_NewPermanentAtom(const nsAString& aUTF16String);
-
-inline nsIAtom* do_GetAtom(const nsAString& aUTF16String)
-    { return NS_NewAtom(aUTF16String); }
-inline nsIAtom* do_GetPermanentAtom(const nsAString& aUTF16String)
     { return NS_NewPermanentAtom(aUTF16String); }
 
 /**
diff --git a/xpcom/ds/nsStaticAtom.h b/xpcom/ds/nsStaticAtom.h
--- a/xpcom/ds/nsStaticAtom.h
+++ b/xpcom/ds/nsStaticAtom.h
@@ -50,12 +50,12 @@
 //          someone asks for this in unicode
 // mAtom:   a convienience pointer - if you want to store the value of
 //          the atom created by this structure somewhere, put its
-//          address here
+//          address here; NOTE: this address should be managed by GC
+//          in order to keep the atom alive
 struct nsStaticAtom {
     const char* mString;
     nsIAtom ** mAtom;
 };
-
 
 // register your lookup function with the atom table. Your function
 // will be called when at atom is not found in the main atom table.