[webkit-reviews] review denied: [Bug 8791] XPath should support custom node resolvers : [Attachment 12892] proposed fix

[bugzilla-request-daemon at macosforge.org]

Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 8791: XPath should support custom node resolvers
http://bugs.webkit.org/show_bug.cgi?id=8791

Attachment 12892: proposed fix
http://bugs.webkit.org/attachment.cgi?id=12892&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good.

I don't see make file changes for the platforms other than Macintosh.

+String JSCustomXPathNSResolver::lookupNamespaceURI(const String& prefix)
+{
+    String result;

Would be better to declare this closer to where it's used.

+    RefPtr<JSCustomXPathNSResolver> selfProtector(this);

Is there a way to avoid this? Can we just get the values out of member
variables and then not access this after calling the function?

Is there a way to cut down on the amount of custom code that's in the
CodeGeneratorJS.pm and CodeGeneratorObjC.pm scripts? Can we make helper
functions for the JSCustomXPathNSResolver path so that a little more of the
work is in a .cpp file rather than inside a script?

The header guards in NativeXPathNSResolver.h say JSNativeXPathNSResolver_h.

I'm going to say review- even though this is very close to a review+.