[webkit-reviews] review granted: [Bug 7141] deploy RefPtr and PassRefPtr in more places : [Attachment 6342] use RefPtr in Range, editing, change local PassRefPtr to RefPtr, remove extra ref/deref

[bugzilla-request-daemon at opendarwin.org]

Eric Seidel <macdome at opendarwin.org> has granted Darin Adler
<darin at apple.com>'s request for review:
Bug 7141: deploy RefPtr and PassRefPtr in more places
http://bugzilla.opendarwin.org/show_bug.cgi?id=7141

Attachment 6342: use RefPtr in Range, editing, change local PassRefPtr to
RefPtr, remove extra ref/deref
http://bugzilla.opendarwin.org/attachment.cgi?id=6342&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
You may need a RefPtr here:

-    while (NodeImpl *n = newParent->firstChild()) {
-	 n->ref();
-	 newParent->removeChild(n,exceptioncode);
-	 n->deref();
+    while (NodeImpl* n = newParent->firstChild()) {
+	 newParent->removeChild(n, exceptioncode);
	 if (exceptioncode)

I found the ExceptionCode typdef a bit out of place for this patch.  But
perhaps we plan to use it in more places?

I'm a fan of this change:
+    ASSERT(refCount() || parent());

Although I can't ever see a situation where a node doesn't have a refCount()
but does have a parent(), so I'm not sure that the "or" is valuable there.

I don't quite understand the need for the double release here:

+	 list->appendItem(t.release().release());

This change should be performance testsed.

Otherwise, things look fine.  r=me.