Bug 1362338: Make nsIAtom::AddRef and nsIAtom::Release final. r?froydnj draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 08 Aug 2017 18:47:04 +0200
changeset 642668 0ae6eae36dae81523f3732fd882780fecfe42627
parent 642667 9b4f5bdd078a70f6ed4535a2fd7a9a8b93bf6105
child 725068 a4aa18d5a3da0a56f36e944a332bb32683cd1520
push id72833
push userbmo:emilio+bugs@crisal.io
push dateTue, 08 Aug 2017 16:50:16 +0000
reviewersfroydnj
bugs1362338
milestone57.0a1
Bug 1362338: Make nsIAtom::AddRef and nsIAtom::Release final. r?froydnj I can put a bit more effort in deduplicating the QueryInterface / mRefCnt declarations if needed. MozReview-Commit-ID: CFWcSNocGIm
parser/html/nsHtml5Atom.cpp
parser/html/nsHtml5Atom.h
xpcom/ds/nsAtomTable.cpp
xpcom/ds/nsIAtom.idl
--- a/parser/html/nsHtml5Atom.cpp
+++ b/parser/html/nsHtml5Atom.cpp
@@ -35,38 +35,24 @@ nsHtml5Atom::nsHtml5Atom(const nsAString
   mozilla::Unused << buf.forget();
 }
 
 nsHtml5Atom::~nsHtml5Atom()
 {
   nsStringBuffer::FromData(mString)->Release();
 }
 
-NS_IMETHODIMP_(MozExternalRefCountType)
-nsHtml5Atom::AddRef()
-{
-  NS_NOTREACHED("Attempt to AddRef an nsHtml5Atom.");
-  return 2;
-}
-
-NS_IMETHODIMP_(MozExternalRefCountType)
-nsHtml5Atom::Release()
-{
-  NS_NOTREACHED("Attempt to Release an nsHtml5Atom.");
-  return 1;
-}
-
 NS_IMETHODIMP
 nsHtml5Atom::QueryInterface(REFNSIID aIID, void** aInstancePtr)
 {
   NS_NOTREACHED("Attempt to call QueryInterface an nsHtml5Atom.");
   return NS_ERROR_UNEXPECTED;
 }
 
-NS_IMETHODIMP 
+NS_IMETHODIMP
 nsHtml5Atom::ScriptableToString(nsAString& aBuf)
 {
   NS_NOTREACHED("Should not call ScriptableToString.");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsHtml5Atom::ToUTF8String(nsACString& aReturn)
--- a/parser/html/nsHtml5Atom.h
+++ b/parser/html/nsHtml5Atom.h
@@ -13,16 +13,16 @@
  * nsHtml5TreeBuilder owned by one nsHtml5Parser or nsHtml5StreamParser 
  * instance.
  *
  * Usage is documented in nsHtml5AtomTable and nsIAtom.
  */
 class nsHtml5Atom final : public nsIAtom
 {
   public:
-    NS_DECL_ISUPPORTS
+    NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) final;
     NS_DECL_NSIATOM
 
     explicit nsHtml5Atom(const nsAString& aString);
     ~nsHtml5Atom();
 };
 
 #endif // nsHtml5Atom_h
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -123,18 +123,26 @@ private:
   }
 
 private:
   // We don't need a virtual destructor because we always delete via a
   // DynamicAtom* pointer (in GCAtomTable()), not an nsIAtom* pointer.
   ~DynamicAtom();
 
 public:
-  NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIATOM
+  NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) final;
+  typedef mozilla::TrueType HasThreadSafeRefCnt;
+
+  MozExternalRefCountType DoAddRef();
+  MozExternalRefCountType DoRelease();
+
+protected:
+  ThreadSafeAutoRefCnt mRefCnt;
+  NS_DECL_OWNINGTHREAD
 };
 
 #if defined(NS_BUILD_REFCNT_LOGGING)
 // nsFakeStringBuffers don't really use the refcounting system, but we
 // have to give a coherent series of addrefs and releases to the
 // refcount logging system, or we'll hit assertions when running with
 // XPCOM_MEM_LOG_CLASSES=nsStringBuffer.
 class FakeBufferRefcountHelper
@@ -194,33 +202,21 @@ public:
                aStringBuffer->StorageSize() == (mLength + 1) * sizeof(char16_t),
                "correct storage");
   }
 
   // We don't need a virtual destructor because we always delete via a
   // StaticAtom* pointer (in AtomTableClearEntry()), not an nsIAtom* pointer.
   ~StaticAtom() {}
 
-  NS_DECL_ISUPPORTS
+  NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) final;
   NS_DECL_NSIATOM
 };
 
