Bug 958576 part 2. Move FakeDependentString to the binding_detail namespace. r=peterv

I took the opportunity to move away from the NonNull<nsAString> hack
we had for string arguments, since just passing in a
FakeDependentString works fine and callees are now less likely to
declare their arg as being of that type.

In the process of doing that, I ran into what looks like a substantive
bug in the "owning union with string default value" case: We were
doing mValue.mString.Value() without ever having constructed
mValue.mString!
This commit is contained in:
Boris Zbarsky 2014-01-22 14:37:10 -05:00
parent 9f27c4fd14
commit 25243ecc6c
5 changed files with 25 additions and 13 deletions

View File

@ -3154,7 +3154,7 @@ nsGenericHTMLElement::SetItemValue(JSContext* aCx, JS::Value aValue,
return;
}
FakeDependentString string;
binding_detail::FakeDependentString string;
JS::Rooted<JS::Value> value(aCx, aValue);
if (!ConvertJSValueToString(aCx, value, &value, eStringify, eStringify, string)) {
aError.Throw(NS_ERROR_UNEXPECTED);

View File

@ -322,7 +322,9 @@ public:
// internal strings. So we just have to forward-declare it and reimplement its
// ToAStringPtr.
namespace binding_detail {
struct FakeDependentString;
} // namespace binding_detail
template<>
class Optional<nsAString>
@ -344,7 +346,7 @@ public:
// If this code ever goes away, remove the comment pointing to it in the
// FakeDependentString class in BindingUtils.h.
void operator=(const FakeDependentString* str)
void operator=(const binding_detail::FakeDependentString* str)
{
MOZ_ASSERT(str);
mStr = reinterpret_cast<const nsDependentString*>(str);

View File

@ -1456,6 +1456,8 @@ 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 {
@ -1533,6 +1535,8 @@ private:
};
};
} // namespace binding_detail
enum StringificationBehavior {
eStringify,
eEmpty,
@ -1545,7 +1549,7 @@ ConvertJSValueToString(JSContext* cx, JS::Handle<JS::Value> v,
JS::MutableHandle<JS::Value> pval,
StringificationBehavior nullBehavior,
StringificationBehavior undefinedBehavior,
FakeDependentString& result)
binding_detail::FakeDependentString& result)
{
JSString *s;
if (v.isString()) {

View File

@ -3636,7 +3636,7 @@ for (uint32_t i = 0; i < length; ++i) {
assignString = "${declName} = str"
return JSToNativeConversionInfo(
"{\n"
" FakeDependentString str;\n"
" binding_detail::FakeDependentString str;\n"
"%s\n"
" %s;\n"
"}\n" % (
@ -3646,16 +3646,20 @@ for (uint32_t i = 0; i < length; ++i) {
if isOptional:
declType = "Optional<nsAString>"
holderType = CGGeneric("binding_detail::FakeDependentString")
conversionCode = ("%s\n"
"${declName} = &${holderName};" %
getConversionCode("${holderName}"))
else:
declType = "NonNull<nsAString>"
declType = "binding_detail::FakeDependentString"
holderType = None
conversionCode = getConversionCode("${declName}")
# No need to deal with optional here; we handled it already
return JSToNativeConversionInfo(
("%s\n"
"${declName} = &${holderName};" %
getConversionCode("${holderName}")),
conversionCode,
declType=CGGeneric(declType),
holderType=CGGeneric("FakeDependentString"))
holderType=holderType)
if type.isByteString():
assert not isEnforceRange and not isClamp
@ -6942,7 +6946,7 @@ class CGUnionStruct(CGThing):
[Argument("const nsString::char_type*", "aData"),
Argument("nsString::size_type", "aLength")],
inline=True, bodyInHeader=True,
body="mValue.mString.Value().Assign(aData, aLength);"))
body="SetAsString().Assign(aData, aLength);"))
body = string.Template('MOZ_ASSERT(Is${name}(), "Wrong type!");\n'
'mValue.m${name}.Destroy();\n'
@ -7141,7 +7145,7 @@ class CGUnionConversionStruct(CGThing):
[Argument("const nsDependentString::char_type*", "aData"),
Argument("nsDependentString::size_type", "aLength")],
inline=True, bodyInHeader=True,
body="mStringHolder.SetData(aData, aLength);"))
body="SetAsString().SetData(aData, aLength);"))
if vars["holderType"] is not None:
members.append(ClassMember("m%sHolder" % vars["name"],
@ -7965,7 +7969,7 @@ class CGProxyNamedOperation(CGProxySpecialOperation):
# seems like probable overkill.
return ("JS::Rooted<JS::Value> nameVal(cx);\n" +
idDecl +
("FakeDependentString %s;\n" % argName) +
("binding_detail::FakeDependentString %s;\n" % argName) +
unwrapString +
("\n"
"\n"
@ -11601,6 +11605,8 @@ struct PrototypeTraits;
includes.add("mozilla/dom/OwningNonNull.h")
includes.add("mozilla/dom/UnionMember.h")
includes.add("mozilla/dom/BindingDeclarations.h")
# Need BindingUtils.h for FakeDependentString
includes.add("mozilla/dom/BindingUtils.h")
implincludes.add("mozilla/dom/PrimitiveConversions.h")
# Wrap all of that in our namespaces.

View File

@ -173,7 +173,7 @@ nsJSEventListener::HandleEvent(nsIDOMEvent* aEvent)
(scriptEvent->message == NS_LOAD_ERROR ||
scriptEvent->typeString.EqualsLiteral("error"))) {
errorMsg = scriptEvent->errorMsg;
msgOrEvent.SetAsString() = static_cast<nsAString*>(&errorMsg);
msgOrEvent.SetAsString().SetData(errorMsg.Data(), errorMsg.Length());
file = scriptEvent->fileName;
fileName = &file;