Last Comment Bug 753518 - Add a way to return a (nsISupports, nsWrapperCache) pair from GetParentObject
: Add a way to return a (nsISupports, nsWrapperCache) pair from GetParentObject
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 753517
  Show dependency treegraph
 
Reported: 2012-05-09 13:36 PDT by Boris Zbarsky [:bz]
Modified: 2012-05-10 18:29 PDT (History)
2 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add an explicit way for GetParentObject to return an (nsISupports*,nsWrapperCache*) pair. (2.75 KB, patch)
2012-05-09 13:40 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Splinter Review
Working on smart pointers too (2.94 KB, patch)
2012-05-09 15:05 PDT, Boris Zbarsky [:bz]
peterv: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2012-05-09 13:36:22 PDT
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 [:bz] 2012-05-09 13:40:21 PDT
Created attachment 622494 [details] [diff] [review]
Add an explicit way for GetParentObject to return an (nsISupports*,nsWrapperCache*) pair.
Comment 2 Boris Zbarsky [:bz] 2012-05-09 15:05:39 PDT
Created attachment 622523 [details] [diff] [review]
Working on smart pointers too
Comment 3 Peter Van der Beken [:peterv] - away till Aug 1st 2012-05-10 02:44:22 PDT
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 [:bz] 2012-05-10 06:32:58 PDT
> 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 [:bz] 2012-05-10 12:25:37 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/893524af2d1a with the name change.
Comment 6 Joe Drew (not getting mail) 2012-05-10 18:29:20 PDT
https://hg.mozilla.org/mozilla-central/rev/893524af2d1a

Note You need to log in before you can comment on or make changes to this bug.