Bug 1407117 - Simplify static atom representation. r=froydnj,bz.

Currently nsAtom::mString points to the interior of an nsStringBuffer. For
static atoms this requires the use of nsFakeStringBuffer, which is pretty
gross.

This patch changes things so that nsAtom::mString points to a static char
buffer for static atoms. This simplifies a number of things:

- nsFakeStringBuffer and CheckStaticAtomSizes are no longer needed.

- FakeBufferRefCountHelper is no longer needed.

- nsAtom's constructor for static atoms is simpler.

- RegisterStaticAtoms() is simpler.

On the flip-side, a couple of things get more complicated.

- nsAtom::ToString() treats static and dynamic atoms differently.

- nsAtom::GetStringBuffer() is now only valid for dynamic atoms. This
  function is only used in two places, both involving DOMString, so those
  locations are updated appropriately. This also requires updating some other
  code assigning nsStrings to DOMStrings, because we can't assume that
  nsStrings are shared.

On Linux64 this change reduces the size of the binary by 8752 B, and moves
81968 B from the .data to the .rodata section, where it can be shared between
processes.

--HG--
extra : rebase_source : 0f6fcdec1c525aa66222e208b66a9f9026f69bcb

dom/base/nsAttrValue.h

@@ -543,7 +543,7 @@ nsAttrValue::ToString(mozilla::dom::DOMString& aResult) const
     case eAtom:
     {
       nsAtom *atom = static_cast<nsAtom*>(GetPtr());
-      aResult.SetStringBuffer(atom->GetStringBuffer(), atom->GetLength());
+      aResult.SetOwnedAtom(atom, mozilla::dom::DOMString::eNullNotExpected);
       break;
     }
     default:

dom/base/nsINode.h

@@ -1801,8 +1801,7 @@ class nsINode : public mozilla::dom::EventTarget
   void GetNodeName(mozilla::dom::DOMString& aNodeName)
   {
     const nsString& nodeName = NodeName();
-    aNodeName.SetStringBuffer(nsStringBuffer::FromString(nodeName),
-                              nodeName.Length());
+    aNodeName.SetOwnedString(nodeName);
   }
   MOZ_MUST_USE nsresult GetBaseURI(nsAString& aBaseURI) const;
   // Return the base URI for the document.
@@ -1866,12 +1865,7 @@ class nsINode : public mozilla::dom::EventTarget
   void GetLocalName(mozilla::dom::DOMString& aLocalName) const
   {
     const nsString& localName = LocalName();
-    if (localName.IsVoid()) {
-      aLocalName.SetNull();
-    } else {
-      aLocalName.SetStringBuffer(nsStringBuffer::FromString(localName),
-                                 localName.Length());
-    }
+    aLocalName.SetOwnedString(localName);
   }
 
   nsDOMAttributeMap* GetAttributes();

dom/bindings/DOMString.h

