From b42103c9d1287d740f775325df3caa3b36359651 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Wed, 13 Dec 2023 01:59:37 +0000 Subject: [PATCH] Remove some SVG types from bindings map Our maps are string-based and assume there's only one one @Native class with a given name. This isn't true for these four classes, where there are two types with the same name for each one. This then results in us dropping the definition of one of the two types. Since one of the two types is essentially unused, we should choose to drop that one instead for now. Long-term, we should deprecate the unused types or change our conformance handling to handle multiple types with the same name. We could also choose to unify both definitions, but this may lead to either false positives or renames of members being dropped. This allows us to catch uses of the types that are actually used. Change-Id: I9386cad5b014bee60cd72d21cdf0546640138704 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341331 Reviewed-by: Sigmund Cherem Commit-Queue: Srujan Gaddam --- tools/dom/scripts/go.sh | 2 +- .../scripts/web_library_bindings_emitter.dart | 22 +++++++- tools/dom/web_library_bindings.dart | 56 +++++++++---------- 3 files changed, 48 insertions(+), 32 deletions(-) diff --git a/tools/dom/scripts/go.sh b/tools/dom/scripts/go.sh index 03b801c2894..ce30428e79d 100755 --- a/tools/dom/scripts/go.sh +++ b/tools/dom/scripts/go.sh @@ -45,5 +45,5 @@ cd ./tools/dom/scripts # Calculate, emit, and format the bindings. BINDINGS="../web_library_bindings.dart" -dart ./web_library_bindings_emitter.dart $BINDINGS +dart --enable-asserts ./web_library_bindings_emitter.dart $BINDINGS dart format $BINDINGS diff --git a/tools/dom/scripts/web_library_bindings_emitter.dart b/tools/dom/scripts/web_library_bindings_emitter.dart index 7d516a94736..afa32787b85 100644 --- a/tools/dom/scripts/web_library_bindings_emitter.dart +++ b/tools/dom/scripts/web_library_bindings_emitter.dart @@ -39,6 +39,13 @@ void main(List args) { 'dart:web_gl' }; + const Set duplicateClassNames = { + 'ImageElement', + 'ScriptElement', + 'StyleElement', + 'TitleElement' + }; + for (var library in component.libraries) { if (webLibraries.contains(library.importUri.toString())) { for (var cls in library.classes) { @@ -46,9 +53,20 @@ void main(List args) { // All strings in the maps are annotated with quotes, so that we print // proper Dart code when we print the maps. var clsName = "'${cls.name}'"; - var nativeTypes = getNativeNames(cls); - nativeTypes = nativeTypes.map((name) => "'$name'").toList(); + var nativeTypes = getNativeNames(cls).map((name) => "'$name'").toList(); if (nativeTypes.isEmpty) nativeTypes = [clsName]; + // There are a couple of cases where there are two classes with the same + // name. They are all element classes bound to an `HTML` and an `SVG` + // version. For now, ignore the `SVG` version, as they're unused in + // google3 and most of them are marked unstable, and their `HTML` + // variants are much more common. + // TODO(srujzs): Remove this if we decide to deprecate these classes. + if (duplicateClassNames.contains(cls.name)) { + if (nativeTypes.length == 1 && nativeTypes[0] == "'SVG${cls.name}'") { + continue; + } + } + assert(!dartTypeToNativeTypes.containsKey(clsName)); dartTypeToNativeTypes[clsName] = SplayTreeSet.from(nativeTypes); var nativePropToDartProp = SplayTreeMap>(); diff --git a/tools/dom/web_library_bindings.dart b/tools/dom/web_library_bindings.dart index 0e10f62d5e1..ca7ccd7d6ce 100644 --- a/tools/dom/web_library_bindings.dart +++ b/tools/dom/web_library_bindings.dart @@ -3486,15 +3486,6 @@ final Map>> nativeTypeToDartMembers = { 'systemLanguage': {'systemLanguage'}, 'transform': {'transform'} }, - 'SVGImageElement': { - 'async': {'async'}, - 'height': {'height'}, - 'href': {'href'}, - 'preserveAspectRatio': {'preserveAspectRatio'}, - 'width': {'width'}, - 'x': {'x'}, - 'y': {'y'} - }, 'SVGLength': { 'convertToSpecifiedUnits': {'convertToSpecifiedUnits'}, 'newValueSpecifiedUnits': {'newValueSpecifiedUnits'}, @@ -3678,10 +3669,6 @@ final Map>> nativeTypeToDartMembers = { 'y': {'y'}, 'zoomAndPan': {'zoomAndPan'} }, - 'SVGScriptElement': { - 'href': {'href'}, - 'type': {'type'} - }, 'SVGStopElement': { 'offset': {'gradientOffset'} }, @@ -3695,12 +3682,6 @@ final Map>> nativeTypeToDartMembers = { 'removeItem': {'removeItem'}, 'replaceItem': {'replaceItem'} }, - 'SVGStyleElement': { - 'disabled': {'disabled'}, - 'media': {'media'}, - 'sheet': {'sheet'}, - 'type': {'type'} - }, 'SVGSymbolElement': { 'preserveAspectRatio': {'preserveAspectRatio'}, 'viewBox': {'viewBox'} @@ -6537,13 +6518,21 @@ final Map> dartTypeToNativeMembers = { 'ImageCapture': {'track': 'track'}, 'ImageData': {'data': 'data', 'height': 'height', 'width': 'width'}, 'ImageElement': { + 'alt': 'alt', 'async': 'async', + 'complete': 'complete', + 'crossOrigin': 'crossOrigin', + 'currentSrc': 'currentSrc', 'height': 'height', - 'href': 'href', - 'preserveAspectRatio': 'preserveAspectRatio', - 'width': 'width', - 'x': 'x', - 'y': 'y' + 'isMap': 'isMap', + 'naturalHeight': 'naturalHeight', + 'naturalWidth': 'naturalWidth', + 'referrerPolicy': 'referrerPolicy', + 'sizes': 'sizes', + 'src': 'src', + 'srcset': 'srcset', + 'useMap': 'useMap', + 'width': 'width' }, 'Index': { 'getAll': 'getAll', @@ -8143,7 +8132,16 @@ final Map> dartTypeToNativeMembers = { 'width': 'width' }, 'ScreenOrientation': {'angle': 'angle', 'type': 'type', 'unlock': 'unlock'}, - 'ScriptElement': {'href': 'href', 'type': 'type'}, + 'ScriptElement': { + 'async': 'async', + 'charset': 'charset', + 'crossOrigin': 'crossOrigin', + 'defer': 'defer', + 'integrity': 'integrity', + 'noModule': 'noModule', + 'src': 'src', + 'type': 'type' + }, 'ScriptProcessorNode': { 'bufferSize': 'bufferSize', 'setEventListener': 'setEventListener' @@ -9465,7 +9463,7 @@ final Map> dartTypeToNativeTypes = { 'ImageButtonInputElement': {'ImageButtonInputElement'}, 'ImageCapture': {'ImageCapture'}, 'ImageData': {'ImageData'}, - 'ImageElement': {'SVGImageElement'}, + 'ImageElement': {'HTMLImageElement'}, 'ImmutableListMixin': {'ImmutableListMixin'}, 'Index': {'IDBIndex'}, 'InputDeviceCapabilities': {'InputDeviceCapabilities'}, @@ -9737,7 +9735,7 @@ final Map> dartTypeToNativeTypes = { 'Sampler': {'WebGLSampler'}, 'Screen': {'Screen'}, 'ScreenOrientation': {'ScreenOrientation'}, - 'ScriptElement': {'SVGScriptElement'}, + 'ScriptElement': {'HTMLScriptElement'}, 'ScriptProcessorNode': {'JavaScriptAudioNode', 'ScriptProcessorNode'}, 'ScrollAlignment': {'ScrollAlignment'}, 'ScrollState': {'ScrollState'}, @@ -9783,7 +9781,7 @@ final Map> dartTypeToNativeTypes = { 'StorageEvent': {'StorageEvent'}, 'StorageManager': {'StorageManager'}, 'StringList': {'SVGStringList'}, - 'StyleElement': {'SVGStyleElement'}, + 'StyleElement': {'HTMLStyleElement'}, 'StyleMedia': {'StyleMedia'}, 'StylePropertyMap': {'StylePropertyMap'}, 'StylePropertyMapReadonly': {'StylePropertyMapReadonly'}, @@ -9831,7 +9829,7 @@ final Map> dartTypeToNativeTypes = { 'TimeInputElement': {'TimeInputElement'}, 'TimeRanges': {'TimeRanges'}, 'TimerQueryExt': {'WebGLTimerQueryEXT'}, - 'TitleElement': {'SVGTitleElement'}, + 'TitleElement': {'HTMLTitleElement'}, 'Touch': {'Touch'}, 'TouchEvent': {'TouchEvent'}, 'TouchList': {'TouchList'},