From 58f4b3a7a6042b0968d7391bb0df7b2dd9e88904 Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Thu, 10 Aug 2023 23:42:16 +0000 Subject: [PATCH] [dart2js] Use symbols for isolate tags All supported browsers have JavaScript Symbols so use Symbols. Avoiding string property names should fix a bug where separate programs running in separate iframes arrive at using the same property. Issue: #53154 Change-Id: I470dc47de3ad381aeab670cf62d62e53f2e72873 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319865 Reviewed-by: Sigmund Cherem Commit-Queue: Stephen Adams --- .../lib/src/js_emitter/js_emitter.dart | 1 - .../lib/src/js_emitter/native_generator.dart | 77 ------------------- .../startup_emitter/fragment_emitter.dart | 21 ++--- .../startup_emitter/model_emitter.dart | 1 + .../js_runtime/lib/interceptors.dart | 17 ++-- .../_internal/js_runtime/lib/js_helper.dart | 15 ++-- .../_internal/js_runtime/lib/js_patch.dart | 20 +++-- .../js_runtime/lib/native_helper.dart | 3 +- 8 files changed, 33 insertions(+), 122 deletions(-) delete mode 100644 pkg/compiler/lib/src/js_emitter/native_generator.dart diff --git a/pkg/compiler/lib/src/js_emitter/js_emitter.dart b/pkg/compiler/lib/src/js_emitter/js_emitter.dart index b5d4f859090..2cabf255cc9 100644 --- a/pkg/compiler/lib/src/js_emitter/js_emitter.dart +++ b/pkg/compiler/lib/src/js_emitter/js_emitter.dart @@ -11,6 +11,5 @@ export 'interceptor_stub_generator.dart'; export 'main_call_stub_generator.dart'; export 'metadata_collector.dart'; export 'native_emitter.dart'; -export 'native_generator.dart'; export 'parameter_stub_generator.dart'; export 'runtime_type_generator.dart'; diff --git a/pkg/compiler/lib/src/js_emitter/native_generator.dart b/pkg/compiler/lib/src/js_emitter/native_generator.dart deleted file mode 100644 index 4c1feee637a..00000000000 --- a/pkg/compiler/lib/src/js_emitter/native_generator.dart +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -library dart2js.js_emitter.native_generator; - -import 'package:js_runtime/synced/embedded_names.dart' as embeddedNames; - -import '../js/js.dart' as jsAst; -import '../js/js.dart' show js; -import '../js_backend/backend_usage.dart' show BackendUsage; - -class NativeGenerator { - static bool needsIsolateAffinityTagInitialization(BackendUsage backendUsage) { - return backendUsage.needToInitializeIsolateAffinityTag; - } - - /// Generates the code for isolate affinity tags. - /// - /// Independently Dart programs on the same page must not interfere and - /// this code sets up the variables needed to guarantee that behavior. - static jsAst.Statement generateIsolateAffinityTagInitialization( - BackendUsage backendUsage, - jsAst.Expression generateEmbeddedGlobalAccess(String global), - jsAst.Expression internStringFunction) { - assert(backendUsage.needToInitializeIsolateAffinityTag); - - jsAst.Expression getIsolateTagAccess = - generateEmbeddedGlobalAccess(embeddedNames.GET_ISOLATE_TAG); - jsAst.Expression isolateTagAccess = - generateEmbeddedGlobalAccess(embeddedNames.ISOLATE_TAG); - jsAst.Expression dispatchPropertyNameAccess = - generateEmbeddedGlobalAccess(embeddedNames.DISPATCH_PROPERTY_NAME); - - return js.statement(''' - !function() { - var intern = #internStringFunction; - - #getIsolateTag = function(name) { - return intern("___dart_" + name + #isolateTag); - }; - - // To ensure that different programs loaded into the same context (page) - // use distinct dispatch properties, we place an object on `Object` to - // contain the names already in use. - var tableProperty = "___dart_isolate_tags_"; - var usedProperties = Object[tableProperty] || - (Object[tableProperty] = Object.create(null)); - - var rootProperty = "_${generateIsolateTagRoot()}"; - for (var i = 0; ; i++) { - var property = intern(rootProperty + "_" + i + "_"); - if (!(property in usedProperties)) { - usedProperties[property] = 1; - #isolateTag = property; - break; - } - } - if (#initializeDispatchProperty) { - #dispatchPropertyName = #getIsolateTag("dispatch_record"); - } - }(); - ''', { - 'initializeDispatchProperty': - backendUsage.needToInitializeDispatchProperty, - 'internStringFunction': internStringFunction, - 'getIsolateTag': getIsolateTagAccess, - 'isolateTag': isolateTagAccess, - 'dispatchPropertyName': dispatchPropertyNameAccess - }); - } - - static String generateIsolateTagRoot() { - // TODO(sra): MD5 of contributing source code or URIs? - return 'ZxYxX'; - } -} diff --git a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart index 93de3899b2f..5b18cae2626 100644 --- a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart +++ b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart @@ -1900,6 +1900,11 @@ class FragmentEmitter { js.string(RECORD_TYPE_TEST_COMBINATORS_PROPERTY), recordStubs)); } + if (_closedWorld.backendUsage.needToInitializeDispatchProperty) { + globals.add(js.Property(js.string(DISPATCH_PROPERTY_NAME), + js.js('Symbol("dispatch_property")'))); + } + js.ObjectInitializer globalsObject = js.ObjectInitializer(globals, isOneLiner: false); @@ -2060,22 +2065,6 @@ class FragmentEmitter { js.Statement emitNativeSupport(Fragment fragment) { List statements = []; - // The isolate-affinity tag must only be initialized once per program. - if (fragment.isMainFragment && - NativeGenerator.needsIsolateAffinityTagInitialization( - _closedWorld.backendUsage)) { - statements.add(NativeGenerator.generateIsolateAffinityTagInitialization( - _closedWorld.backendUsage, generateEmbeddedGlobalAccess, js.js(""" - // On V8, the 'intern' function converts a string to a symbol, which - // makes property access much faster. - // TODO(sra): Use Symbol on non-IE11 browsers. - function (s) { - var o = {}; - o[s] = 1; - return Object.keys(hunkHelpers.convertToFastObject(o))[0]; - }""", []))); - } - Map interceptorsByTag = {}; Map leafTags = {}; List subclassAssignments = []; diff --git a/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart b/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart index 4f92f8ac696..d3ce8725916 100644 --- a/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart +++ b/pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart @@ -12,6 +12,7 @@ import 'package:js_runtime/synced/embedded_names.dart' DEFERRED_LIBRARY_PARTS, DEFERRED_PART_URIS, DEFERRED_PART_HASHES, + DISPATCH_PROPERTY_NAME, INITIALIZATION_EVENT_LOG, INITIALIZE_LOADED_HUNK, INTERCEPTORS_BY_TAG, diff --git a/sdk/lib/_internal/js_runtime/lib/interceptors.dart b/sdk/lib/_internal/js_runtime/lib/interceptors.dart index cd4d96c9a52..9a37bfc8ae5 100644 --- a/sdk/lib/_internal/js_runtime/lib/interceptors.dart +++ b/sdk/lib/_internal/js_runtime/lib/interceptors.dart @@ -66,17 +66,17 @@ part 'js_array.dart'; part 'js_number.dart'; part 'js_string.dart'; -final String DART_CLOSURE_PROPERTY_NAME = +final JavaScriptSymbol DART_CLOSURE_PROPERTY_NAME = getIsolateAffinityTag(r'_$dart_dartClosure'); getDispatchProperty(object) { - return JS( - '', '#[#]', object, JS_EMBEDDED_GLOBAL('String', DISPATCH_PROPERTY_NAME)); + return JS('', '#[#]', object, + JS_EMBEDDED_GLOBAL('JavaScriptSymbol', DISPATCH_PROPERTY_NAME)); } setDispatchProperty(object, value) { - defineProperty( - object, JS_EMBEDDED_GLOBAL('String', DISPATCH_PROPERTY_NAME), value); + defineProperty(object, + JS_EMBEDDED_GLOBAL('JavaScriptSymbol', DISPATCH_PROPERTY_NAME), value); } // Avoid inlining this method because inlining gives us multiple allocation @@ -191,11 +191,8 @@ getNativeInterceptor(object) { return JS_INTERCEPTOR_CONSTANT(UnknownJavaScriptObject); } -// A JS String or Symbol. -dynamic _JS_INTEROP_INTERCEPTOR_TAG = null; -get JS_INTEROP_INTERCEPTOR_TAG { - return _JS_INTEROP_INTERCEPTOR_TAG ??= getIsolateAffinityTag(r'_$dart_js'); -} +final JavaScriptSymbol JS_INTEROP_INTERCEPTOR_TAG = + getIsolateAffinityTag(r'_$dart_js'); lookupInterceptorByConstructor(constructor) { return constructor == null diff --git a/sdk/lib/_internal/js_runtime/lib/js_helper.dart b/sdk/lib/_internal/js_runtime/lib/js_helper.dart index 20a9331919f..48b038170c7 100644 --- a/sdk/lib/_internal/js_runtime/lib/js_helper.dart +++ b/sdk/lib/_internal/js_runtime/lib/js_helper.dart @@ -2808,15 +2808,12 @@ String jsonEncodeNative(String string) { return JS('String', 'JSON.stringify(#)', string); } -/// Returns a property name for placing data on JavaScript objects shared -/// between DOM isolates. This happens when multiple programs are loaded in the -/// same JavaScript context (i.e. page). The name is based on [name] but with -/// an additional part that is unique for each isolate. -/// -/// The form of the name is '___dart_$name_$id'. -String getIsolateAffinityTag(String name) { - var isolateTagGetter = JS_EMBEDDED_GLOBAL('', GET_ISOLATE_TAG); - return JS('String', '#(#)', isolateTagGetter, name); +/// Returns a property symbol for this isolate to place private data on +/// JavaScript objects where we need to avoid other isolates from seeing the +/// data. This happens when multiple programs are loaded in the same JavaScript +/// context (i.e. page). The property is based on [name]. +JavaScriptSymbol getIsolateAffinityTag(String name) { + return JS('JavaScriptSymbol', 'Symbol(#)', name); } final Map?> _loadingLibraries = {}; diff --git a/sdk/lib/_internal/js_runtime/lib/js_patch.dart b/sdk/lib/_internal/js_runtime/lib/js_patch.dart index 221a721b96b..e2155dbcdef 100644 --- a/sdk/lib/_internal/js_runtime/lib/js_patch.dart +++ b/sdk/lib/_internal/js_runtime/lib/js_patch.dart @@ -7,7 +7,7 @@ import 'dart:collection' show HashMap, ListMixin; import 'dart:typed_data' show TypedData; import 'dart:_foreign_helper' show JS, DART_CLOSURE_TO_JS; -import 'dart:_interceptors' show DART_CLOSURE_PROPERTY_NAME; +import 'dart:_interceptors' show DART_CLOSURE_PROPERTY_NAME, JavaScriptSymbol; import 'dart:_internal' show patch; import 'dart:_js_helper' show @@ -17,6 +17,10 @@ import 'dart:_js_helper' JS_FUNCTION_PROPERTY_NAME; import 'dart:_js' show isBrowserObject, convertFromBrowserObject; +// TODO(sra): Replace with an extension type when they become available. + +typedef _JavaScriptProperty = Object; // String | JavaScriptSymbol + @patch JsObject get context => _context; @@ -371,7 +375,7 @@ class JsArray /*extends JsObject with ListMixin*/ { } // property added to a Dart object referencing its JS-side DartObject proxy -final String _DART_OBJECT_PROPERTY_NAME = +final JavaScriptSymbol _DART_OBJECT_PROPERTY_NAME = getIsolateAffinityTag(r'_$dart_dartObject'); // property added to a JS object referencing its Dart-side JsObject proxy @@ -380,7 +384,7 @@ const _JS_OBJECT_PROPERTY_NAME = r'_$dart_jsObject'; @pragma('dart2js:tryInline') JsObject _castToJsObject(o) => JS('', '#', o); -bool _defineProperty(o, String name, value) { +bool _defineProperty(o, _JavaScriptProperty name, value) { try { if (_isExtensible(o) && // TODO(ahe): Calling _hasOwnProperty to work around @@ -397,13 +401,13 @@ bool _defineProperty(o, String name, value) { return false; } -bool _hasOwnProperty(o, String name) { +bool _hasOwnProperty(o, _JavaScriptProperty name) { return JS('bool', 'Object.prototype.hasOwnProperty.call(#, #)', o, name); } bool _isExtensible(o) => JS('bool', 'Object.isExtensible(#)', o); -Object? _getOwnProperty(o, String name) { +Object? _getOwnProperty(o, _JavaScriptProperty name) { if (_hasOwnProperty(o, name)) { return JS('', '#[#]', o, name); } @@ -491,8 +495,8 @@ Object _wrapToDart(o) { o, _DART_OBJECT_PROPERTY_NAME, (o) => JsObject._fromJs(o)); } -Object _getDartProxy(o, String propertyName, JsObject createProxy(o)) { - var dartProxy = _getOwnProperty(o, propertyName); +Object _getDartProxy(o, _JavaScriptProperty property, JsObject createProxy(o)) { + var dartProxy = _getOwnProperty(o, property); // Temporary fix for dartbug.com/15193 // In some cases it's possible to see a JavaScript object that // came from a different context and was previously proxied to @@ -502,7 +506,7 @@ Object _getDartProxy(o, String propertyName, JsObject createProxy(o)) { // to cache proxies from multiple JS contexts and Dart isolates. if (dartProxy == null || !_isLocalObject(o)) { dartProxy = createProxy(o); - _defineProperty(o, propertyName, dartProxy); + _defineProperty(o, property, dartProxy); } return dartProxy; } diff --git a/sdk/lib/_internal/js_runtime/lib/native_helper.dart b/sdk/lib/_internal/js_runtime/lib/native_helper.dart index b1abffc18c0..6411a7e8627 100644 --- a/sdk/lib/_internal/js_runtime/lib/native_helper.dart +++ b/sdk/lib/_internal/js_runtime/lib/native_helper.dart @@ -68,7 +68,8 @@ String toStringForNativeObject(var obj) { int hashCodeForNativeObject(object) => Primitives.objectHashCode(object); /// Sets a JavaScript property on an object. -void defineProperty(var obj, String property, var value) { +void defineProperty( + var obj, Object /*String | JavaScriptSymbol*/ property, var value) { JS( 'void', 'Object.defineProperty(#, #, '