Bug 27001 – Fix improper use of PassRefPtr<Node> to RefPtr<Node>

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[David Kilzer (:ddkilzer)]

Created attachment 32318 [details]
Patch v1

Fix improper use of PassRefPtr<Node> to RefPtr<Node>

Reviewed by NOBODY (OOPS!).

PassRefPtr<> should only be used for arguments to functions that take ownership of the object, or as return values from functions that relinquish ownership of the object.

* editing/Editor.cpp:
(WebCore::Editor::increaseSelectionListLevelOrdered): Changed stack-allocated PassRefPtr<Node> to RefPtr<Node> and call release() on returned object to prevent ref count churn.
(WebCore::Editor::increaseSelectionListLevelUnordered): Ditto.

[Geoffrey Garen]

Comment on attachment 32318 [details]
Patch v1

>         (WebCore::Editor::increaseSelectionListLevelOrdered): Changed
>         stack-allocated PassRefPtr<Node> to RefPtr<Node> and call
>         release() on returned object to prevent ref count churn.

Technically, I don't think there would be refcount churn, since the default behavior of PassRefPtr is to give up its reference upon assignment:

        template <typename U> PassRefPtr(const PassRefPtr<U>& o) : m_ptr(o.releaseRef()) { }

However, this is still a good change for the style reasons you cited.

[David Kilzer (:ddkilzer)]

(In reply to comment #1)
> (From update of attachment 32318 [details])
> >         (WebCore::Editor::increaseSelectionListLevelOrdered): Changed
> >         stack-allocated PassRefPtr<Node> to RefPtr<Node> and call
> >         release() on returned object to prevent ref count churn.
> 
> Technically, I don't think there would be refcount churn, since the default
> behavior of PassRefPtr is to give up its reference upon assignment:
> 
>         template <typename U> PassRefPtr(const PassRefPtr<U>& o) :
> m_ptr(o.releaseRef()) { }

I'll adjust the changelog entry.  (If you didn't call release(), though, and returned the RefPtr<Node> object, I do believe some ref count churn would be involved creating a PassRefPtr<Node> from a RefPtr<Node>.  That's what I was referring to when using release().)

[David Kilzer (:ddkilzer)]

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/editing/Editor.cpp
Committed r45577

http://trac.webkit.org/changeset/45577