Bug 1013576 - Guard against duplicate property holder creation in XBL. r=billm

This commit is contained in:
Bobby Holley 2014-05-22 18:44:03 -07:00
parent d7647deae8
commit be363a4ae6
2 changed files with 40 additions and 9 deletions

View File

@ -81,9 +81,27 @@ nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aPrototypeBinding,
NS_ENSURE_TRUE(scopeObject, NS_ERROR_OUT_OF_MEMORY);
JSAutoCompartment ac(cx, scopeObject);
// If they're different, create our safe holder object in the XBL scope.
// Determine the appropriate property holder.
//
// Note: If |targetIsNew| is false, we'll early-return above. However, that only
// tells us if the content-side object is new, which may be the case even if
// we've already set up the binding on the XBL side. For example, if we apply
// a binding #foo to a <span> when we've already applied it to a <div>, we'll
// end up with a different content prototype, but we'll already have a property
// holder called |foo| in the XBL scope. Check for that to avoid wasteful and
// weird property holder duplication.
const char* className = aPrototypeBinding->ClassName().get();
JS::Rooted<JSObject*> propertyHolder(cx);
if (scopeObject != globalObject) {
JS::Rooted<JSPropertyDescriptor> existingHolder(cx);
if (scopeObject != globalObject &&
!JS_GetOwnPropertyDescriptor(cx, scopeObject, className, &existingHolder)) {
return NS_ERROR_FAILURE;
}
bool propertyHolderIsNew = !existingHolder.object() || !existingHolder.value().isObject();
if (!propertyHolderIsNew) {
propertyHolder = &existingHolder.value().toObject();
} else if (scopeObject != globalObject) {
// This is just a property holder, so it doesn't need any special JSClass.
propertyHolder = JS_NewObjectWithGivenProto(cx, nullptr, JS::NullPtr(), scopeObject);
@ -91,8 +109,8 @@ nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aPrototypeBinding,
// Define it as a property on the scopeObject, using the same name used on
// the content side.
bool ok = JS_DefineProperty(cx, scopeObject, aPrototypeBinding->ClassName().get(),
propertyHolder, JSPROP_PERMANENT | JSPROP_READONLY,
bool ok = JS_DefineProperty(cx, scopeObject, className, propertyHolder,
JSPROP_PERMANENT | JSPROP_READONLY,
JS_PropertyStub, JS_StrictPropertyStub);
NS_ENSURE_TRUE(ok, NS_ERROR_UNEXPECTED);
} else {
@ -100,10 +118,12 @@ nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aPrototypeBinding,
}
// Walk our member list and install each one in turn on the XBL scope object.
for (nsXBLProtoImplMember* curr = mMembers;
curr;
curr = curr->GetNext())
curr->InstallMember(cx, propertyHolder);
if (propertyHolderIsNew) {
for (nsXBLProtoImplMember* curr = mMembers;
curr;
curr = curr->GetNext())
curr->InstallMember(cx, propertyHolder);
}
// Now, if we're using a separate XBL scope, enter the compartment of the
// bound node and copy exposable properties to the prototype there. This

View File

@ -12,7 +12,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=821850
// Wait for both constructors to fire.
window.constructorCount = (window.constructorCount + 1) || 1;
if (window.constructorCount != 2)
if (window.constructorCount != 3)
return;
// Grab some basic infrastructure off the content window.
@ -37,6 +37,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=821850
ok(Cu.isXrayWrapper(document), "Document is Xrayed");
var bound = document.getElementById('bound');
var bound2 = document.getElementById('bound2');
var bound3 = document.getElementById('bound3');
ok(bound, "bound is non-null");
is(bound.method('baz'), "method:baz", "Xray methods work");
is(bound.prop, "propVal", "Property Xrays work");
@ -68,6 +70,13 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=821850
is(Object.getPrototypeOf(Function.prototype), Object.getPrototypeOf({foo: 42}),
"Function constructor is local");
ok(Object.getPrototypeOf(bound.wrappedJSObject) == Object.getPrototypeOf(bound2.wrappedJSObject),
"Div and div should have the same content-side prototype");
ok(Object.getPrototypeOf(bound.wrappedJSObject) != Object.getPrototypeOf(bound3.wrappedJSObject),
"Div and span should have different content-side prototypes");
ok(bound.wrappedJSObject.method == bound3.wrappedJSObject.method,
"Methods should be shared");
// This gets invoked by an event handler.
window.finish = function() {
// Content messed with stuff. Make sure we still see the right thing.
@ -279,6 +288,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=821850
// test will continue.
document.getElementById('bound').style.MozBinding = 'url(#testBinding)';
document.getElementById('bound2').style.MozBinding = 'url(#testBinding)';
document.getElementById('bound3').style.MozBinding = 'url(#testBinding)';
}
]]>
@ -290,6 +300,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=821850
<div id="content">
<div id="bound">Bound element</div>
<div id="bound2">Bound element</div>
<span id="bound3">Bound element</span>
<img/>
</div>
<pre id="test">