[webkit-dev] Why are PassRefPtr<>s used as function parameters?

[Drew Wilson]

Aside from the ref-counter munging, it still seems like there's value in
using PassRefPtr as a signifier that a given API takes ownership of a
pointer.

So I'm not certain (in the case of HTMLNameCollection) that it'd be a better
solution to change it not to take an ordinary pointer - yes, it would
eliminate some ref churn, but at the cost of losing that connotation.

A better solution (since the constructor is private) would be to make the
constructor explicitly take a CollectionCache* parameter, thereby allowing
us to directly pass the document straight through to the base class.

-atw

On Thu, Oct 29, 2009 at 3:31 PM, Maciej Stachowiak <mjs at apple.com> wrote:

>
> On Oct 29, 2009, at 12:56 PM, Darin Adler wrote:
>
>  On Oct 27, 2009, at 10:55 AM, Jens Alfke wrote:
>>
>>  Looking at how refcounting is implemented in WebCore, I was surprised to
>>> find that there are a lot of functions/methods that take PassRefPtr<>s as
>>> parameters instead of regular pointers to those objects. I can't see any
>>> benefit to this, and it adds the overhead of a ref() and deref() at every
>>> call-site.
>>>
>>
>> Have you read the RefPtr document? It is at <
>> http://webkit.org/coding/RefPtr.html>.
>>
>> Only functions that take ownership of the passed-in objects should take
>> PassRefPtr. This makes it so we can optimally handle the case where that
>> object was just created and lets us hand off the reference instead of having
>> to do a round of reference count thrash. If the function is going to be
>> storing the value in a RefPtr, then the PassRefPtr does not introduce any
>> reference count thrash even if the passed in object was not just created.
>>
>> It’s wrong for the HTMLNameCollection constructor to take a PassRefPtr for
>> its document argument because it doesn’t take ownership of the document. So
>> that one is definitely a mistake.
>>
>
> However, its base class HTMLCollection can take ownership. The
> HTMLNameCollection constructor misses out on the optimization though:
>
> HTMLNameCollection::HTMLNameCollection(PassRefPtr<Document> document,
> CollectionType type, const String& name)
>    : HTMLCollection(document.get(), type,
> document->nameCollectionInfo(type, name))
>    , m_name(name)
> {
> }
>
> Note that it passes document.get(), but also uses document after that so it
> couldn't just pass document. Thus, it probably shouldn't take a PassRefPtr
> (unless there is a clever way to fix it that I'm not seeing).
>
>  - Maciej
>
>
>
>
>> It is true that PassRefPtr arguments can be tricky as you mentioned and
>> the RefPtr document also mentions.
>>
>>   -- Darin
>>
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20091029/fec84895/attachment.html>