diff --git a/dom/base/nsAttrValue.h b/dom/base/nsAttrValue.h index b5e80690b76f..d47e2cca56f4 100644 --- a/dom/base/nsAttrValue.h +++ b/dom/base/nsAttrValue.h @@ -543,7 +543,7 @@ nsAttrValue::ToString(mozilla::dom::DOMString& aResult) const case eAtom: { nsAtom *atom = static_cast(GetPtr()); - aResult.SetStringBuffer(atom->GetStringBuffer(), atom->GetLength()); + aResult.SetOwnedAtom(atom, mozilla::dom::DOMString::eNullNotExpected); break; } default: diff --git a/dom/base/nsINode.h b/dom/base/nsINode.h index ed1059ea62f3..54e06efe4504 100644 --- a/dom/base/nsINode.h +++ b/dom/base/nsINode.h @@ -1801,8 +1801,7 @@ public: 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 @@ public: 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(); diff --git a/dom/bindings/DOMString.h b/dom/bindings/DOMString.h index 2ef997b81b28..2747647dd884 100644 --- a/dom/bindings/DOMString.h +++ b/dom/bindings/DOMString.h @@ -179,7 +179,14 @@ public: 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(); } diff --git a/dom/xhr/XMLHttpRequestString.cpp b/dom/xhr/XMLHttpRequestString.cpp index b086d1f5c987..f0be2a821460 100644 --- a/dom/xhr/XMLHttpRequestString.cpp +++ b/dom/xhr/XMLHttpRequestString.cpp @@ -65,6 +65,9 @@ public: { 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 diff --git a/parser/htmlparser/nsHTMLTags.cpp b/parser/htmlparser/nsHTMLTags.cpp index 6596a37aa238..e0102a76682e 100644 --- a/parser/htmlparser/nsHTMLTags.cpp +++ b/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!"); } diff --git a/xpcom/ds/nsAtom.h b/xpcom/ds/nsAtom.h index b29af9e21c60..df5c54baefaf 100644 --- a/xpcom/ds/nsAtom.h +++ b/xpcom/ds/nsAtom.h @@ -52,17 +52,14 @@ public: 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 @@ private: // 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 @@ private: 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; }; diff --git a/xpcom/ds/nsAtomTable.cpp b/xpcom/ds/nsAtomTable.cpp index 96c35269067a..6b57885d8e4b 100644 --- a/xpcom/ds/nsAtomTable.cpp +++ b/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 @@ public: // See nsAtom::AddRef() and nsAtom::Release(). static Atomic 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> 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(AtomKind::StaticAtom)) , mHash(aHash) - , mString(static_cast(aStringBuffer->Data())) + , mString(const_cast(aString)) { -#if defined(NS_BUILD_REFCNT_LOGGING) - MOZ_ASSERT(NS_IsMainThread()); - if (!gFakeBuffers) { - gFakeBuffers = MakeUnique>(); - } - 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(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(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; diff --git a/xpcom/ds/nsStaticAtom.h b/xpcom/ds/nsStaticAtom.h index 3b9c329e44fa..c58fb58fe0f4 100644 --- a/xpcom/ds/nsStaticAtom.h +++ b/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 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 -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