[webkit-reviews] review denied: [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 bugzilla-request-daemon at opendarwin.org
Wed Feb 8 10:18:44 PST 2006


Geoffrey Garen <ggaren at apple.com> has denied Eric Seidel
<macdome at opendarwin.org>'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 Geoffrey Garen <ggaren at apple.com>
I love this patch.

In RangeImpl::~RangeImpl(), it looks like you overzealously removed the lines
  -    if (!m_detached)
  -	   detach(exceptioncode);
I don't see how that relates to the RefPtr change. r- until we straighten that
out.

In RangeImpl::processContents, 
  RefPtr<NodeImpl> rightContents = 0;
could nix the "= 0", and
  rightContents = c;
could change to "c.release()", to match what you've done with leftContents.

I'd like to see a comment about why
  ASSERT(refCount() || parent());
must be true. Maybe if I worked in this code it would be obvious, though.



More information about the webkit-reviews mailing list