IDL: Fix handling of nullable NodeFilter / XPathNSResolver arguments

IDL: Fix handling of nullable NodeFilter / XPathNSResolver arguments

Arguments of NodeFilter and XPathNSResolver have a special case that adds
a ".release()" suffix to the argument expression. This special case didn't
trigger if the argument was nullable.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176933

[Jens Widell, 2014-06-25 14:01:02]

PTAL

Only two functions take a NodeFilter argument, and in both cases it's supposed
to be nullable per spec. Changing that caused an unexpected CG change, fixed
here.

Note: I *think* we can just stop special casing NodeFilter and XPathNSResolver
here; see discussion below. Would be happy to change this CL to do that instead.

https://codereview.chromium.org/340783012/diff/1/Source/bindings/scripts/v8_m...
File Source/bindings/scripts/v8_methods.py (right):

https://codereview.chromium.org/340783012/diff/1/Source/bindings/scripts/v8_m...
Source/bindings/scripts/v8_methods.py:226: idl_type.name in ['NodeFilter',
'NodeFilterOrNull',
For both NodeFilter and XPathNSResolver, the local variable will be of type
RefPtrWillBeRawPtr<X> and the argument will be of type
PassRefPtrWillBeRawPtr<X>. If I read RefPtr<>/PassRefPtr<> correctly, the
difference would be that the local holds on to a reference until it goes out of
scope without ".release()", and that's it. Should we just drop the special
casing?

With Oilpan and actual raw pointers, we will want to get rid of ".release()"
anyway, and then the effect will be that the local keeps the object alive until
it goes out of scope (at least.)

Note: is_callback_interface still needs the special casing, since it's sometimes
OwnPtr<>, where we certainly need to release.

[haraken, 2014-06-25 14:18:32]

LGTM

> Note: I *think* we can just stop special casing NodeFilter and XPathNSResolver
> here; see discussion below. Would be happy to change this CL to do that
instead.

In terms of correctness, your investigation is right. However, in terms of
semantics, we should call .release() since we're passing an ownership to the
method. So let's land this CL for now.

In long-term, a right fix is to completely remove special-casing for NodeFilter
and XPathNSResolver (remove all custom bindings). I haven't investigated it in
details, but I think it's possible.

[Jens Widell, 2014-06-25 14:24:07]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-06-25 14:25:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jl@opera.com/340783012/1

[commit-bot: I haz the power, 2014-06-25 15:32:03]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/12755)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/13548)

[commit-bot: I haz the power, 2014-06-25 16:36:32]

Change committed as 176933