The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla15

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

unspecified
mozilla15
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.  ;)
Created attachment 622494 [details] [diff] [review]
Add an explicit way for GetParentObject to return an (nsISupports*,nsWrapperCache*) pair.
Attachment #622494 - Flags: review?(peterv)
Whiteboard: [need review]
Created attachment 622523 [details] [diff] [review]
Working on smart pointers too
Attachment #622523 - Flags: review?(peterv)
Attachment #622494 - Attachment is obsolete: true
Attachment #622494 - Flags: review?(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 :-(.
Attachment #622523 - Flags: review?(peterv) → review+
> 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.  ;)
http://hg.mozilla.org/integration/mozilla-inbound/rev/893524af2d1a with the name change.
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/893524af2d1a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.