Bug 1401873 - Slim down nsAtom some more. r=froydnj. draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 21 Sep 2017 17:19:34 +1000
changeset 668167 a9623a347803d81761462fd7c3dd1f2552f422e1
parent 668166 a67aeed48a4fe3305afa83349c5e3ccd0f84d3f9
child 668168 94ddb8d4843f5b5a91ecb72539725b1f74ed62aa
push id80945
push usernnethercote@mozilla.com
push dateThu, 21 Sep 2017 07:22:53 +0000
reviewersfroydnj
bugs1401873
milestone57.0a1
Bug 1401873 - Slim down nsAtom some more. r=froydnj. This patch removes the factory methods, instead using friend declarations to expose the private constructors as necessary. This ensures we never create atoms from unexpected places. MozReview-Commit-ID: 5mIFUbpIbF7
xpcom/ds/nsAtomTable.cpp
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -106,31 +106,24 @@ private:
   nsStringBuffer* mBuffer;
 };
 
 UniquePtr<nsTArray<FakeBufferRefcountHelper>> gFakeBuffers;
 #endif
 
 class nsAtom final : public nsIAtom
 {
-public:
-  static already_AddRefed<nsAtom> CreateDynamic(const nsAString& aString,
-                                                uint32_t aHash)
-  {
-    // The refcount is appropriately initialized in the constructor.
-    return dont_AddRef(new nsAtom(aString, aHash));
-  }
+private:
+  // nsAtom constructors are private because they must be constructed in very
+  // restricted ways. The following functions are those responsible.
+  friend void RegisterStaticAtoms(const nsStaticAtom*, uint32_t);
+  friend already_AddRefed<nsIAtom> NS_Atomize(const nsACString&);
+  friend already_AddRefed<nsIAtom> NS_Atomize(const nsAString&);
+  friend already_AddRefed<nsIAtom> NS_AtomizeMainThread(const nsAString&);
 
-  static nsAtom* CreateStatic(nsStringBuffer* aStringBuffer, uint32_t aLength,
-                              uint32_t aHash)
-  {
-    return new nsAtom(aStringBuffer, aLength, aHash);
-  }
-
-private:
   // This constructor is for dynamic atoms.
   nsAtom(const nsAString& aString, uint32_t aHash)
     : mRefCnt(1)
   {
     mLength = aString.Length();
     SetKind(AtomKind::DynamicAtom);
     RefPtr<nsStringBuffer> buf = nsStringBuffer::FromString(aString);
     if (buf) {
@@ -680,17 +673,17 @@ RegisterStaticAtoms(const nsStaticAtom* 
       // C++ here, not Smalltalk.
       if (!atom->IsStaticAtom()) {
         nsAutoCString name;
         atom->ToUTF8String(name);
         MOZ_CRASH_UNSAFE_PRINTF(
           "Static atom registration for %s should be pushed back", name.get());
       }
     } else {
-      atom = nsAtom::CreateStatic(stringBuffer, stringLen, hash);
+      atom = new nsAtom(stringBuffer, stringLen, hash);
       he->mAtom = atom;
     }
     *atomp = atom;
 
     if (!gStaticAtomTableSealed) {
       StaticAtomEntry* entry =
         gStaticAtomTable->PutEntry(nsDependentAtomString(atom));
       MOZ_ASSERT(atom->IsStaticAtom());
@@ -720,17 +713,17 @@ NS_Atomize(const nsACString& aUTF8String
     return atom.forget();
   }
 
   // This results in an extra addref/release of the nsStringBuffer.
   // Unfortunately there doesn't seem to be any APIs to avoid that.
   // Actually, now there is, sort of: ForgetSharedBuffer.
   nsString str;
   CopyUTF8toUTF16(aUTF8String, str);
-  RefPtr<nsAtom> atom = nsAtom::CreateDynamic(str, hash);
+  RefPtr<nsAtom> atom = dont_AddRef(new nsAtom(str, hash));
 
   he->mAtom = atom;
 
   return atom.forget();
 }
 
 already_AddRefed<nsIAtom>
 NS_Atomize(const char16_t* aUTF16String)
@@ -748,17 +741,17 @@ NS_Atomize(const nsAString& aUTF16String
                                         &hash);
 
   if (he->mAtom) {
     nsCOMPtr<nsIAtom> atom = he->mAtom;
 
     return atom.forget();
   }
 
-  RefPtr<nsAtom> atom = nsAtom::CreateDynamic(aUTF16String, hash);
+  RefPtr<nsAtom> atom = dont_AddRef(new nsAtom(aUTF16String, hash));
   he->mAtom = atom;
 
   return atom.forget();
 }
 
 already_AddRefed<nsIAtom>
 NS_AtomizeMainThread(const nsAString& aUTF16String)
 {
@@ -779,17 +772,17 @@ NS_AtomizeMainThread(const nsAString& aU
   }
 
   MutexAutoLock lock(*gAtomTableLock);
   AtomTableEntry* he = static_cast<AtomTableEntry*>(gAtomTable->Add(&key));
 
   if (he->mAtom) {
     retVal = he->mAtom;
   } else {
-    RefPtr<nsAtom> newAtom = nsAtom::CreateDynamic(aUTF16String, hash);
+    RefPtr<nsAtom> newAtom = dont_AddRef(new nsAtom(aUTF16String, hash));
     he->mAtom = newAtom;
     retVal = newAtom.forget();
   }
 
   sRecentlyUsedMainThreadAtoms[index] = he->mAtom;
   return retVal.forget();
 }