Bug 1392185 - Remove dynamic HTML5 atoms. r=njn,hsivonen

This is a rebase + manual refcounting on some places, + cleanup of the original
patch in the bug.

Co-authored-by: Nicholas Nethercote <nnethercote@mozilla.com>

Differential Revision: https://phabricator.services.mozilla.com/D11035

Author: emilio

parser/html/nsHtml5AtomTable.cpp

@@ -5,24 +5,6 @@
 #include "nsHtml5AtomTable.h"
 #include "nsThreadUtils.h"
 
-nsHtml5AtomEntry::nsHtml5AtomEntry(KeyTypePointer aStr)
-  : nsStringHashKey(aStr)
-  , mAtom(nsDynamicAtom::Create(*aStr))
-{
-}
-
-nsHtml5AtomEntry::nsHtml5AtomEntry(nsHtml5AtomEntry&& aOther)
-  : nsStringHashKey(std::move(aOther))
-  , mAtom(nullptr)
-{
-  MOZ_ASSERT_UNREACHABLE("nsHtml5AtomTable is broken; tried to copy an entry");
-}
-
-nsHtml5AtomEntry::~nsHtml5AtomEntry()
-{
-  nsDynamicAtom::Destroy(mAtom);
-}
-
 nsHtml5AtomTable::nsHtml5AtomTable()
   : mRecentlyUsedParserAtoms{}
 {
@@ -37,27 +19,18 @@ nsAtom*
 nsHtml5AtomTable::GetAtom(const nsAString& aKey)
 {
 #ifdef DEBUG
-  {
-    MOZ_ASSERT(mPermittedLookupEventTarget->IsOnCurrentThread());
-  }
+  MOZ_ASSERT(mPermittedLookupEventTarget->IsOnCurrentThread());
 #endif
 
   uint32_t index = mozilla::HashString(aKey) % RECENTLY_USED_PARSER_ATOMS_SIZE;
-  nsAtom* cachedAtom = mRecentlyUsedParserAtoms[index];
-  if (cachedAtom && cachedAtom->Equals(aKey)) {
-    return cachedAtom;
-  }
-
-  nsStaticAtom* atom = NS_GetStaticAtom(aKey);
-  if (atom) {
-    mRecentlyUsedParserAtoms[index] = atom;
-    return atom;
-  }
-  nsHtml5AtomEntry* entry = mTable.PutEntry(aKey);
-  if (!entry) {
-    return nullptr;
+  if (nsAtom* atom = mRecentlyUsedParserAtoms[index]) {
+    if (atom->Equals(aKey)) {
+      return atom;
+    }
   }
 
-  mRecentlyUsedParserAtoms[index] = entry->GetAtom();
-  return entry->GetAtom();
+  RefPtr<nsAtom> atom = NS_Atomize(aKey);
+  nsAtom* ret = atom.get();
+  mRecentlyUsedParserAtoms[index] = atom.forget();
+  return ret;
 }

parser/html/nsHtml5AtomTable.h

@@ -12,18 +12,6 @@
 
 #define RECENTLY_USED_PARSER_ATOMS_SIZE 31
 
-class nsHtml5AtomEntry : public nsStringHashKey
-{
-public:
-  explicit nsHtml5AtomEntry(KeyTypePointer aStr);
-  nsHtml5AtomEntry(nsHtml5AtomEntry&& aOther);
-  ~nsHtml5AtomEntry();
-  inline nsAtom* GetAtom() { return mAtom; }
-
-private:
-  nsDynamicAtom* mAtom;
-};
-
 /**
  * nsHtml5AtomTable provides non-locking lookup and creation of atoms for
  * nsHtml5Parser or nsHtml5StreamParser.
@@ -74,9 +62,9 @@ class nsHtml5AtomTable
   nsHtml5AtomTable();
   ~nsHtml5AtomTable();
 
-  /**
-   * Obtains the atom for the given string in the scope of this atom table.
-   */
+  // NOTE: We rely on mRecentlyUsedParserAtoms keeping alive the returned atom,
+  // but the caller is responsible to take a reference before calling GetAtom
+  // again.
   nsAtom* GetAtom(const nsAString& aKey);
 
   /**
@@ -87,7 +75,6 @@ class nsHtml5AtomTable
     for (uint32_t i = 0; i < RECENTLY_USED_PARSER_ATOMS_SIZE; ++i) {
       mRecentlyUsedParserAtoms[i] = nullptr;
     }
-    mTable.Clear();
   }
 
 #ifdef DEBUG
@@ -98,8 +85,7 @@ class nsHtml5AtomTable
 #endif
 
 private:
-  nsTHashtable<nsHtml5AtomEntry> mTable;
-  nsAtom* mRecentlyUsedParserAtoms[RECENTLY_USED_PARSER_ATOMS_SIZE];
+  RefPtr<nsAtom> mRecentlyUsedParserAtoms[RECENTLY_USED_PARSER_ATOMS_SIZE];
 #ifdef DEBUG
   nsCOMPtr<nsISerialEventTarget> mPermittedLookupEventTarget;
 #endif

parser/html/nsHtml5AttributeEntry.h

@@ -71,7 +71,7 @@ class nsHtml5AttributeEntry final
       if (!local->IsStatic()) {
         nsAutoString str;
         local->ToString(str);
-        local = aInterner->GetAtom(str);
+        nsAtom* local = aInterner->GetAtom(str);
         clone.mLocals[0] = local;
         clone.mLocals[1] = local;
         clone.mLocals[2] = local;
@@ -81,8 +81,8 @@ class nsHtml5AttributeEntry final
   }
 
 private:
-  nsAtom* mLocals[3];
-  nsAtom* mPrefixes[3];
+  RefPtr<nsAtom> mLocals[3];
+  RefPtr<nsAtom> mPrefixes[3];
   int32_t mUris[3];
   int32_t mLine;
   nsHtml5String mValue;

parser/html/nsHtml5AttributeName.cpp

@@ -58,53 +58,53 @@ int32_t* nsHtml5AttributeName::ALL_NO_NS = 0;
 int32_t* nsHtml5AttributeName::XMLNS_NS = 0;
 int32_t* nsHtml5AttributeName::XML_NS = 0;
 int32_t* nsHtml5AttributeName::XLINK_NS = 0;
-nsAtom** nsHtml5AttributeName::ALL_NO_PREFIX = 0;
-nsAtom** nsHtml5AttributeName::XMLNS_PREFIX = 0;
-nsAtom** nsHtml5AttributeName::XLINK_PREFIX = 0;
-nsAtom** nsHtml5AttributeName::XML_PREFIX = 0;
-nsAtom**
+nsStaticAtom** nsHtml5AttributeName::ALL_NO_PREFIX = 0;
+nsStaticAtom** nsHtml5AttributeName::XMLNS_PREFIX = 0;
+nsStaticAtom** nsHtml5AttributeName::XLINK_PREFIX = 0;
+nsStaticAtom** nsHtml5AttributeName::XML_PREFIX = 0;
+RefPtr<nsAtom>*
 nsHtml5AttributeName::SVG_DIFFERENT(nsAtom* name, nsAtom* camel)
 {
-  nsAtom** arr = new nsAtom*[4];
+  RefPtr<nsAtom>* arr = new RefPtr<nsAtom>[4];
   arr[0] = name;
   arr[1] = name;
   arr[2] = camel;
   return arr;
 }
 
-nsAtom**
+RefPtr<nsAtom>*
 nsHtml5AttributeName::MATH_DIFFERENT(nsAtom* name, nsAtom* camel)
 {
-  nsAtom** arr = new nsAtom*[4];
+  RefPtr<nsAtom>* arr = new RefPtr<nsAtom>[4];
   arr[0] = name;
   arr[1] = camel;
   arr[2] = name;
   return arr;
 }
 
-nsAtom**
+RefPtr<nsAtom>*
 nsHtml5AttributeName::COLONIFIED_LOCAL(nsAtom* name, nsAtom* suffix)
 {
-  nsAtom** arr = new nsAtom*[4];
+  RefPtr<nsAtom>* arr = new RefPtr<nsAtom>[4];
   arr[0] = name;
   arr[1] = suffix;
   arr[2] = suffix;
   return arr;
 }
 
-nsAtom**
+RefPtr<nsAtom>*
 nsHtml5AttributeName::SAME_LOCAL(nsAtom* name)
 {
-  nsAtom** arr = new nsAtom*[4];
+  RefPtr<nsAtom>* arr = new RefPtr<nsAtom>[4];
   arr[0] = name;
   arr[1] = name;
   arr[2] = name;
   return arr;
 }
 
 nsHtml5AttributeName::nsHtml5AttributeName(int32_t* uri,
-                                           nsAtom** local,
-                                           nsAtom** prefix)
+                                           RefPtr<nsAtom>* local,
+                                           nsStaticAtom** prefix)
   : uri(uri)
   , local(local)
   , prefix(prefix)
@@ -148,7 +148,7 @@ nsHtml5AttributeName::getLocal(int32_t mode)
   return local[mode];
 }
 
-nsAtom*
+nsStaticAtom*
 nsHtml5AttributeName::getPrefix(int32_t mode)
 {
   return prefix[mode];
@@ -767,19 +767,19 @@ nsHtml5AttributeName::initializeStatics()
   XLINK_NS[0] = kNameSpaceID_None;
   XLINK_NS[1] = kNameSpaceID_XLink;
   XLINK_NS[2] = kNameSpaceID_XLink;
-  ALL_NO_PREFIX = new nsAtom*[3];
+  ALL_NO_PREFIX = new nsStaticAtom*[3];
   ALL_NO_PREFIX[0] = nullptr;
   ALL_NO_PREFIX[1] = nullptr;
   ALL_NO_PREFIX[2] = nullptr;
-  XMLNS_PREFIX = new nsAtom*[3];
+  XMLNS_PREFIX = new nsStaticAtom*[3];
   XMLNS_PREFIX[0] = nullptr;
   XMLNS_PREFIX[1] = nsGkAtoms::xmlns;
   XMLNS_PREFIX[2] = nsGkAtoms::xmlns;
-  XLINK_PREFIX = new nsAtom*[3];
+  XLINK_PREFIX = new nsStaticAtom*[3];
   XLINK_PREFIX[0] = nullptr;
   XLINK_PREFIX[1] = nsGkAtoms::xlink;
   XLINK_PREFIX[2] = nsGkAtoms::xlink;
-  XML_PREFIX = new nsAtom*[3];
+  XML_PREFIX = new nsStaticAtom*[3];
   XML_PREFIX[0] = nullptr;
   XML_PREFIX[1] = nsGkAtoms::xml;
   XML_PREFIX[2] = nsGkAtoms::xml;

parser/html/nsHtml5AttributeName.h

@@ -63,16 +63,18 @@ class nsHtml5AttributeName
   static int32_t* XML_NS;
   static int32_t* XLINK_NS;
 public:
-  static nsAtom** ALL_NO_PREFIX;
+  static nsStaticAtom** ALL_NO_PREFIX;
+
 private:
-  static nsAtom** XMLNS_PREFIX;
-  static nsAtom** XLINK_PREFIX;
-  static nsAtom** XML_PREFIX;
-  static nsAtom** SVG_DIFFERENT(nsAtom* name, nsAtom* camel);
-  static nsAtom** MATH_DIFFERENT(nsAtom* name, nsAtom* camel);
-  static nsAtom** COLONIFIED_LOCAL(nsAtom* name, nsAtom* suffix);
+  static nsStaticAtom** XMLNS_PREFIX;
+  static nsStaticAtom** XLINK_PREFIX;
+  static nsStaticAtom** XML_PREFIX;
+  static RefPtr<nsAtom>* SVG_DIFFERENT(nsAtom* name, nsAtom* camel);
+  static RefPtr<nsAtom>* MATH_DIFFERENT(nsAtom* name, nsAtom* camel);
+  static RefPtr<nsAtom>* COLONIFIED_LOCAL(nsAtom* name, nsAtom* suffix);
+
 public:
-  static nsAtom** SAME_LOCAL(nsAtom* name);
+  static RefPtr<nsAtom>* SAME_LOCAL(nsAtom* name);
   inline static int32_t levelOrderBinarySearch(jArray<int32_t, int32_t> data,
                                                int32_t key)
   {
@@ -154,10 +156,13 @@ class nsHtml5AttributeName
 
 private:
   int32_t* uri;
-  nsAtom** local;
-  nsAtom** prefix;
+  RefPtr<nsAtom>* local;
+  nsStaticAtom** prefix;
   bool custom;
-  nsHtml5AttributeName(int32_t* uri, nsAtom** local, nsAtom** prefix);
+  nsHtml5AttributeName(int32_t* uri,
+                       RefPtr<nsAtom>* local,
+                       nsStaticAtom** prefix);
+
 public:
   nsHtml5AttributeName();
   inline bool isInterned() { return !custom; }
@@ -174,7 +179,7 @@ class nsHtml5AttributeName
   ~nsHtml5AttributeName();
   int32_t getUri(int32_t mode);
   nsAtom* getLocal(int32_t mode);
-  nsAtom* getPrefix(int32_t mode);
+  nsStaticAtom* getPrefix(int32_t mode);
   bool equalsAnother(nsHtml5AttributeName* another);
   static nsHtml5AttributeName* ATTR_ALT;
   static nsHtml5AttributeName* ATTR_DIR;

parser/html/nsHtml5ElementName.h

@@ -76,8 +76,8 @@ class nsHtml5ElementName
   static const int32_t OPTIONAL_END_TAG = (1 << 23);
 
 private:
-  nsAtom* name;
-  nsAtom* camelCaseName;
+  RefPtr<nsAtom> name;
+  RefPtr<nsAtom> camelCaseName;
   mozilla::dom::HTMLContentCreatorFunction htmlCreator;
   mozilla::dom::SVGContentCreatorFunction svgCreator;
 public:

parser/html/nsHtml5Portability.cpp

@@ -17,6 +17,14 @@ nsHtml5Portability::newLocalNameFromBuffer(char16_t* buf,
   return interner->GetAtom(nsDependentSubstring(buf, buf + length));
 }
 
+nsAtom*
+nsHtml5Portability::newLocalFromLocal(nsAtom* local,
+                                      nsHtml5AtomTable* interner)
+{
+  // FIXME(emilio): This function should be removed.
+  return local;
+}
+
 static bool
 ContainsWhiteSpace(mozilla::Span<char16_t> aSpan)
 {
@@ -86,19 +94,6 @@ nsHtml5Portability::newCharArrayFromString(nsHtml5String string)
   return arr;
 }
 
-nsAtom*
-nsHtml5Portability::newLocalFromLocal(nsAtom* local, nsHtml5AtomTable* interner)
-{
-  MOZ_ASSERT(local, "Atom was null.");
-  MOZ_ASSERT(interner, "Atom table was null");
-  if (!local->IsStatic()) {
-    nsAutoString str;
-    local->ToString(str);
-    local = interner->GetAtom(str);
-  }
-  return local;
-}
-
 bool
 nsHtml5Portability::localEqualsBuffer(nsAtom* local,
                                       char16_t* buf,

parser/html/nsHtml5StackNode.h

@@ -61,8 +61,8 @@ class nsHtml5StackNode
 public:
   int32_t idxInTreeBuilder;
   int32_t flags;
-  nsAtom* name;
-  nsAtom* popName;
+  RefPtr<nsAtom> name;
+  RefPtr<nsAtom> popName;
   int32_t ns;
   nsIContentHandle* node;
   nsHtml5HtmlAttributes* attributes;

parser/html/nsHtml5Tokenizer.h

@@ -282,7 +282,7 @@ class nsHtml5Tokenizer
   nsHtml5AttributeName* attributeName;
 private:
   nsHtml5AttributeName* nonInternedAttributeName;
-  nsAtom* doctypeName;
+  RefPtr<nsAtom> doctypeName;
   nsHtml5String publicIdentifier;
   nsHtml5String systemIdentifier;
   nsHtml5HtmlAttributes* attributes;

parser/html/nsHtml5TreeBuilder.h

@@ -293,7 +293,7 @@ class nsHtml5TreeBuilder : public nsAHtml5TreeBuilderState
   bool scriptingEnabled;
   bool needToDropLF;
   bool fragment;
-  nsAtom* contextName;
+  RefPtr<nsAtom> contextName;
   int32_t contextNamespace;
   nsIContentHandle* contextNode;
   autoJArray<int32_t, int32_t> templateModeStack;