From 400567dcde5475ffda16fd95e0c5c89433f87a2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 7 Jan 2020 21:37:12 +0000 Subject: [PATCH] Bug 1606958 - Use FakeString for UTF8String. r=bzbarsky,froydnj Differential Revision: https://phabricator.services.mozilla.com/D58683 --HG-- extra : moz-landing-system : lando --- dom/bindings/BindingDeclarations.h | 41 +++-------- dom/bindings/BindingUtils.cpp | 2 +- dom/bindings/BindingUtils.h | 18 +++-- dom/bindings/Codegen.py | 17 ++--- dom/bindings/FakeString.h | 109 ++++++++++++++++++----------- xpcom/string/nsStringFlags.h | 9 ++- xpcom/string/nsTSubstring.cpp | 1 + 7 files changed, 109 insertions(+), 88 deletions(-) diff --git a/dom/bindings/BindingDeclarations.h b/dom/bindings/BindingDeclarations.h index e6f78727e874..988af16d260f 100644 --- a/dom/bindings/BindingDeclarations.h +++ b/dom/bindings/BindingDeclarations.h @@ -280,29 +280,32 @@ class Optional > : public Optional_base > { // ToAStringPtr. namespace binding_detail { +template struct FakeString; } // namespace binding_detail -template <> -class Optional { +template +class Optional> { + using AString = nsTSubstring; + public: Optional() : mStr(nullptr) {} bool WasPassed() const { return !!mStr; } - void operator=(const nsAString* str) { + void operator=(const AString* str) { MOZ_ASSERT(str); mStr = str; } // If this code ever goes away, remove the comment pointing to it in the // FakeString class in BindingUtils.h. - void operator=(const binding_detail::FakeString* str) { + void operator=(const binding_detail::FakeString* str) { MOZ_ASSERT(str); - mStr = reinterpret_cast(str); + mStr = reinterpret_cast*>(str); } - const nsAString& Value() const { + const AString& Value() const { MOZ_ASSERT(WasPassed()); return *mStr; } @@ -312,31 +315,7 @@ class Optional { Optional(const Optional& other) = delete; const Optional& operator=(const Optional& other) = delete; - const nsAString* mStr; -}; - -template <> -class Optional { - public: - Optional() : mStr(nullptr) {} - - bool WasPassed() const { return !!mStr; } - - void operator=(const nsACString* str) { - MOZ_ASSERT(str); - mStr = str; - } - const nsACString& Value() const { - MOZ_ASSERT(WasPassed()); - return *mStr; - } - - private: - // Forbid copy-construction and assignment - Optional(const Optional& other) = delete; - const Optional& operator=(const Optional& other) = delete; - - const nsACString* mStr; + const AString* mStr; }; template diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp index 2dadb616987f..c60440bdff90 100644 --- a/dom/bindings/BindingUtils.cpp +++ b/dom/bindings/BindingUtils.cpp @@ -2541,7 +2541,7 @@ bool NormalizeUSVString(nsAString& aString) { return EnsureUTF16Validity(aString); } -bool NormalizeUSVString(binding_detail::FakeString& aString) { +bool NormalizeUSVString(binding_detail::FakeString& aString) { uint32_t upTo = Utf16ValidUpTo(aString); uint32_t len = aString.Length(); if (upTo == len) { diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h index a4c19bb6bfe3..b609fdffaa33 100644 --- a/dom/bindings/BindingUtils.h +++ b/dom/bindings/BindingUtils.h @@ -1853,7 +1853,8 @@ static inline bool ConvertJSValueToString(JSContext* cx, MOZ_MUST_USE bool NormalizeUSVString(nsAString& aString); -MOZ_MUST_USE bool NormalizeUSVString(binding_detail::FakeString& aString); +MOZ_MUST_USE bool NormalizeUSVString( + binding_detail::FakeString& aString); template static inline bool ConvertJSValueToUSVString(JSContext* cx, @@ -2389,29 +2390,34 @@ const T& NonNullHelper(const OwningNonNull& aArg) { return aArg; } -inline void NonNullHelper(NonNull& aArg) { +template +inline void NonNullHelper(NonNull>& aArg) { // This overload is here to make sure that we never end up applying // NonNullHelper to a NonNull. If we // try to, it should fail to compile, since presumably the caller will try to // use our nonexistent return value. } -inline void NonNullHelper(const NonNull& aArg) { +template +inline void NonNullHelper( + const NonNull>& aArg) { // This overload is here to make sure that we never end up applying // NonNullHelper to a NonNull. If we // try to, it should fail to compile, since presumably the caller will try to // use our nonexistent return value. } -inline void NonNullHelper(binding_detail::FakeString& aArg) { +template +inline void NonNullHelper(binding_detail::FakeString& aArg) { // This overload is here to make sure that we never end up applying // NonNullHelper to a FakeString before we've constified it. If we // try to, it should fail to compile, since presumably the caller will try to // use our nonexistent return value. } -MOZ_ALWAYS_INLINE -const nsAString& NonNullHelper(const binding_detail::FakeString& aArg) { +template +MOZ_ALWAYS_INLINE const nsTSubstring& NonNullHelper( + const binding_detail::FakeString& aArg) { return aArg; } diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index f0ddadebd86e..d5c7981fe747 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -6013,20 +6013,18 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None, if isOptional: if type.isUTF8String(): declType = "Optional" - holderType = CGGeneric("nsAutoCString") + holderType = CGGeneric("binding_detail::FakeString") else: declType = "Optional" - holderType = CGGeneric("binding_detail::FakeString") + holderType = CGGeneric("binding_detail::FakeString") conversionCode = ("%s" "${declName} = &${holderName};\n" % getConversionCode("${holderName}")) else: if type.isUTF8String(): - # TODO(emilio, bug 1606958): We could make FakeString generic - # if we deem it worth it, instead of using nsAutoCString. - declType = "nsAutoCString" + declType = "binding_detail::FakeString" else: - declType = "binding_detail::FakeString" + declType = "binding_detail::FakeString" holderType = None conversionCode = getConversionCode("${declName}") @@ -10344,7 +10342,10 @@ def getUnionAccessorSignatureType(type, descriptorProvider): if type.isDOMString() or type.isUSVString(): return CGGeneric("const nsAString&") - if type.isByteString() or type.isUTF8String(): + if type.isUTF8String(): + return CGGeneric("const nsACString&") + + if type.isByteString(): return CGGeneric("const nsCString&") if type.isEnum(): @@ -11761,7 +11762,7 @@ class CGProxyNamedOperation(CGProxySpecialOperation): decls = "" idName = "id" - decls += "FakeString %s;\n" % argName + decls += "FakeString %s;\n" % argName main = fill( """ diff --git a/dom/bindings/FakeString.h b/dom/bindings/FakeString.h index 520c73a32626..0e1f82882ece 100644 --- a/dom/bindings/FakeString.h +++ b/dom/bindings/FakeString.h @@ -22,15 +22,25 @@ namespace binding_detail { // point to a literal (static-lifetime) string that's compiled into the binary, // or point at the buffer of an nsAString whose lifetime is longer than that of // the FakeString. +template struct FakeString { - using char_type = nsString::char_type; + using char_type = CharT; + using string_type = nsTString; + using size_type = typename string_type::size_type; + using DataFlags = typename string_type::DataFlags; + using ClassFlags = typename string_type::ClassFlags; + using AString = nsTSubstring; + + static const size_t kInlineCapacity = 64; + using AutoString = nsTAutoStringN; FakeString() - : mDataFlags(nsString::DataFlags::TERMINATED), - mClassFlags(nsString::ClassFlags(0)) {} + : mDataFlags(DataFlags::TERMINATED), + mClassFlags(ClassFlags::INLINE), + mInlineCapacity(kInlineCapacity - 1) {} ~FakeString() { - if (mDataFlags & nsString::DataFlags::REFCOUNTED) { + if (mDataFlags & DataFlags::REFCOUNTED) { MOZ_ASSERT(mDataInitialized); nsStringBuffer::FromData(mData)->Release(); } @@ -39,24 +49,24 @@ struct FakeString { // Share aString's string buffer, if it has one; otherwise, make this string // depend upon aString's data. aString should outlive this instance of // FakeString. - void ShareOrDependUpon(const nsAString& aString) { + void ShareOrDependUpon(const AString& aString) { RefPtr sharedBuffer = nsStringBuffer::FromString(aString); if (!sharedBuffer) { InitData(aString.BeginReading(), aString.Length()); if (!aString.IsTerminated()) { - mDataFlags &= ~nsString::DataFlags::TERMINATED; + mDataFlags &= ~DataFlags::TERMINATED; } } else { AssignFromStringBuffer(sharedBuffer.forget(), aString.Length()); } } - void Truncate() { InitData(nsString::char_traits::sEmptyBuffer, 0); } + void Truncate() { InitData(string_type::char_traits::sEmptyBuffer, 0); } void SetIsVoid(bool aValue) { MOZ_ASSERT(aValue, "We don't support SetIsVoid(false) on FakeString!"); Truncate(); - mDataFlags |= nsString::DataFlags::VOIDED; + mDataFlags |= DataFlags::VOIDED; } char_type* BeginWriting() { @@ -65,7 +75,7 @@ struct FakeString { return mData; } - nsString::size_type Length() const { return mLength; } + size_type Length() const { return mLength; } operator mozilla::Span() const { MOZ_ASSERT(mDataInitialized); @@ -76,12 +86,23 @@ struct FakeString { return mozilla::MakeSpan(BeginWriting(), Length()); } + mozilla::BulkWriteHandle BulkWrite(size_type aCapacity, + size_type aPrefixToPreserve, + bool aAllowShrinking, + nsresult& aRv) { + MOZ_ASSERT(!mDataInitialized); + InitData(mStorage, 0); + mDataFlags |= DataFlags::INLINE; + return ToAStringPtr()->BulkWrite(aCapacity, aPrefixToPreserve, + aAllowShrinking, aRv); + } + // Reserve space to write aLength chars, not including null-terminator. - bool SetLength(nsString::size_type aLength, mozilla::fallible_t const&) { - // Use mInlineStorage for small strings. - if (aLength < sInlineCapacity) { - InitData(mInlineStorage, aLength); - mDataFlags |= nsString::DataFlags::INLINE; + bool SetLength(size_type aLength, mozilla::fallible_t const&) { + // Use mStorage for small strings. + if (aLength < kInlineCapacity) { + InitData(mStorage, aLength); + mDataFlags |= DataFlags::INLINE; } else { RefPtr buf = nsStringBuffer::Alloc((aLength + 1) * sizeof(char_type)); @@ -93,7 +114,7 @@ struct FakeString { } MOZ_ASSERT(mDataInitialized); - mData[mLength] = char16_t(0); + mData[mLength] = char_type(0); return true; } @@ -106,14 +127,14 @@ struct FakeString { } RefPtr buffer; - if (mDataFlags & nsString::DataFlags::REFCOUNTED) { + if (mDataFlags & DataFlags::REFCOUNTED) { // Make sure we'll drop it when we're done. buffer = dont_AddRef(nsStringBuffer::FromData(mData)); // And make sure we don't release it twice by accident. } const char_type* oldChars = mData; - mDataFlags = nsString::DataFlags::TERMINATED; + mDataFlags = DataFlags::TERMINATED; #ifdef DEBUG // Reset mDataInitialized because we're explicitly reinitializing // it via the SetLength call. @@ -135,7 +156,7 @@ struct FakeString { void AssignFromStringBuffer(already_AddRefed aBuffer, size_t aLength) { InitData(static_cast(aBuffer.take()->Data()), aLength); - mDataFlags |= nsString::DataFlags::REFCOUNTED; + mDataFlags |= DataFlags::REFCOUNTED; } // The preferred way to assign literals to a FakeString. This should only be @@ -151,31 +172,29 @@ struct FakeString { // from an nsAString that tested true for IsLiteral()). void AssignLiteral(const char_type* aData, size_t aLength) { InitData(aData, aLength); - mDataFlags |= nsString::DataFlags::LITERAL; + mDataFlags |= DataFlags::LITERAL; } // If this ever changes, change the corresponding code in the - // Optional specialization as well. - const nsAString* ToAStringPtr() const { - return reinterpret_cast(this); + // Optional specialization as well. + const AString* ToAStringPtr() const { + return reinterpret_cast(this); } - operator const nsAString&() const { - return *reinterpret_cast(this); - } + operator const AString&() const { return *ToAStringPtr(); } private: - nsAString* ToAStringPtr() { return reinterpret_cast(this); } + AString* ToAStringPtr() { return reinterpret_cast(this); } // mData is left uninitialized for optimization purposes. MOZ_INIT_OUTSIDE_CTOR char_type* mData; // mLength is left uninitialized for optimization purposes. - MOZ_INIT_OUTSIDE_CTOR nsString::size_type mLength; - nsString::DataFlags mDataFlags; - nsString::ClassFlags mClassFlags; + MOZ_INIT_OUTSIDE_CTOR size_type mLength; + DataFlags mDataFlags; + const ClassFlags mClassFlags; - static const size_t sInlineCapacity = 64; - char_type mInlineStorage[sInlineCapacity]; + const size_type mInlineCapacity; + char_type mStorage[kInlineCapacity]; #ifdef DEBUG bool mDataInitialized = false; #endif // DEBUG @@ -183,8 +202,8 @@ struct FakeString { FakeString(const FakeString& other) = delete; void operator=(const FakeString& other) = delete; - void InitData(const char_type* aData, nsString::size_type aLength) { - MOZ_ASSERT(mDataFlags == nsString::DataFlags::TERMINATED); + void InitData(const char_type* aData, size_type aLength) { + MOZ_ASSERT(mDataFlags == DataFlags::TERMINATED); MOZ_ASSERT(!mDataInitialized); mData = const_cast(aData); mLength = aLength; @@ -194,23 +213,26 @@ struct FakeString { } bool IsMutable() { - return (mDataFlags & nsString::DataFlags::INLINE) || - ((mDataFlags & nsString::DataFlags::REFCOUNTED) && + return (mDataFlags & DataFlags::INLINE) || + ((mDataFlags & DataFlags::REFCOUNTED) && !nsStringBuffer::FromData(mData)->IsReadonly()); } - friend class NonNull; + friend class NonNull; // A class to use for our static asserts to ensure our object layout // matches that of nsString. class StringAsserter; friend class StringAsserter; - class StringAsserter : public nsString { + class StringAsserter : public AutoString { public: static void StaticAsserts() { - static_assert(offsetof(FakeString, mInlineStorage) == sizeof(nsString), - "FakeString should include all nsString members"); + static_assert(sizeof(AutoString) == sizeof(FakeString), + "Should be binary compatible with nsTAutoString"); + static_assert( + offsetof(FakeString, mInlineCapacity) == sizeof(string_type), + "FakeString should include all nsString members"); static_assert( offsetof(FakeString, mData) == offsetof(StringAsserter, mData), "Offset of mData should match"); @@ -223,6 +245,12 @@ struct FakeString { static_assert(offsetof(FakeString, mClassFlags) == offsetof(StringAsserter, mClassFlags), "Offset of mClassFlags should match"); + static_assert(offsetof(FakeString, mInlineCapacity) == + offsetof(StringAsserter, mInlineCapacity), + "Offset of mInlineCapacity should match"); + static_assert( + offsetof(FakeString, mStorage) == offsetof(StringAsserter, mStorage), + "Offset of mStorage should match"); } }; }; @@ -230,9 +258,10 @@ struct FakeString { } // namespace dom } // namespace mozilla +template inline void AssignFromStringBuffer( nsStringBuffer* aBuffer, size_t aLength, - mozilla::dom::binding_detail::FakeString& aDest) { + mozilla::dom::binding_detail::FakeString& aDest) { aDest.AssignFromStringBuffer(do_AddRef(aBuffer), aLength); } diff --git a/xpcom/string/nsStringFlags.h b/xpcom/string/nsStringFlags.h index 1011571e9fa1..00f103c284db 100644 --- a/xpcom/string/nsStringFlags.h +++ b/xpcom/string/nsStringFlags.h @@ -50,6 +50,8 @@ enum class StringDataFlags : uint16_t { // VOIDED implies TERMINATED, and moreover it implies that mData // points to char_traits::sEmptyBuffer. Therefore, VOIDED is // mutually exclusive with REFCOUNTED, OWNED, and INLINE. + // + // INLINE requires StringClassFlags::INLINE to be set on the type. // IsTerminated returns true TERMINATED = 1 << 0, @@ -72,8 +74,11 @@ enum class StringDataFlags : uint16_t { // bits for mClassFlags enum class StringClassFlags : uint16_t { - INLINE = 1 << 0, // |this|'s buffer is inline - NULL_TERMINATED = 1 << 1 // |this| requires its buffer is null-terminated + // |this|'s buffer is inline, and requires the type to be binary-compatible + // with nsTAutoStringN + INLINE = 1 << 0, + // |this| requires its buffer is null-terminated + NULL_TERMINATED = 1 << 1 }; MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(StringDataFlags) diff --git a/xpcom/string/nsTSubstring.cpp b/xpcom/string/nsTSubstring.cpp index 20d1381e8257..c6010c74ad0f 100644 --- a/xpcom/string/nsTSubstring.cpp +++ b/xpcom/string/nsTSubstring.cpp @@ -324,6 +324,7 @@ typename nsTSubstring::size_type nsTSubstring::Capacity() const { capacity = (hdr->StorageSize() / sizeof(char_type)) - 1; } } else if (this->mDataFlags & DataFlags::INLINE) { + MOZ_ASSERT(this->mClassFlags & ClassFlags::INLINE); capacity = AsAutoString(this)->mInlineCapacity; } else if (this->mDataFlags & DataFlags::OWNED) { // we don't store the capacity of an adopted buffer because that would