Closed Bug 753518 Opened 10 years ago Closed 10 years ago

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

Categories

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

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 1 obsolete file)

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.  ;)
Whiteboard: [need review]
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
Closed: 10 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.