[webkit-reviews] review denied: [Bug 20868] webkit should use AX array centric API for performance : [Attachment 23478] patch to implement AX API

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 16 11:54:24 PDT 2008

Darin Adler <darin at apple.com> has denied chris fleizach <cfleizach at apple.com>'s
request for review:
Bug 20868: webkit should use AX array centric API for performance

Attachment 23478: patch to implement AX API

------- Additional Comments from Darin Adler <darin at apple.com>
 7	   Implemenent a few AX API methods that will be called by AppKit,
which will 

Typo in "Implement".

 103 @interface NSObject (WebKitAccessibilityArrayCategory)

This seems strange. Why is this a category on an arbitrary NSObject? If this
really is part of a contract with AppKit I'd like to see it in a header. Will
we always want to declare it this way, or will we have a more elegant way to do
it long term.

 1934	  unsigned k = 0, count = children.size();

We don't do more than one of these at the same time.

 1949	      NSArray* widgetChildren = [self renderWidgetChildren];
 1950	      if (widgetChildren)
 1951		  return [widgetChildren count];
 1952	      return 0;

There's no need for a nil check here since Objective-C already gives 0 if you
message to nil.

 1966		  if (childCount > ([children count]-index))
 1967		  {
 1968		      childCount = ([children count]-index);
 1969		  }

We don't do parentheses around if statement bodies like this; and there are
missing spaces around the "-" operator and the parentheses are unneeded around
the "-" expression. I think this would read better if you used the min()
function and it would be more efficient too since it wouldn't call the function

What guarantees that index is not > [children count]?

 1974	      unsigned added = 0, k = index, count = children.size(), available
= MIN(count - index, maxCount);

It'd be better to use the min() function from C++ rather than the MIN macro.

I think I'll say review- because of the comments above, but this seems close.

More information about the webkit-reviews mailing list