@@ -179,7 +179,14 @@ class MOZ_STACK_CLASS DOMString {
     MOZ_ASSERT(!mStringBuffer, "Setting stringbuffer twice?");
     MOZ_ASSERT(aAtom || aNullHandling != eNullNotExpected);
     if (aNullHandling == eNullNotExpected || aAtom) {
-      SetStringBuffer(aAtom->GetStringBuffer(), aAtom->GetLength());
+      if (aAtom->IsStaticAtom()) {
+        // XXX: bug 1407858 will replace this with a direct assignment of the
+        // static atom that doesn't go via nsString.
+        AsAString().AssignLiteral(aAtom->GetUTF16String(), aAtom->GetLength());
+      } else {
+        // Dynamic atoms always have a string buffer.
+        SetStringBuffer(aAtom->GetStringBuffer(), aAtom->GetLength());
+      }
     } else if (aNullHandling == eTreatNullAsNull) {
       SetNull();
     }

dom/xhr/XMLHttpRequestString.cpp

@@ -65,6 +65,9 @@ class XMLHttpRequestStringBuffer final
   {
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(aLength <= mData.Length());
+
+    // XXX: Bug 1408793 suggests encapsulating the following sequence within
+    //      DOMString.
     nsStringBuffer* buf = nsStringBuffer::FromString(mData);
     if (buf) {
       // We have to use SetEphemeralStringBuffer, because once we release our

parser/htmlparser/nsHTMLTags.cpp

@@ -83,8 +83,8 @@ nsHTMLTags::RegisterAtoms(void)
   {
     // let's verify that all names in the the table are lowercase...
     for (int32_t i = 0; i < NS_HTML_TAG_MAX; ++i) {
-      nsAutoString temp1((char16_t*)sTagAtoms_info[i].mStringBuffer->Data());
-      nsAutoString temp2((char16_t*)sTagAtoms_info[i].mStringBuffer->Data());
+      nsAutoString temp1((char16_t*)sTagAtoms_info[i].mString);
+      nsAutoString temp2((char16_t*)sTagAtoms_info[i].mString);
       ToLowerCase(temp1);
       NS_ASSERTION(temp1.Equals(temp2), "upper case char in table");
     }
@@ -93,7 +93,7 @@ nsHTMLTags::RegisterAtoms(void)
     // correct.
     for (int32_t i = 0; i < NS_HTML_TAG_MAX; ++i) {
       nsAutoString temp1(sTagUnicodeTable[i]);
-      nsAutoString temp2((char16_t*)sTagAtoms_info[i].mStringBuffer->Data());
+      nsAutoString temp2((char16_t*)sTagAtoms_info[i].mString);
       NS_ASSERTION(temp1.Equals(temp2), "Bad unicode tag name!");
     }
 

xpcom/ds/nsAtom.h

@@ -52,17 +52,14 @@ class nsAtom
 
   uint32_t GetLength() const { return mLength; }
 
-  void ToString(nsAString& aBuf) const
-  {
-    // See the comment on |mString|'s declaration.
-    nsStringBuffer::FromData(mString)->ToString(mLength, aBuf);
-  }
-
+  void ToString(nsAString& aString) const;
   void ToUTF8String(nsACString& aString) const;
 
+  // This is only valid for dynamic atoms.
   nsStringBuffer* GetStringBuffer() const
   {
     // See the comment on |mString|'s declaration.
+    MOZ_ASSERT(IsDynamicAtom());
     return nsStringBuffer::FromData(mString);
   }
 
@@ -90,7 +87,7 @@ class nsAtom
 
   // Construction and destruction is done entirely by |friend|s.
   nsAtom(AtomKind aKind, const nsAString& aString, uint32_t aHash);
-  nsAtom(nsStringBuffer* aStringBuffer, uint32_t aLength, uint32_t aHash);
+  nsAtom(const char16_t* aString, uint32_t aLength, uint32_t aHash);
 protected:
   ~nsAtom();
 
@@ -99,9 +96,10 @@ class nsAtom
   uint32_t mLength: 30;
   uint32_t mKind: 2; // nsAtom::AtomKind
   uint32_t mHash;
-  // WARNING! There is an invisible constraint on |mString|: the chars it
-  // points to must belong to an nsStringBuffer. This is so that the
-  // nsStringBuffer::FromData() calls above are valid.
+  // WARNING! For static atoms, this is a pointer to a static char buffer. For
+  // non-static atoms it points to the chars in an nsStringBuffer. This means
+  // that nsStringBuffer::FromData(mString) calls are only valid for non-static
+  // atoms.
   char16_t* mString;
 };
 

xpcom/ds/nsAtomTable.cpp

@@ -47,26 +47,6 @@ using namespace mozilla;
 
 //----------------------------------------------------------------------
 
-class CheckStaticAtomSizes
-{
-  CheckStaticAtomSizes()
-  {
-    static_assert((sizeof(nsFakeStringBuffer<1>().mRefCnt) ==
-                   sizeof(nsStringBuffer().mRefCount)) &&
-                  (sizeof(nsFakeStringBuffer<1>().mSize) ==
-                   sizeof(nsStringBuffer().mStorageSize)) &&
-                  (offsetof(nsFakeStringBuffer<1>, mRefCnt) ==
-                   offsetof(nsStringBuffer, mRefCount)) &&
-                  (offsetof(nsFakeStringBuffer<1>, mSize) ==
-                   offsetof(nsStringBuffer, mStorageSize)) &&
-                  (offsetof(nsFakeStringBuffer<1>, mStringData) ==
-                   sizeof(nsStringBuffer)),
-                  "mocked-up strings' representations should be compatible");
-  }
-};
-
-//----------------------------------------------------------------------
-
 enum class GCKind {
   RegularOperation,
   Shutdown,
@@ -102,39 +82,6 @@ class nsAtomFriend
 // See nsAtom::AddRef() and nsAtom::Release().
 static Atomic<int32_t, ReleaseAcquire> gUnusedAtomCount(0);
 
-#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
-{
-public:
-  explicit FakeBufferRefcountHelper(nsStringBuffer* aBuffer)
-    : mBuffer(aBuffer)
-  {
-    // Account for the initial static refcount of 1, so that we don't
-    // hit a refcount logging assertion when this object first appears
-    // with a refcount of 2.
-    NS_LOG_ADDREF(aBuffer, 1, "nsStringBuffer", sizeof(nsStringBuffer));
-  }
-
-  ~FakeBufferRefcountHelper()
-  {
-    // We told the refcount logging system in the ctor that this
-    // object was created, so now we have to tell it that it was
-    // destroyed, to avoid leak reports. This may cause odd the
-    // refcount isn't actually 0.
-    NS_LOG_RELEASE(mBuffer, 0, "nsStringBuffer");
-  }
-
-private:
-  nsStringBuffer* mBuffer;
-};
-
-UniquePtr<nsTArray<FakeBufferRefcountHelper>> gFakeBuffers;
-#endif
-
 // This constructor is for dynamic atoms and HTML5 atoms.
 nsAtom::nsAtom(AtomKind aKind, const nsAString& aString, uint32_t aHash)
   : mRefCnt(1)
@@ -171,30 +118,16 @@ nsAtom::nsAtom(AtomKind aKind, const nsAString& aString, uint32_t aHash)
 }
 
 // This constructor is for static atoms.
-nsAtom::nsAtom(nsStringBuffer* aStringBuffer, uint32_t aLength, uint32_t aHash)
+nsAtom::nsAtom(const char16_t* aString, uint32_t aLength, uint32_t aHash)
   : mLength(aLength)
   , mKind(static_cast<uint32_t>(AtomKind::StaticAtom))
   , mHash(aHash)
-  , mString(static_cast<char16_t*>(aStringBuffer->Data()))
+  , mString(const_cast<char16_t*>(aString))
 {
-#if defined(NS_BUILD_REFCNT_LOGGING)
-  MOZ_ASSERT(NS_IsMainThread());
-  if (!gFakeBuffers) {
-    gFakeBuffers = MakeUnique<nsTArray<FakeBufferRefcountHelper>>();
-  }
-  gFakeBuffers->AppendElement(aStringBuffer);
-#endif
-
-  // Technically we could currently avoid doing this addref by instead making
-  // the static atom buffers have an initial refcount of 2.
-  aStringBuffer->AddRef();
-
   MOZ_ASSERT(mHash == HashString(mString, mLength));
 
   MOZ_ASSERT(mString[mLength] == char16_t(0), "null terminated");
-  MOZ_ASSERT(aStringBuffer &&
-             aStringBuffer->StorageSize() == (mLength + 1) * sizeof(char16_t),
-             "correct storage");
+  MOZ_ASSERT(NS_strlen(mString) == mLength, "correct storage");
 }
 
 nsAtom::~nsAtom()
@@ -205,6 +138,20 @@ nsAtom::~nsAtom()
   }
 }
 
+void
+nsAtom::ToString(nsAString& aString) const
+{
+  // See the comment on |mString|'s declaration.
+  if (IsStaticAtom()) {
+    // AssignLiteral() lets us assign without copying. This isn't a string
+    // literal, but it's a static atom and thus has an unbounded lifetime,
+    // which is what's important.
+    aString.AssignLiteral(mString, mLength);
+  } else {
+    nsStringBuffer::FromData(mString)->ToString(mLength, aString);
+  }
+}
+
 void
 nsAtom::ToUTF8String(nsACString& aBuf) const
 {
@@ -560,10 +507,6 @@ NS_InitAtomTable()
 void
 NS_ShutdownAtomTable()
 {
-#if defined(NS_BUILD_REFCNT_LOGGING)
-  gFakeBuffers = nullptr;
-#endif
-
   delete gStaticAtomTable;
   gStaticAtomTable = nullptr;
 
@@ -632,17 +575,15 @@ nsAtomFriend::RegisterStaticAtoms(const nsStaticAtom* aAtoms,
   }
 
   for (uint32_t i = 0; i < aAtomCount; ++i) {
-    nsStringBuffer* stringBuffer = aAtoms[i].mStringBuffer;
+    const char16_t* string = aAtoms[i].mString;
     nsAtom** atomp = aAtoms[i].mAtom;
 
-    MOZ_ASSERT(nsCRT::IsAscii(static_cast<char16_t*>(stringBuffer->Data())));
+    MOZ_ASSERT(nsCRT::IsAscii(string));
 
-    uint32_t stringLen = stringBuffer->StorageSize() / sizeof(char16_t) - 1;
+    uint32_t stringLen = NS_strlen(string);
 
     uint32_t hash;
-    AtomTableEntry* he =
-      GetAtomHashEntry(static_cast<char16_t*>(stringBuffer->Data()),
-                       stringLen, &hash);
+    AtomTableEntry* he = GetAtomHashEntry(string, stringLen, &hash);
 
     nsAtom* atom = he->mAtom;
     if (atom) {
@@ -657,7 +598,7 @@ nsAtomFriend::RegisterStaticAtoms(const nsStaticAtom* aAtoms,
           "Static atom registration for %s should be pushed back", name.get());
       }
     } else {
-      atom = new nsAtom(stringBuffer, stringLen, hash);
+      atom = new nsAtom(string, stringLen, hash);
       he->mAtom = atom;
     }
     *atomp = atom;

xpcom/ds/nsStaticAtom.h

@@ -11,34 +11,24 @@
 #include "nsStringBuffer.h"
 
 #define NS_STATIC_ATOM(buffer_name, atom_ptr) \
-  { (nsStringBuffer*) &buffer_name, atom_ptr }
+  { buffer_name, atom_ptr }
 
+// Note that |str_data| is an 8-bit string, and so |sizeof(str_data)| is equal
+// to the number of chars (including the terminating '\0'). The |u""| prefix
+// converts |str_data| to a 16-bit string, which is assigned.
 #define NS_STATIC_ATOM_BUFFER(buffer_name, str_data) \
-  static nsFakeStringBuffer<sizeof(str_data)> buffer_name = \
-    { 1, sizeof(str_data) * sizeof(char16_t), (u"" str_data) };
+  static const char16_t buffer_name[sizeof(str_data)] = u"" str_data; \
+  static_assert(sizeof(str_data[0]) == 1, "non-8-bit static atom literal");
 
 /**
  * Holds data used to initialize large number of atoms during startup. Use
  * the above macros to initialize these structs. They should never be accessed
- * directly other than from AtomTable.cpp
+ * directly other than from AtomTable.cpp.
  */
 struct nsStaticAtom
 {
-  // mStringBuffer points to the string buffer for a permanent atom, and is
-  // therefore safe as a non-owning reference.
-  nsStringBuffer* MOZ_NON_OWNING_REF mStringBuffer;
-  nsAtom** mAtom;
-};
-
-/**
- * This is a struct with the same binary layout as a nsStringBuffer.
- */
-template<uint32_t size>
-struct nsFakeStringBuffer
-{
-  int32_t mRefCnt;
-  uint32_t mSize;
-  char16_t mStringData[size];
+  const char16_t* const mString;
+  nsAtom** const mAtom;
 };
 
 // Register an array of static atoms with the atom table