[webkit-reviews] review granted: [Bug 4247] addEventListener in Obj-C does not work : [Attachment 3190] patch to implement this

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sat Sep 17 14:33:28 PDT 2005


Eric Seidel <macdome at opendarwin.org> has granted Darin Adler
<darin at apple.com>'s request for review:
Bug 4247: addEventListener in Obj-C does not work
http://bugzilla.opendarwin.org/show_bug.cgi?id=4247

Attachment 3190: patch to implement this
http://bugzilla.opendarwin.org/attachment.cgi?id=3190&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
Looks fine.  Two comments:

1.  I don't understand the ListenerMap map = listenerMap; dance that you do in
two places.  You had made some comment before about locals being faster to
access... but particularly in the ObjCEventListener::find case it seems
superfluous.

2.  Although I personally agree with the style of returning objects
ref-counted... ObjCEventListener::create differs from the rest of the C++ code
in WebCore by doing so.  I think that wrapper should be returned floating and
stuck into a SharedPtr if necessary, or simply returned as a
SharedPtr<ObjCEventListener> for consistancy.

I'll leave those corrections to your discression.  r=me.



More information about the webkit-reviews mailing list