1013576 - Duplicate property holders created in the XBL scope when bindings are applied to multiple element types

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Bobby Holley (:bholley)]

Bill and I discovered this when crawling through the XBL code yesterday. I've got a patch with a large comment explaining the issue.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Guard against duplicate property holder creation in XBL. v1

Comment 2

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=00de36e85f11

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8425772 [details] [diff] [review]
Guard against duplicate property holder creation in XBL. v1

Review of attachment 8425772 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/xbl/nsXBLProtoImpl.cpp
@@ +96,5 @@
> +  if (scopeObject != globalObject &&
> +      !JS_GetOwnPropertyDescriptor(cx, scopeObject, className, &existingHolder)) {
> +    return NS_ERROR_FAILURE;
> +  }
> +  bool propertyHolderIsNew = !existingHolder.object() || !existingHolder.value().isObject();

Could we be a little stronger here and assert that, if it exists, it's an object?

Comment 4

[Bobby Holley (:bholley)]

(In reply to Bill McCloskey (:billm) from comment #3)
> Could we be a little stronger here and assert that, if it exists, it's an
> object?

I don't think we should assert against conditions that script could trigger. And in this case, XBL script could do just that.

Comment 5

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa8381640f9e

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aa8381640f9e