[webkit-reviews] review denied: [Bug 19898] [Gtk] WebKitWebHistoryItem: Fixes and improvements : [Attachment 22504] updated patch based on christian and sven's comments

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Jan Alonzo <jmalonzo at gmail.com>'s
request for review:
Bug 19898: [Gtk] WebKitWebHistoryItem: Fixes and improvements
https://bugs.webkit.org/show_bug.cgi?id=19898

Attachment 22504: updated patch based on christian and sven's comments
https://bugs.webkit.org/attachment.cgi?id=22504&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
This just leads to unsafe access:
 53	WTF::RefPtr<WebCore::HistoryItem> core(WebKitWebHistoryItem*);


Returning a RefPtr is bad-news.  You end up churning the ref-count (the
temporary RefPtr used for the return increments the ref count only to
immediately decrement it again when it's destroyed).  Also things like this end
up being wrong:

core(item).get()

If someone holds onto that pointer, it could have already gone away!   Cause
the temporary RefPtr returned from core() could have been the only thing
holding on to it.

If you already have a RefPtr, you should return:
const RefPtr& which is safe, although really not any better than Class*

PassRefPtr is how we return or pass ref counted objects.