Bug 1121623 part 10. Use a more-accurate default value for 'concrete' in Web IDL bindings. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 24 May 2019 10:43:37 +0000
changeset 475392 1bec5b571ed1d86f15386e0fedb187319e199d4a
parent 475391 99d7cbadce926aad011bc60980eadb87bfa11f81
child 475393 ac95bdf3c0b3f6f0bb31bb49e7714ca93a5a9c9a
child 475422 37b6e7e47cce663039fa21df71433c4add1142c4
push id36060
push usercbrindusan@mozilla.com
push dateFri, 24 May 2019 21:47:21 +0000
treeherdermozilla-central@ac95bdf3c0b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1121623
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1121623 part 10. Use a more-accurate default value for 'concrete' in Web IDL bindings. r=peterv The idea is that we should only generate concreate-binding (wrap methods, etc) machinery for an interface by default if we have reason to expect that the interface is used as the primary interface for some objects. Two clear signals that would indicate that are the interface being a leaf interface (with no descendants) and the interface having a constructor. Other cases would require a 'concrete' annotation in Bindings.conf. Differential Revision: https://phabricator.services.mozilla.com/D32208
dom/bindings/Bindings.conf
dom/bindings/Configuration.py
layout/style/CSSFontFaceRule.cpp
--- a/dom/bindings/Bindings.conf
+++ b/dom/bindings/Bindings.conf
@@ -44,28 +44,16 @@
 #   * implicitJSContext - attributes and methods specified in the .webidl file
 #                         that require a JSContext as the first argument
 #
 # The value for an interface is a dictionary which specifies the
 # descriptor to use when generating that interface's binding.
 
 DOMInterfaces = {
 
-'AbstractWorker': {
-    'concrete': False
-},
-
-'AnimationEffect': {
-    'concrete': False
-},
-
-'AnimationTimeline': {
-    'concrete': False
-},
-
 'AnonymousContent': {
     'wrapperCache': False
 },
 
 'ArchiveReader': {
     'nativeType': 'mozilla::dom::archivereader::ArchiveReader',
 },
 
@@ -77,17 +65,16 @@ DOMInterfaces = {
     'implicitJSContext': [ 'copyToChannel' ],
 },
 
 'AudioBufferSourceNode': {
     'implicitJSContext': [ 'buffer' ],
 },
 
 'AudioNode' : {
-    'concrete': False,
     'binaryNames': {
         'channelCountMode': 'channelCountModeValue',
         'channelInterpretation': 'channelInterpretationValue',
     },
 },
 
 'AudioWorklet': {
     'nativeType': 'mozilla::dom::Worklet',
@@ -105,16 +92,20 @@ DOMInterfaces = {
     'nativeType': 'mozilla::dom::AudioContext',
 },
 
 'BatteryManager': {
     'nativeType': 'mozilla::dom::battery::BatteryManager',
     'headerFile': 'BatteryManager.h'
 },
 
+'BrowsingContext': {
+    'concrete': True,
+},
+
 'Cache': {
     'implicitJSContext': [ 'add', 'addAll', 'match', 'matchAll', 'put',
                            'delete', 'keys' ],
     'nativeType': 'mozilla::dom::cache::Cache',
 },
 
 'CacheStorage': {
     'implicitJSContext': [ 'match' ],
@@ -133,18 +124,18 @@ DOMInterfaces = {
 'CaretPosition' : {
     'nativeType': 'nsDOMCaretPosition',
 },
 
 'ChannelWrapper': {
     'nativeType': 'mozilla::extensions::ChannelWrapper',
 },
 
-'CharacterData': {
-    'concrete': False
+'Client' : {
+    'concrete': True,
 },
 
 'Clipboard' : {
     'implicitJSContext' : ['write', 'writeText', 'read', 'readText'],
 },
 
 'console': {
     'nativeType': 'mozilla::dom::Console',
@@ -157,46 +148,49 @@ DOMInterfaces = {
 'ConvolverNode': {
     'implicitJSContext': [ 'buffer' ],
 },
 
 'Coordinates': {
     'headerFile': 'nsGeoPosition.h'
 },
 
+'Credential' : {
+    'concrete': True,
+},
+
 'Crypto' : {
     'headerFile': 'Crypto.h'
 },
 
 'CSS2Properties': {
     'nativeType': 'nsDOMCSSDeclaration'
 },
 
 'CSSConditionRule': {
-    'concrete': False,
     'nativeType': 'mozilla::css::ConditionRule',
     'headerFile': 'mozilla/css/GroupRule.h',
 },
 
 'CSSGroupingRule': {
-    'concrete': False,
     'nativeType': 'mozilla::css::GroupRule',
 },
 
 'CSSLexer': {
     'wrapperCache': False
 },
 
 'CSSRule': {
-    'concrete': False,
     'nativeType': 'mozilla::css::Rule'
 },
 
 'CSSStyleDeclaration': {
-    'nativeType': 'nsICSSDeclaration'
+    'nativeType': 'nsICSSDeclaration',
+    # Concrete because of the font-face mess.
+    'concrete': True,
 },
 
 'CSSStyleRule': {
     'nativeType': 'mozilla::BindingStyleRule',
 },
 
 'CSSStyleSheet': {
     'nativeType': 'mozilla::StyleSheet',
@@ -247,32 +241,37 @@ DOMInterfaces = {
 },
 
 'DOMRectReadOnly': {
     'headerFile': 'mozilla/dom/DOMRect.h',
 },
 
 'DOMRequest': {
     'implicitJSContext': [ 'then' ],
+    'concrete': True,
 },
 
 'DOMStringMap': {
     'nativeType': 'nsDOMStringMap'
 },
 
 'DOMTokenList': {
     'nativeType': 'nsDOMTokenList',
 },
 
 'DynamicsCompressorNode': {
     'binaryNames': {
         'release': 'getRelease'
     },
 },
 
+'Element': {
+    'concrete': True,
+},
+
 'Event': {
     'implicitJSContext': [ 'preventDefault' ],
 },
 
 'EventTarget': {
     'jsImplParent': 'mozilla::DOMEventTargetHelper',
 },
 
@@ -304,16 +303,20 @@ DOMInterfaces = {
 'FileReader': {
     'implicitJSContext': [ 'readAsArrayBuffer' ],
 },
 
 'FileReaderSync': {
     'wrapperCache': False,
 },
 
+'FileSystemEntry': {
+    'concrete': True,
+},
+
 'FontFaceSet': {
     'implicitJSContext': [ 'load' ],
 },
 
 'FontFaceSetIterator': {
     'wrapperCache': False,
 },
 
@@ -344,46 +347,44 @@ DOMInterfaces = {
 'HTMLBaseElement': {
     'nativeType': 'mozilla::dom::HTMLSharedElement'
 },
 
 'HTMLCollection': {
     'nativeType': 'nsIHTMLCollection',
     # nsContentList.h pulls in nsIHTMLCollection.h
     'headerFile': 'nsContentList.h',
+    'concrete': True,
 },
 
 'HTMLDirectoryElement': {
     'nativeType': 'mozilla::dom::HTMLSharedElement'
 },
 
 'HTMLDListElement': {
     'nativeType' : 'mozilla::dom::HTMLSharedListElement'
 },
 
 'HTMLDocument': {
     'nativeType': 'nsHTMLDocument',
+    'concrete': True,
 },
 
 'HTMLElement': {
     'nativeType': 'nsGenericHTMLElement',
 },
 
 'HTMLHeadElement': {
     'nativeType': 'mozilla::dom::HTMLSharedElement'
 },
 
 'HTMLHtmlElement': {
     'nativeType': 'mozilla::dom::HTMLSharedElement'
 },
 
-'HTMLMediaElement': {
-    'concrete': False
-},
-
 'HTMLOListElement': {
     'nativeType' : 'mozilla::dom::HTMLSharedListElement'
 },
 
 'HTMLParamElement': {
     'nativeType': 'mozilla::dom::HTMLSharedElement'
 },
 
@@ -400,17 +401,18 @@ DOMInterfaces = {
 'HTMLUListElement': {
     'nativeType' : 'mozilla::dom::HTMLSharedListElement'
 },
 
 'IDBCursor': {
     'implicitJSContext': [ 'delete' ],
     'binaryNames': {
         'direction': 'getDirection'
-    }
+    },
+    'concrete': True,
 },
 
 'IDBCursorWithValue': {
     'nativeType': 'mozilla::dom::IDBCursor',
 },
 
 'IDBDatabase': {
     'implicitJSContext': [ 'transaction', 'createMutableFile',
@@ -426,16 +428,17 @@ DOMInterfaces = {
     'binaryNames': {
         'mozGetAll': 'getAll',
         'mozGetAllKeys': 'getAllKeys',
     }
 },
 
 'IDBKeyRange': {
     'wrapperCache': False,
+    'concrete': True,
 },
 
 'IDBLocaleAwareKeyRange': {
     'headerFile': 'IDBKeyRange.h',
     'wrapperCache': False,
 },
 
 'IDBObjectStore': {
@@ -444,16 +447,20 @@ DOMInterfaces = {
     },
     'implicitJSContext': [ 'clear' ],
 },
 
 'IDBOpenDBRequest': {
     'headerFile': 'IDBRequest.h'
 },
 
+'IDBRequest': {
+    'concrete': True,
+},
+
 'IDBVersionChangeEvent': {
     'headerFile': 'IDBEvents.h',
 },
 
 'ImageCapture': {
     'binaryNames': { 'videoStreamTrack': 'GetVideoStreamTrack' }
 },
 
@@ -478,20 +485,16 @@ DOMInterfaces = {
     'nativeType': 'mozilla::dom::DOMIntersectionObserverEntry',
     'headerFile': 'DOMIntersectionObserver.h',
 },
 
 'KeyboardEvent': {
     'binaryNames': { 'constructor': 'ConstructorJS' },
 },
 
-'KeyEvent': {
-    'concrete': False
-},
-
 'LegacyMozTCPSocket': {
     'headerFile': 'TCPSocket.h',
     'wrapperCache': False,
 },
 
 'MatchGlob': {
     'nativeType': 'mozilla::extensions::MatchGlob',
 },
@@ -525,28 +528,16 @@ DOMInterfaces = {
 'MediaStreamList': {
     'headerFile': 'MediaStreamList.h',
 },
 
 'MediaRecorder': {
     'headerFile': 'MediaRecorder.h',
 },
 
-'MessageBroadcaster': {
-    'concrete': False
-},
-
-'MessageListenerManager': {
-    'concrete': False
-},
-
-'MessageSender': {
-    'concrete': False
-},
-
 'MimeType': {
     'headerFile' : 'nsMimeTypeArray.h',
     'nativeType': 'nsMimeType',
 },
 
 'MimeTypeArray': {
     'nativeType': 'nsMimeTypeArray',
 },
@@ -567,16 +558,17 @@ DOMInterfaces = {
 },
 
 'MozDocumentObserver': {
     'nativeType': 'mozilla::extensions::DocumentObserver',
 },
 
 'MozSharedMap': {
     'nativeType': 'mozilla::dom::ipc::SharedMap',
+    'concrete': True,
 },
 
 'MozWritableSharedMap': {
     'headerFile': 'mozilla/dom/ipc/SharedMap.h',
     'nativeType': 'mozilla::dom::ipc::WritableSharedMap',
 },
 
 'MozSharedMapChangeEvent': {
@@ -616,25 +608,25 @@ DOMInterfaces = {
 },
 
 'NetworkInformation': {
     'nativeType': 'mozilla::dom::network::Connection',
 },
 
 'Node': {
     'nativeType': 'nsINode',
-    'concrete': False,
 },
 
 'NodeIterator': {
     'wrapperCache': False,
 },
 
 'NodeList': {
     'nativeType': 'nsINodeList',
+    'concrete': True,
 },
 
 'NotificationEvent': {
     'binaryNames': {
         'notification': 'notification_'
     }
 },
 
@@ -656,16 +648,28 @@ DOMInterfaces = {
 },
 
 'PeerConnectionImpl': {
     'nativeType': 'mozilla::PeerConnectionImpl',
     'headerFile': 'PeerConnectionImpl.h',
     'wrapperCache': False
 },
 
+'PerformanceResourceTiming' : {
+    'concrete': True,
+},
+
+'PlacesBookmark' : {
+    'concrete': True,
+},
+
+'PlacesEvent' : {
+    'concrete': True,
+},
+
 'TransceiverImpl': {
     'nativeType': 'mozilla::TransceiverImpl',
     'headerFile': 'TransceiverImpl.h',
     'wrapperCache': False
 },
 
 'Plugin': {
     'headerFile' : 'nsPluginArray.h',
@@ -825,22 +829,18 @@ DOMInterfaces = {
     'headerFile': 'DOMSVGAnimatedTransformList.h'
 },
 
 'SVGAngle': {
     'nativeType': 'mozilla::dom::DOMSVGAngle',
     'headerFile': 'DOMSVGAngle.h'
 },
 
-'SVGAnimationElement': {
-    'concrete': False
-},
-
-'SVGComponentTransferFunctionElement': {
-    'concrete': False,
+'SVGElement': {
+    'concrete': True,
 },
 
 'SVGFEFuncAElement': {
     'headerFile': 'mozilla/dom/SVGComponentTransferFunctionElement.h',
 },
 
 'SVGFEFuncBElement': {
     'headerFile': 'mozilla/dom/SVGComponentTransferFunctionElement.h',
@@ -849,24 +849,16 @@ DOMInterfaces = {
 'SVGFEFuncGElement': {
     'headerFile': 'mozilla/dom/SVGComponentTransferFunctionElement.h',
 },
 
 'SVGFEFuncRElement': {
     'headerFile': 'mozilla/dom/SVGComponentTransferFunctionElement.h',
 },
 
-'SVGGraphicsElement': {
-    'concrete': False,
-},
-
-'SVGGradientElement': {
-    'concrete': False,
-},
-
 'SVGLength': {
     'nativeType': 'mozilla::dom::DOMSVGLength',
     'headerFile': 'DOMSVGLength.h'
 },
 
 'SVGLengthList': {
     'nativeType': 'mozilla::dom::DOMSVGLengthList',
     'headerFile': 'DOMSVGLengthList.h'
@@ -884,17 +876,16 @@ DOMInterfaces = {
 'SVGNumberList': {
     'nativeType': 'mozilla::dom::DOMSVGNumberList',
     'headerFile': 'DOMSVGNumberList.h'
 },
 
 'SVGPathSeg': {
     'nativeType': 'mozilla::DOMSVGPathSeg',
     'headerFile': 'DOMSVGPathSeg.h',
-    'concrete': False,
 },
 
 'SVGPathSegClosePath': {
     'nativeType': 'mozilla::DOMSVGPathSegClosePath',
     'headerFile': 'DOMSVGPathSeg.h'
 },
 
 'SVGPathSegMovetoAbs': {
@@ -1015,46 +1006,37 @@ DOMInterfaces = {
     'nativeType': 'mozilla::dom::SVGIRect'
 },
 
 'SVGStringList': {
     'nativeType': 'mozilla::DOMSVGStringList',
     'headerFile': 'DOMSVGStringList.h',
 },
 
-'SVGTextContentElement': {
-    'concrete': False
-},
-
-'SVGTextPositioningElement': {
-    'concrete': False
-},
-
 'SVGTransform': {
     'nativeType': 'mozilla::dom::DOMSVGTransform',
     'headerFile': 'DOMSVGTransform.h',
     'binaryNames': {
         "matrix": "GetMatrix"
     }
 },
 
 'SVGTransformList': {
     'nativeType': 'mozilla::DOMSVGTransformList',
     'headerFile': 'DOMSVGTransformList.h'
 },
 
 'SVGUnitTypes' : {
+    # Maybe should be a namespace.
     'concrete': False,
 },
 
 'SVGZoomAndPan' : {
-    'concrete': False,
-},
-
-'SyncMessageSender' : {
+    # Part of a kinda complicated legacy setup for putting some constants on
+    # both interfaces and this thing, which ideally should be a namespace.
     'concrete': False,
 },
 
 'TelemetryStopwatch': {
     'nativeType': 'mozilla::telemetry::Stopwatch',
 },
 
 'TestFunctions': {
@@ -1547,39 +1529,40 @@ DOMInterfaces = {
     'implicitJSContext': [
         'dump', 'global', 'reportError', 'setConsoleEventHandler',
     ],
 },
 
 'WorkerGlobalScope': {
     'headerFile': 'mozilla/dom/WorkerScope.h',
     'implicitJSContext': [ 'createImageBitmap', 'importScripts' ],
-    'concrete': False,
     # Rename a few things so we don't have both classes and methods
     # with the same name
     'binaryNames': {
         'performance': 'getPerformance',
     },
 },
 
+'Worklet': {
+    # Paint worklets just use the Worklet interface.
+    'concrete': True,
+},
+
 'XMLHttpRequest': {
     'implicitJSContext': [ 'send'],
 },
 
-'XMLHttpRequestEventTarget': {
-    'concrete': False
-},
-
 'XMLSerializer': {
     'nativeType': 'nsDOMSerializer',
     'wrapperCache': False
 },
 
 'XPathEvaluator': {
-    'wrapperCache': False
+    'wrapperCache': False,
+    'concrete': True,
 },
 
 'XPathExpression': {
     'wrapperCache': False,
 },
 
 'XSLTProcessor': {
     'nativeType': 'txMozillaXSLTProcessor',
@@ -1689,63 +1672,55 @@ DOMInterfaces = {
         'register': False,
         'wrapperCache': False
         },
 
 'IndirectlyImplementedInterface': {
         'headerFile': 'TestBindingHeader.h',
         'register': False,
         'castable': False,
-        'concrete': False
         },
 
 'OnlyForUseInConstructor' : {
         'headerFile': 'TestBindingHeader.h',
         'register': False
         },
 
 'ImplementedInterface' : {
         'headerFile': 'TestBindingHeader.h',
-        'concrete': False,
         'register': False,
         },
 
 'ImplementedInterfaceParent' : {
         'headerFile': 'TestBindingHeader.h',
-        'concrete': False,
         'register': False
         },
 
 'DiamondImplements' : {
         'headerFile': 'TestBindingHeader.h',
-        'concrete': False,
         'register': False
         },
 
 'DiamondBranch1A' : {
         'headerFile': 'TestBindingHeader.h',
-        'concrete': False,
         'register': False
         },
 
 'DiamondBranch1B' : {
         'headerFile': 'TestBindingHeader.h',
-        'concrete': False,
         'register': False
         },
 
 'DiamondBranch2A' : {
         'headerFile': 'TestBindingHeader.h',
-        'concrete': False,
         'register': False
         },
 
 'DiamondBranch2B' : {
         'headerFile': 'TestBindingHeader.h',
-        'concrete': False,
         'register': False
         },
 
 'TestIndexedGetterInterface' : {
         'headerFile': 'TestBindingHeader.h',
         'register': False
         },
 
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -396,20 +396,31 @@ class Descriptor(DescriptorProvider):
         else:
             self.jsImplParentHeader = self.jsImplParent.replace("::", "/") + ".h"
 
         self.notflattened = desc.get('notflattened', False)
         self.register = desc.get('register', True)
 
         # If we're concrete, we need to crawl our ancestor interfaces and mark
         # them as having a concrete descendant.
-        self.concrete = (not self.interface.isExternal() and
-                         not self.interface.isCallback() and
-                         not self.interface.isNamespace() and
-                         desc.get('concrete', True))
+        concreteDefault = (not self.interface.isExternal() and
+                           not self.interface.isCallback() and
+                           # Exclude interfaces that are used as the RHS of
+                           # "implements", because those would typically not be
+                           # concrete.
+                           not self.interface.isConsequential() and
+                           not self.interface.isNamespace() and
+                           # We're going to assume that leaf interfaces are
+                           # concrete; otherwise what's the point?  Also
+                           # interfaces with constructors had better be
+                           # concrete; otherwise how can you construct them?
+                           (not self.interface.hasChildInterfaces() or
+                            self.interface.ctor() is not None))
+
+        self.concrete = desc.get('concrete', concreteDefault)
         self.hasUnforgeableMembers = (self.concrete and
                                       any(MemberIsUnforgeable(m, self) for m in
                                           self.interface.members))
         self.operations = {
             'IndexedGetter': None,
             'IndexedSetter': None,
             'IndexedDeleter': None,
             'NamedGetter': None,
@@ -725,17 +736,17 @@ class Descriptor(DescriptorProvider):
         """
         assert self.supportsNamedProperties()
         namedGetter = self.operations['NamedGetter']
         return namedGetter.getExtendedAttribute("NeedsCallerType")
 
     def isMaybeCrossOriginObject(self):
         # If we're isGlobal and have cross-origin members, we're a Window, and
         # that's not a cross-origin object.  The WindowProxy is.
-        return self.hasCrossOriginMembers and not self.isGlobal()
+        return self.concrete and self.hasCrossOriginMembers and not self.isGlobal()
 
     def needsHeaderInclude(self):
         """
         An interface doesn't need a header file if it is not concrete, not
         pref-controlled, has no prototype object, has no static methods or
         attributes and has no parent.  The parent matters because we assert
         things about refcounting that depend on the actual underlying type if we
         have a parent.
--- a/layout/style/CSSFontFaceRule.cpp
+++ b/layout/style/CSSFontFaceRule.cpp
@@ -124,16 +124,18 @@ void CSSFontFaceRuleDecl::IndexedGetter(
 css::Rule* CSSFontFaceRuleDecl::GetParentRule() { return ContainingRule(); }
 
 nsINode* CSSFontFaceRuleDecl::GetParentObject() {
   return ContainingRule()->GetParentObject();
 }
 
 JSObject* CSSFontFaceRuleDecl::WrapObject(JSContext* cx,
                                           JS::Handle<JSObject*> aGivenProto) {
+  // If this changes to use a different type, remove the 'concrete'
+  // annotation from CSSStyleDeclaration.
   return CSSStyleDeclaration_Binding::Wrap(cx, this, aGivenProto);
 }
 
 // -------------------------------------------
 // CSSFontFaceRule
 //
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(CSSFontFaceRule)