-NS_IMPL_QUERY_INTERFACE(StaticAtom, nsIAtom)
-
-NS_IMETHODIMP_(MozExternalRefCountType)
-StaticAtom::AddRef()
-{
-  return 2;
-}
-
-NS_IMETHODIMP_(MozExternalRefCountType)
-StaticAtom::Release()
-{
-  return 1;
-}
+NS_IMPL_QUERY_INTERFACE(StaticAtom, nsIAtom);
 
 NS_IMETHODIMP
 DynamicAtom::ScriptableToString(nsAString& aBuf)
 {
   nsStringBuffer::FromData(mString)->ToString(mLength, aBuf);
   return NS_OK;
 }
 
@@ -274,16 +270,38 @@ StaticAtom::SizeOfIncludingThis(MallocSi
   size_t n = aMallocSizeOf(this);
   // Don't measure the string buffer pointed to by the StaticAtom because it's
   // in static memory.
   return n;
 }
 
 //----------------------------------------------------------------------
 
+MozExternalRefCountType
+nsIAtom::AddRef()
+{
+  MOZ_ASSERT(!IsHTML5Atom(), "Attempt to AddRef an nsHtml5Atom");
+  if (!IsDynamicAtom()) {
+    return 2;
+  }
+  return static_cast<DynamicAtom*>(this)->DoAddRef();
+}
+
+MozExternalRefCountType
+nsIAtom::Release()
+{
+  MOZ_ASSERT(!IsHTML5Atom(), "Attempt to Release an nsHtml5Atom");
+  if (!IsDynamicAtom()) {
+    return 1;
+  }
+  return static_cast<DynamicAtom*>(this)->DoRelease();
+}
+
+//----------------------------------------------------------------------
+
 /**
  * The shared hash table for atom lookups.
  *
  * Callers must hold gAtomTableLock before manipulating the table.
  */
 static PLDHashTable* gAtomTable;
 static Mutex* gAtomTableLock;
 
@@ -479,36 +497,36 @@ DynamicAtom::GCAtomTableLocked(const Mut
 
   MOZ_ASSERT_IF(aKind == GCKind::Shutdown, removedCount == gUnusedAtomCount);
 
   gUnusedAtomCount -= removedCount;
 }
 
 NS_IMPL_QUERY_INTERFACE(DynamicAtom, nsIAtom)
 
-NS_IMETHODIMP_(MozExternalRefCountType)
-DynamicAtom::AddRef(void)
+MozExternalRefCountType
+DynamicAtom::DoAddRef()
 {
   nsrefcnt count = ++mRefCnt;
   if (count == 1) {
     gUnusedAtomCount--;
   }
   return count;
 }
 
 #ifdef DEBUG
 // We set a lower GC threshold for atoms in debug builds so that we exercise
 // the GC machinery more often.
 static const uint32_t kAtomGCThreshold = 20;
 #else
 static const uint32_t kAtomGCThreshold = 10000;
 #endif
 
-NS_IMETHODIMP_(MozExternalRefCountType)
-DynamicAtom::Release(void)
+MozExternalRefCountType
+DynamicAtom::DoRelease()
 {
   MOZ_ASSERT(mRefCnt > 0);
   nsrefcnt count = --mRefCnt;
   if (count == 0) {
     if (++gUnusedAtomCount >= kAtomGCThreshold) {
       GCAtomTable();
     }
   }
--- a/xpcom/ds/nsIAtom.idl
+++ b/xpcom/ds/nsIAtom.idl
@@ -64,16 +64,20 @@ interface nsIAtom : nsISupports
   inline Kind Kind() const {
     return static_cast<enum Kind>(mKind);
   }
 
   inline bool IsDynamicAtom() const {
     return Kind() == Kind::DynamicAtom;
   }
 
+  inline bool IsHTML5Atom() const {
+    return Kind() == Kind::HTML5Atom;
+  }
+
   inline bool IsStaticAtom() const {
     return Kind() == Kind::StaticAtom;
   }
 
   inline char16ptr_t GetUTF16String() const {
     return mString;
   }
 
@@ -86,16 +90,19 @@ interface nsIAtom : nsISupports
     nsStringBuffer::FromData(mString)->ToString(mLength, aBuf);
   }
 
   inline nsStringBuffer* GetStringBuffer() const {
     // See the comment on |mString|'s declaration.
     return nsStringBuffer::FromData(mString);
   }
 
+  MozExternalRefCountType AddRef() final;
+  MozExternalRefCountType Release() final;
+
   /**
    * A hashcode that is better distributed than the actual atom
    * pointer, for use in situations that need a well-distributed
    * hashcode.
    */
   inline uint32_t hash() const {
     return mHash;
   }