Closed Bug 1021670 Opened 6 years ago Closed 6 years ago

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

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: ehsan, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Attached patch Test patchSplinter Review
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',
This error was originally added in bug 750297, and it seems like if I make XPathNSResolver wrapperCached, it goes away.
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?
Flags: needinfo?(ehsan)
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?
Flags: needinfo?(bzbarsky)
Might as well, so someone else won't run into it.
Flags: needinfo?(bzbarsky)
OK.
Assignee: nobody → ehsan
Flags: needinfo?(ehsan)
Attachment #8436248 - Flags: review?(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.
Attachment #8436248 - Flags: review?(bzbarsky) → review-
Attachment #8436248 - Attachment is obsolete: true
Attachment #8436266 - Flags: review?(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
Attachment #8436266 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/7d4ed44d5913
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.