Bug 216945

Created attachment 409642 [details] Patch

Created attachment 409645 [details] Patch

Created attachment 409647 [details] Patch

Created attachment 409648 [details] Patch

Any thoughts on this?

Comment on attachment 409648 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409648&action=review > Source/WTF/ChangeLog:11 > + Add third template parameter to RefPtr allowing to define the traits > + from incrementing / decrementing the refcount. The default traits > + call ref() / deref() but this can now be customized to call other > + functions. What about all the other RefPtr-like classes? Could RetainPtr be defined like this, or does it have features that are too fancy? WKRetainPtr? JSRetainPtr? NakedPtr? OSObjectPtr? > Source/WTF/wtf/RefPtr.h:49 > +template<typename T, typename _PtrTraits, typename _RefDerefTraits> Why the leading underscores? I think that’s technically reserved for the standard library.

(In reply to Darin Adler from comment #6) > Comment on attachment 409648 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=409648&action=review > > > Source/WTF/ChangeLog:11 > > + Add third template parameter to RefPtr allowing to define the traits > > + from incrementing / decrementing the refcount. The default traits > > + call ref() / deref() but this can now be customized to call other > > + functions. > > What about all the other RefPtr-like classes? Could RetainPtr be defined > like this, or does it have features that are too fancy? WKRetainPtr? > JSRetainPtr? NakedPtr? OSObjectPtr? We don't have such need for the other classes yet. I had initially written my own class to wrap an AudioNode and call incrementConnectionCount() in constructor and decrementConnectionCount() in the destructor. I eventually realized I was duplicating a lot of the RefPtr code so I decided to make RefPtr more flexible so that I can reuse it instead. If you don't like this approach though, I can try and go back to my original idea. I do want the calls to incrementConnectionCount() / decrementConnectionCount() to be abstracted away though to reduce the chance of leaks. > > Source/WTF/wtf/RefPtr.h:49 > > +template<typename T, typename _PtrTraits, typename _RefDerefTraits> > > Why the leading underscores? I think that’s technically reserved for the > standard library. I could not do: using PtrTraits = PtrTraits; So I needed a way to differentiate the template parameter. I decided to use an underscore but I am open to suggestions.

Committed r267591: <https://trac.webkit.org/changeset/267591> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409648 [details].

<rdar://problem/69591533>

By the way, it looks like this has broken binary compatibility with open-source builds (because those builds use the system WebInspector.framework).

(In reply to Myles C. Maxfield from comment #10) > By the way, it looks like this has broken binary compatibility with > open-source builds (because those builds use the system > WebInspector.framework). I am unclear what this change has to do with WebInspector.framework? I do not see any changes to inspector bits in this patch.

I’m guessing it’s the change to RefPtr template arguments.

(In reply to Darin Adler from comment #12) > I’m guessing it’s the change to RefPtr template arguments. Oh, that makes sense indeed.

I don’t understand how mismatched WebKit and WebInspector frameworks is expected to work. It’s untenable to have such a fragile API contract between them; not sure when we decided on this strategy.

See also: https://bugs.webkit.org/show_bug.cgi?id=217778 and https://bugs.webkit.org/show_bug.cgi?id=215961