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

[bugzilla-request-daemon at macosforge.org]

Alexey Proskuryakov <ap at webkit.org> has asked  for review:
Bug 8791: XPath should support custom node resolvers
http://bugs.webkit.org/show_bug.cgi?id=8791

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
(In reply to comment #4)

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

  Done.

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

  Moved it a little down.

> +    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?

  I don't know for sure - took it from JSAbstractEventListener::handleEvent().
I suppose it may be needed to protect the frame. Would be nice if these
functions could share more code some day, but I don't feel myself familiar
enough with this code to refactor it.

> 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?

  I added a create() function to JSCustomXPathNSResolver, and that saved a few
lines in generated code, but not much - so maybe I don't fully understand your
suggestion.

> The header guards in NativeXPathNSResolver.h say JSNativeXPathNSResolver_h.

  Fixed.

  Also added BLOCK_OBJC_EXCEPTIONS to the ObjC resolver.