Bug 1032726 part 5 - Make new DOM bindings work with Latin1 strings. r=bz
--- a/content/html/content/src/nsGenericHTMLElement.cpp
+++ b/content/html/content/src/nsGenericHTMLElement.cpp
@@ -3095,17 +3095,17 @@ nsGenericHTMLElement::SetItemValue(JSCon
ErrorResult& aError)
{
if (!HasAttr(kNameSpaceID_None, nsGkAtoms::itemprop) ||
HasAttr(kNameSpaceID_None, nsGkAtoms::itemscope)) {
aError.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
return;
}
- nsDependentString string;
+ nsAutoString string;
JS::Rooted<JS::Value> value(aCx, aValue);
if (!ConvertJSValueToString(aCx, value, &value, eStringify, eStringify, string)) {
aError.Throw(NS_ERROR_UNEXPECTED);
return;
}
SetItemValueText(string);
}
--- a/dom/bindings/BindingDeclarations.h
+++ b/dom/bindings/BindingDeclarations.h
@@ -297,22 +297,22 @@ public:
// shadowing the one on the superclass.
OwningNonNull<T>& Value()
{
return this->mImpl.ref();
}
};
// Specialization for strings.
-// XXXbz we can't pull in FakeDependentString here, because it depends on
-// internal strings. So we just have to forward-declare it and reimplement its
+// XXXbz we can't pull in FakeString here, because it depends on internal
+// strings. So we just have to forward-declare it and reimplement its
// ToAStringPtr.
namespace binding_detail {
-struct FakeDependentString;
+struct FakeString;
} // namespace binding_detail
template<>
class Optional<nsAString>
{
public:
Optional() : mPassed(false) {}
@@ -324,21 +324,21 @@ public:
void operator=(const nsAString* str)
{
MOZ_ASSERT(str);
mStr = str;
mPassed = true;
}
// If this code ever goes away, remove the comment pointing to it in the
- // FakeDependentString class in BindingUtils.h.
- void operator=(const binding_detail::FakeDependentString* str)
+ // FakeString class in BindingUtils.h.
+ void operator=(const binding_detail::FakeString* str)
{
MOZ_ASSERT(str);
- mStr = reinterpret_cast<const nsDependentString*>(str);
+ mStr = reinterpret_cast<const nsString*>(str);
mPassed = true;
}
const nsAString& Value() const
{
MOZ_ASSERT(WasPassed());
return *mStr;
}
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -2166,19 +2166,19 @@ ConvertJSValueToByteString(JSContext* cx
PR_snprintf(badCharArray, sizeof(badCharArray), "%d", badChar);
ThrowErrorMessage(cx, MSG_INVALID_BYTESTRING, index, badCharArray);
return false;
}
} else {
length = JS_GetStringLength(s);
}
- if (length >= UINT32_MAX) {
- return false;
- }
+ static_assert(js::MaxStringLength < UINT32_MAX,
+ "length+1 shouldn't overflow");
+
result.SetCapacity(length+1);
JS_EncodeStringToBuffer(cx, s, result.BeginWriting(), length);
result.BeginWriting()[length] = '\0';
result.SetLength(length);
return true;
}
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1716,95 +1716,164 @@ HasPropertyOnPrototype(JSContext* cx, JS
// then the "proxy" argument is ignored.
bool
AppendNamedPropertyIds(JSContext* cx, JS::Handle<JSObject*> proxy,
nsTArray<nsString>& names,
bool shadowPrototypeProperties, JS::AutoIdVector& props);
namespace binding_detail {
-// A struct that has the same layout as an nsDependentString but much
-// faster constructor and destructor behavior
-struct FakeDependentString {
- FakeDependentString() :
- mFlags(nsDependentString::F_TERMINATED)
+// A struct that has the same layout as an nsString but much faster
+// constructor and destructor behavior. FakeString uses inline storage
+// for small strings and a nsStringBuffer for longer strings.
+struct FakeString {
+ FakeString() :
+ mFlags(nsString::F_TERMINATED)
{
}
- void Rebind(const nsDependentString::char_type* aData,
- nsDependentString::size_type aLength) {
- MOZ_ASSERT(mFlags == nsDependentString::F_TERMINATED);
- mData = aData;
+ ~FakeString() {
+ if (mFlags & nsString::F_SHARED) {
+ nsStringBuffer::FromData(mData)->Release();
+ }
+ }
+
+ void Rebind(const nsString::char_type* aData, nsString::size_type aLength) {
+ MOZ_ASSERT(mFlags == nsString::F_TERMINATED);
+ mData = const_cast<nsString::char_type*>(aData);
mLength = aLength;
}
void Truncate() {
- mData = nsDependentString::char_traits::sEmptyBuffer;
+ MOZ_ASSERT(mFlags == nsString::F_TERMINATED);
+ mData = nsString::char_traits::sEmptyBuffer;
mLength = 0;
}
void SetIsVoid(bool aValue) {
MOZ_ASSERT(aValue,
- "We don't support SetIsVoid(false) on FakeDependentString!");
+ "We don't support SetIsVoid(false) on FakeString!");
Truncate();
- mFlags |= nsDependentString::F_VOIDED;
+ mFlags |= nsString::F_VOIDED;
}
- const nsDependentString::char_type* Data() const
+ const nsString::char_type* Data() const
+ {
+ return mData;
+ }
+
+ nsString::char_type* BeginWriting()
{
return mData;
}
- nsDependentString::size_type Length() const
+ nsString::size_type Length() const
{
return mLength;
}
+ // Reserve space to write aCapacity chars, including null-terminator.
+ // Caller is responsible for calling SetLength and writing the null-
+ // terminator.
+ bool SetCapacity(nsString::size_type aCapacity, mozilla::fallible_t const&) {
+ MOZ_ASSERT(aCapacity > 0, "Capacity must include null-terminator");
+
+ // Use mInlineStorage for small strings.
+ if (aCapacity <= sInlineCapacity) {
+ SetData(mInlineStorage);
+ return true;
+ }
+
+ nsRefPtr<nsStringBuffer> buf = nsStringBuffer::Alloc(aCapacity * sizeof(nsString::char_type));
+ if (MOZ_UNLIKELY(!buf)) {
+ return false;
+ }
+
+ SetData(static_cast<nsString::char_type*>(buf.forget().take()->Data()));
+ mFlags = nsString::F_SHARED | nsString::F_TERMINATED;
+ return true;
+ }
+
+ void SetLength(nsString::size_type aLength) {
+ mLength = aLength;
+ }
+
// If this ever changes, change the corresponding code in the
// Optional<nsAString> specialization as well.
const nsAString* ToAStringPtr() const {
- return reinterpret_cast<const nsDependentString*>(this);
+ return reinterpret_cast<const nsString*>(this);
}
nsAString* ToAStringPtr() {
- return reinterpret_cast<nsDependentString*>(this);
+ return reinterpret_cast<nsString*>(this);
}
operator const nsAString& () const {
- return *reinterpret_cast<const nsDependentString*>(this);
+ return *reinterpret_cast<const nsString*>(this);
}
private:
- const nsDependentString::char_type* mData;
- nsDependentString::size_type mLength;
+ nsString::char_type* mData;
+ nsString::size_type mLength;
uint32_t mFlags;
+ static const size_t sInlineCapacity = 64;
+ nsString::char_type mInlineStorage[sInlineCapacity];
+
+ FakeString(const FakeString& other) MOZ_DELETE;
+ void operator=(const FakeString& other) MOZ_DELETE;
+
+ void SetData(nsString::char_type* aData) {
+ MOZ_ASSERT(mFlags == nsString::F_TERMINATED);
+ mData = const_cast<nsString::char_type*>(aData);
+ }
+
// A class to use for our static asserts to ensure our object layout
- // matches that of nsDependentString.
- class DependentStringAsserter;
- friend class DependentStringAsserter;
-
- class DepedentStringAsserter : public nsDependentString {
+ // matches that of nsString.
+ class StringAsserter;
+ friend class StringAsserter;
+
+ class StringAsserter : public nsString {
public:
static void StaticAsserts() {
- static_assert(sizeof(FakeDependentString) == sizeof(nsDependentString),
- "Must have right object size");
- static_assert(offsetof(FakeDependentString, mData) ==
- offsetof(DepedentStringAsserter, mData),
+ static_assert(offsetof(FakeString, mInlineStorage) ==
+ sizeof(nsString),
+ "FakeString should include all nsString members");
+ static_assert(offsetof(FakeString, mData) ==
+ offsetof(StringAsserter, mData),
"Offset of mData should match");
- static_assert(offsetof(FakeDependentString, mLength) ==
- offsetof(DepedentStringAsserter, mLength),
+ static_assert(offsetof(FakeString, mLength) ==
+ offsetof(StringAsserter, mLength),
"Offset of mLength should match");
- static_assert(offsetof(FakeDependentString, mFlags) ==
- offsetof(DepedentStringAsserter, mFlags),
+ static_assert(offsetof(FakeString, mFlags) ==
+ offsetof(StringAsserter, mFlags),
"Offset of mFlags should match");
}
};
};
+template<typename T>
+inline bool
+AssignJSString(JSContext *cx, T &dest, JSString *s)
+{
+ size_t len = js::GetStringLength(s);
+ static_assert(js::MaxStringLength < (1 << 28),
+ "Shouldn't overflow here or in FakeString::SetCapacity");
+ if (MOZ_UNLIKELY(!dest.SetCapacity(len + 1, mozilla::fallible_t()))) {
+ JS_ReportOutOfMemory(cx);
+ return false;
+ }
+ if (MOZ_UNLIKELY(!js::CopyStringChars(cx, dest.BeginWriting(), s, len))) {
+ return false;
+ }
+ dest.BeginWriting()[len] = '\0';
+ dest.SetLength(len);
+ return true;
+}
+
} // namespace binding_detail
enum StringificationBehavior {
eStringify,
eEmpty,
eNull
};
@@ -1841,24 +1910,17 @@ ConvertJSValueToString(JSContext* cx, JS
s = JS::ToString(cx, v);
if (!s) {
return false;
}
pval.set(JS::StringValue(s)); // Root the new string.
}
- size_t len;
- const jschar *chars = JS_GetStringCharsZAndLength(cx, s, &len);
- if (!chars) {
- return false;
- }
-
- result.Rebind(chars, len);
- return true;
+ return binding_detail::AssignJSString(cx, result, s);
}
bool
ConvertJSValueToByteString(JSContext* cx, JS::Handle<JS::Value> v,
JS::MutableHandle<JS::Value> pval, bool nullable,
nsACString& result);
template<typename T>
@@ -2401,44 +2463,44 @@ T& NonNullHelper(OwningNonNull<T>& aArg)
template<typename T>
const T& NonNullHelper(const OwningNonNull<T>& aArg)
{
return aArg;
}
inline
-void NonNullHelper(NonNull<binding_detail::FakeDependentString>& aArg)
+void NonNullHelper(NonNull<binding_detail::FakeString>& aArg)
{
// This overload is here to make sure that we never end up applying
- // NonNullHelper to a NonNull<binding_detail::FakeDependentString>. If we
+ // NonNullHelper to a NonNull<binding_detail::FakeString>. 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<binding_detail::FakeDependentString>& aArg)
+void NonNullHelper(const NonNull<binding_detail::FakeString>& aArg)
{
// This overload is here to make sure that we never end up applying
- // NonNullHelper to a NonNull<binding_detail::FakeDependentString>. If we
+ // NonNullHelper to a NonNull<binding_detail::FakeString>. 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::FakeDependentString& aArg)
+void NonNullHelper(binding_detail::FakeString& aArg)
{
// This overload is here to make sure that we never end up applying
- // NonNullHelper to a FakeDependentString before we've constified it. If we
+ // 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::FakeDependentString& aArg)
+const nsAString& NonNullHelper(const binding_detail::FakeString& aArg)
{
return aArg;
}
// Reparent the wrapper of aObj to whatever its native now thinks its
// parent should be.
nsresult
ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObj);
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3840,17 +3840,17 @@ def getJSToNativeConversionInfo(type, de
JS::Rooted<JS::Value> propNameValue(cx);
JS::Rooted<JS::Value> temp(cx);
JS::Rooted<jsid> curId(cx);
for (size_t i = 0; i < ids.length(); ++i) {
// Make sure we get the value before converting the name, since
// getting the value can trigger GC but our name is a dependent
// string.
curId = ids[i];
- binding_detail::FakeDependentString propName;
+ binding_detail::FakeString propName;
if (!JS_GetPropertyById(cx, mozMapObj, curId, &temp) ||
!JS_IdToValue(cx, curId, &propNameValue) ||
!ConvertJSValueToString(cx, propNameValue, &propNameValue,
eStringify, eStringify, propName)) {
$*{exceptionCode}
}
${valueType}* slotPtr = mozMap.AddEntry(propName);
@@ -4437,47 +4437,31 @@ def getJSToNativeConversionInfo(type, de
assert(type.nullable())
defaultCode = "%s.SetIsVoid(true)" % varName
else:
defaultCode = handleDefaultStringValue(defaultValue,
"%s.Rebind" % varName)
return handleDefault(conversionCode, defaultCode + ";\n")
if isMember:
- # We have to make a copy, except in the variadic case, because our
- # jsval may well not live as long as our string needs to.
+ # Convert directly into the nsString member we have.
declType = CGGeneric("nsString")
- if isMember == "Variadic":
- # The string is kept alive by the argument, so we can just
- # depend on it.
- assignString = "${declName}.Rebind(str.Data(), str.Length());\n"
- else:
- assignString = "${declName} = str;\n"
return JSToNativeConversionInfo(
- fill(
- """
- {
- binding_detail::FakeDependentString str;
- $*{convert}
- $*{assign}
- }
- """,
- convert=getConversionCode("str"),
- assign=assignString),
+ getConversionCode("${declName}"),
declType=declType,
dealWithOptional=isOptional)
if isOptional:
declType = "Optional<nsAString>"
- holderType = CGGeneric("binding_detail::FakeDependentString")
+ holderType = CGGeneric("binding_detail::FakeString")
conversionCode = ("%s"
"${declName} = &${holderName};\n" %
getConversionCode("${holderName}"))
else:
- declType = "binding_detail::FakeDependentString"
+ declType = "binding_detail::FakeString"
holderType = None
conversionCode = getConversionCode("${declName}")
# No need to deal with optional here; we handled it already
return JSToNativeConversionInfo(
conversionCode,
declType=CGGeneric(declType),
holderType=holderType)
@@ -9369,20 +9353,20 @@ class CGProxyNamedOperation(CGProxySpeci
}
""",
argName=argName)
if self.value is None:
# We're just using 'id', and if it's an atom we can take a
# fast path here.
unwrapString = fill(
"""
- if (MOZ_LIKELY(JSID_IS_ATOM(${idName}))) {
- JS::AutoCheckCannotGC nogc;
- JSAtom* atom = JSID_TO_ATOM(${idName});
- ${argName}.Rebind(js::GetTwoByteAtomChars(nogc, atom), js::GetAtomLength(atom));
+ if (MOZ_LIKELY(JSID_IS_STRING(${idName}))) {
+ if (!AssignJSString(cx, ${argName}, JSID_TO_STRING(${idName}))) {
+ return false;
+ }
} else {
nameVal = js::IdToValue(${idName});
$*{unwrapString}
}
""",
idName=idName,
argName=argName,
unwrapString=unwrapString)
@@ -9392,17 +9376,17 @@ class CGProxyNamedOperation(CGProxySpeci
# Sadly, we have to set up nameVal even if we have an atom id,
# because we don't know for sure, and we can end up needing it
# so it needs to be higher up the stack. Using a Maybe here
# seems like probable overkill.
return fill(
"""
JS::Rooted<JS::Value> nameVal(cx);
$*{idDecl}
- binding_detail::FakeDependentString ${argName};
+ binding_detail::FakeString ${argName};
$*{unwrapString}
${nativeType}* self = UnwrapProxy(proxy);
$*{op}
""",
idDecl=idDecl,
argName=argName,
unwrapString=unwrapString,
@@ -13538,17 +13522,17 @@ class GlobalGenRoots():
(includes, implincludes,
declarations, unions) = UnionTypes(config.getDescriptors(),
config.getDictionaries(),
config.getCallbacks(),
config)
includes.add("mozilla/dom/OwningNonNull.h")
includes.add("mozilla/dom/UnionMember.h")
includes.add("mozilla/dom/BindingDeclarations.h")
- # Need BindingUtils.h for FakeDependentString
+ # Need BindingUtils.h for FakeString
includes.add("mozilla/dom/BindingUtils.h")
implincludes.add("mozilla/dom/PrimitiveConversions.h")
# Wrap all of that in our namespaces.
curr = CGNamespace.build(['mozilla', 'dom'], unions)
curr = CGWrapper(curr, post='\n')
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -765,16 +765,18 @@ GetObjectSlot(JSObject *obj, size_t slot
}
MOZ_ALWAYS_INLINE size_t
GetAtomLength(JSAtom *atom)
{
return reinterpret_cast<shadow::String*>(atom)->length;
}
+static const uint32_t MaxStringLength = (1 << 28) - 1;
+
MOZ_ALWAYS_INLINE size_t
GetStringLength(JSString *s)
{
return reinterpret_cast<shadow::String*>(s)->length;
}
MOZ_ALWAYS_INLINE bool
LinearStringHasLatin1Chars(JSLinearString *s)
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -251,17 +251,17 @@ class JSString : public js::gc::Barriere
/* Initial flags for inline and fat inline strings. */
static const uint32_t INIT_INLINE_FLAGS = FLAT_BIT | INLINE_CHARS_BIT;
static const uint32_t INIT_FAT_INLINE_FLAGS = FLAT_BIT | FAT_INLINE_MASK;
static const uint32_t TYPE_FLAGS_MASK = JS_BIT(6) - 1;
static const uint32_t LATIN1_CHARS_BIT = JS_BIT(6);
- static const uint32_t MAX_LENGTH = JS_BIT(28) - 1;
+ static const uint32_t MAX_LENGTH = js::MaxStringLength;
static const JS::Latin1Char MAX_LATIN1_CHAR = 0xff;
/*
* Helper function to validate that a string of a given length is
* representable by a JSString. An allocation overflow is reported if false
* is returned.
*/