1021670 - Codegen.py throws MethodNotNewObjectError with a method which takes a union of a callback and a normal interface

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[(no longer active)]

Attachment: Test patch

STR: apply the attached patch and build.  You'll get:

 0:04.60 Traceback (most recent call last):
 0:04.60   File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 162, in _run_module_as_main
 0:04.60     "__main__", fname, loader, pkg_name)
 0:04.60   File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
 0:04.60     exec code in run_globals
 0:04.60   File "/Users/ehsan/moz/src/python/mozbuild/mozbuild/action/webidl.py", line 17, in <module>
 0:04.60     sys.exit(main(sys.argv[1:]))
 0:04.60   File "/Users/ehsan/moz/src/python/mozbuild/mozbuild/action/webidl.py", line 13, in main
 0:04.60     manager.generate_build_files()
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/mozwebidlcodegen/__init__.py", line 244, in generate_build_files
 0:04.60     created, updated, unchanged = self._write_global_derived()
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/mozwebidlcodegen/__init__.py", line 335, in _write_global_derived
 0:04.60     root = getattr(GlobalGenRoots, stem)(self._config)
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 13385, in UnionTypes
 0:04.60     config)
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 1172, in UnionTypes
 0:04.60     unionStructs[name] = CGUnionStruct(t, providers[0])
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 8087, in __init__
 0:04.60     self.struct = self.getStruct()
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 8218, in getStruct
 0:04.60     self.getConversionToJS(vars, t)))
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 8321, in getConversionToJS
 0:04.60     "typedArraysAreStructs": True
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 5534, in wrapForType
 0:04.60     False))[0]
 0:04.60   File "/Users/ehsan/moz/src/dom/bindings/Codegen.py", line 5332, in getWrapTemplateForType
 0:04.61     raise MethodNotNewObjectError(descriptor.interface.identifier.name)
 0:04.61 Codegen.MethodNotNewObjectError

Here's what I added on top of the existing patches in bug 1018658:

diff --git a/dom/webidl/XPathEvaluator.webidl b/dom/webidl/XPathEvaluator.webidl
index 7f8c7e8..3f6cc88 100644
--- a/dom/webidl/XPathEvaluator.webidl
+++ b/dom/webidl/XPathEvaluator.webidl
@@ -17,6 +17,6 @@ interface XPathEvaluator {
   XPathNSResolver createNSResolver(Node? nodeResolver);
   [Throws]
   XPathResult evaluate(DOMString expression, Node? contextNode,
-                       XPathNSResolver? resolver, unsigned short type,
-                       XPathResult? result);
+                       (XPathNSResolver or XPathNSResolverInternal)? resolver,
+                       unsigned short type, XPathResult? result);
 };
diff --git a/dom/webidl/XPathNSResolverInternal.webidl b/dom/webidl/XPathNSResolverInternal.webidl
new file mode 100644
index 0000000..082faf7
--- /dev/null
+++ b/dom/webidl/XPathNSResolverInternal.webidl
@@ -0,0 +1,9 @@
+/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+callback interface XPathNSResolverInternal {
+  DOMString          lookupNamespaceURI(DOMString prefix);
+};
diff --git a/dom/webidl/moz.build b/dom/webidl/moz.build
index 37ded8e..68ae2b7 100644
--- a/dom/webidl/moz.build
+++ b/dom/webidl/moz.build
@@ -474,6 +474,7 @@ WEBIDL_FILES = [
     'XMLStylesheetProcessingInstruction.webidl',
     'XPathEvaluator.webidl',
     'XPathNSResolver.webidl',
+    'XPathNSResolverInternal.webidl',
     'XULCommandEvent.webidl',
     'XULDocument.webidl',
     'XULElement.webidl',

Comment 1

[(no longer active)]

This error was originally added in bug 750297, and it seems like if I make XPathNSResolver wrapperCached, it goes away.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, but that's silly.

The reason the error is there is that unions have a "convert this union to JS" function, and _that_ can't work right with a non-wrapper-cached union member, in general.

What I think we should do in this case is to catch the exception from self.getConversionToJS and not output the ToJSVal method on the union.  This is what we already do for the self.toObjectMethod() bit on dictionaries, say.

Ehsan, do you want to do that, or should I?

Comment 3

[(no longer active)]

I'd be happy to do that, but it seems like Peter has patches for bug 1018658 which are better than mine, so we may not need this immediately.  With that, do you still think we should fix this now?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Might as well, so someone else won't run into it.

Comment 5

[(no longer active)]

OK.

Comment 6

[(no longer active)]

Attachment: Enable using non-wrapper-cached objects in unions; r=bzbarsky

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8436248 [details] [diff] [review]
Enable using non-wrapper-cached objects in unions; r=bzbarsky

You need to set a flag that will then be used to actually skip generating ToJSVal, instead of generating one with missing cases.

Comment 8

[(no longer active)]

Attachment: Enable using non-wrapper-cached objects in unions; r=bzbarsky

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8436266 [details] [diff] [review]
Enable using non-wrapper-cached objects in unions; r=bzbarsky

>+                pass

Don't need that bit.

r=me

Comment 10

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d4ed44d5913

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7d4ed44d5913