753518 - Add a way to return a (nsISupports, nsWrapperCache) pair from GetParentObject

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

We'll need this for CSS declarations, which sometimes want to return a DOM rule and sometimes want to return a node as the parent... they could return the relevant nsISupports, and then set the wrapper cache bit as needed.

Please feel free to tell me that the GetPointer and ParentObject names suck, of course.  ;)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Add an explicit way for GetParentObject to return an (nsISupports*,nsWrapperCache*) pair.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Working on smart pointers too

Comment 3

[Peter Van der Beken [:peterv]]

Comment on attachment 622523 [details] [diff] [review]
Working on smart pointers too

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

::: dom/bindings/BindingUtils.h
@@ +423,5 @@
> +struct ParentObject {
> +  template<class T>
> +  ParentObject(T* aObject) :
> +    mObject(aObject),
> +    mWrapperCache(GetWrapperCache(aObject))

Could we get away with mWrapperCache(aObject)? I sort of like forcing things that return ParentObject to either pass in a parent pointer that inherits from nsWrapperCache or pass a nsWrapperCache into the other constructor.

@@ +432,5 @@
> +    mObject(aObject.get()),
> +    mWrapperCache(GetWrapperCache(aObject.get()))
> +  {}
> +
> +  ParentObject(nsISupports* aObject, nsWrapperCache* aCache) :

If we can do the above, MOZ_ASSERT that aCache is non-null here (probably only if aObject is non-null)?

@@ +449,5 @@
> +}
> +
> +template<class T>
> +inline nsISupports*
> +GetPointer(T* aObject)

GetParentPointer? Naming is hard :-(.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Could we get away with mWrapperCache(aObject)?

Wouldn't that do the wrong thing for Windows?  Modulo that, I believe the behavior of GetWrapperCache() on a pointer type is identical to pointer assignment, right?

> If we can do the above, MOZ_ASSERT that aCache is non-null here 

That would fail; we have parent objects that are not nsWrapperCached so far.

> GetParentPointer

Can do.  And I _said_ I wasn't happy with the names.  ;)

Comment 5

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/893524af2d1a with the name change.

Comment 6

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/893524af2d1a