[webkit-reviews] review denied: [Bug 23490] Remove initialRefCount argument from RefCounted class : [Attachment 27063] Patch v1

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied David Kilzer (ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 23490: Remove initialRefCount argument from RefCounted class
https://bugs.webkit.org/show_bug.cgi?id=23490

Attachment 27063: Patch v1
https://bugs.webkit.org/attachment.cgi?id=27063&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   static PassRefPtr<AccessibilityObjectWrapper>
create(PassRefPtr<AccessibilityObject> obj)
> +	   {
> +	       return adoptRef(new AccessibilityObjectWrapper(obj));
> +	   }
> +	   static PassRefPtr<AccessibilityObjectWrapper> create()
> +	   {
> +	       return adoptRef(new AccessibilityObjectWrapper());
> +	   }

This is an abstract class which can't be instantiated, because of the pure
virtual detach function, so these create functions won't do anyone any good.
Presumably the code that calls new and needs to do adoptRef is somewhere else
in the Chromium project, not checked into the WebKit source tree. So you can't
make this change without creating a Chrome-specific storage leak unless the
Chrome team checks in some more of their code!

> +	   AccessibilityObjectWrapper(PassRefPtr<AccessibilityObject> obj) :
m_object(obj) { }

How about "object" instead of "obj"?

> -	   AccessibilityObject* m_object;
> +	   RefPtr<AccessibilityObject> m_object;

Are you sure it's right to change this to a RefPtr? This doesn't seem related
to your RefCounted initial reference count fix. Maybe the Chrome folks have a
reason to not use a RefPtr here?

> +	       return adoptRef(new FrameLoaderClientWx());

No need for the parentheses here.

Why not take the initial reference count argument out of RefCountedBase too,
while you're at it? The only client for that is in ByteArray.h and it passes a
1 too.

review- for now, but this looks mostly fine.