[dart2js] Handle `Object` members of `dart:html` types
`dart:html` types have changed to inherit `JavaScriptObject`. Therefore,
the dart2js runtime needs to be changed so that interceptors are still
created for those types, and they're properly handled in `toString`
calculations. Includes tests on `Object` members that are currently
inconsistent between both compilers.
This test passes on dart2js with and without making the types in `dart:html`
extend `JavaScriptObject`. This test fails in DDC for the following reasons:
- `toString` of native types calls the native `toString`
- `hashCode` for interop objects are random and not 0
- `runtimeType` of interop objects is `LegacyJavaScriptObject` not `JSObject`
Change-Id: Ibf80109174615120df9e64995fa13016f7a1677b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/228741
Reviewed-by: Stephen Adams <sra@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
diff --git a/pkg/compiler/lib/src/js_emitter/native_emitter.dart b/pkg/compiler/lib/src/js_emitter/native_emitter.dart
index a3137e7..857b67b 100644
--- a/pkg/compiler/lib/src/js_emitter/native_emitter.dart
+++ b/pkg/compiler/lib/src/js_emitter/native_emitter.dart
@@ -94,6 +94,7 @@
Class objectClass = null;
Class jsInterceptorClass = null;
+ Class jsJavaScriptObjectClass = null;
void walk(Class cls) {
if (cls.element == _commonElements.objectClass) {
@@ -104,6 +105,11 @@
jsInterceptorClass = cls;
return;
}
+ // Native classes may inherit either `Interceptor` e.g. `JSBool` or
+ // `JavaScriptObject` e.g. `dart:html` classes.
+ if (cls.element == _commonElements.jsJavaScriptObjectClass) {
+ jsJavaScriptObjectClass = cls;
+ }
if (seen.contains(cls)) return;
seen.add(cls);
walk(cls.superclass);
@@ -215,6 +221,9 @@
// by getNativeInterceptor and custom elements.
if (_nativeCodegenEnqueuer.hasInstantiatedNativeClasses) {
fillNativeInfo(jsInterceptorClass);
+ if (jsJavaScriptObjectClass != null) {
+ fillNativeInfo(jsJavaScriptObjectClass);
+ }
for (Class cls in classes) {
if (!cls.isNative || neededClasses.contains(cls)) {
fillNativeInfo(cls);
diff --git a/pkg/compiler/test/jsinterop/internal_annotations_test.dart b/pkg/compiler/test/jsinterop/internal_annotations_test.dart
index 4299a38..62deced 100644
--- a/pkg/compiler/test/jsinterop/internal_annotations_test.dart
+++ b/pkg/compiler/test/jsinterop/internal_annotations_test.dart
@@ -184,7 +184,15 @@
"Expected $name to be indirectly instantiated in `${mainSource}`:"
"\n${world.classHierarchy.dump(cls)}");
}
- if (!isInstantiated && (name != 'Object' && name != 'Interceptor')) {
+ // Classes that are expected to be instantiated by default. `Object` and
+ // `Interceptor` are base types for non-native and native types, and
+ // `JavaScriptObject` is the base type for `dart:html` types.
+ var insantiatedBaseClasses = [
+ 'Object',
+ 'Interceptor',
+ 'JavaScriptObject'
+ ];
+ if (!isInstantiated && !insantiatedBaseClasses.contains(name)) {
Expect.isFalse(
world.classHierarchy.isInstantiated(cls),
"Expected $name to be uninstantiated in `${mainSource}`:"
diff --git a/pkg/compiler/test/jsinterop/world_test.dart b/pkg/compiler/test/jsinterop/world_test.dart
index 6c3253f..230873f 100644
--- a/pkg/compiler/test/jsinterop/world_test.dart
+++ b/pkg/compiler/test/jsinterop/world_test.dart
@@ -178,7 +178,15 @@
"Expected $name to be indirectly instantiated in `${mainSource}`:"
"\n${world.classHierarchy.dump(cls)}");
}
- if (!isInstantiated && (name != 'Object' && name != 'Interceptor')) {
+ // Classes that are expected to be instantiated by default. `Object` and
+ // `Interceptor` are base types for non-native and native types, and
+ // `JavaScriptObject` is the base type for `dart:html` types.
+ var insantiatedBaseClasses = [
+ 'Object',
+ 'Interceptor',
+ 'JavaScriptObject'
+ ];
+ if (!isInstantiated && !insantiatedBaseClasses.contains(name)) {
Expect.isFalse(
world.classHierarchy.isInstantiated(cls),
"Expected $name to be uninstantiated in `${mainSource}`:"
diff --git a/sdk/lib/_internal/js_runtime/lib/js_helper.dart b/sdk/lib/_internal/js_runtime/lib/js_helper.dart
index a7f3ca8..d41e511 100644
--- a/sdk/lib/_internal/js_runtime/lib/js_helper.dart
+++ b/sdk/lib/_internal/js_runtime/lib/js_helper.dart
@@ -415,19 +415,21 @@
var interceptor = getInterceptor(object);
if (identical(interceptor, JS_INTERCEPTOR_CONSTANT(Interceptor)) ||
+ identical(interceptor, JS_INTERCEPTOR_CONSTANT(JavaScriptObject)) ||
object is UnknownJavaScriptObject) {
// Try to do better. If we do not find something better, fallthrough to
- // Dart-type based name that leave the name as 'UnknownJavaScriptObject'
- // or 'Interceptor' (or the minified versions thereof).
+ // Dart-type based name that leave the name as 'UnknownJavaScriptObject',
+ // 'Interceptor', or 'JavaScriptObject' (or their minified versions).
//
// When we get here via the UnknownJavaScriptObject test (for JavaScript
// objects from outside the program), the object's constructor has a
// better name that 'UnknownJavaScriptObject'.
//
- // When we get here the Interceptor test (for Native classes that are
- // declared in the Dart program but have been 'folded' into Interceptor),
- // the native class's constructor name is better than the generic
- // 'Interceptor' (an abstract class).
+ // When we get here via either the Interceptor or JavaScriptObject test
+ // (for Native classes that are declared in the Dart program but have been
+ // 'folded' into one of those interceptors), the native class's
+ // constructor name is better than the generic 'Interceptor' or
+ // 'JavaScriptObject'.
// Try the [constructorNameFallback]. This gets the constructor name for
// any browser (used by [getNativeInterceptor]).
diff --git a/tests/web/dart2js.status b/tests/web/dart2js.status
index 9faa92a..826b784 100644
--- a/tests/web/dart2js.status
+++ b/tests/web/dart2js.status
@@ -24,12 +24,16 @@
[ $compiler == dart2js && $runtime == chrome && $csp ]
deferred/load_in_correct_order_test: SkipByDesign # Purposely uses `eval`
+[ $compiler == dart2js && $runtime == d8 ]
+internal/object_members_test: SkipByDesign # Browser test
+
[ $compiler == dart2js && $runtime == ff && $system == windows ]
consistent_index_error_string_test: Slow, Pass # Issue 25940
[ $compiler == dart2js && $csp ]
deferred_custom_loader_test: SkipByDesign # Issue 25683
deferred_fail_and_retry_test: SkipByDesign # Uses eval to simulate failed loading.
+internal/object_members_test: SkipByDesign # Uses eval for interop
[ $compiler == dart2js && !$host_checked ]
dummy_compiler_test: Slow, Pass # Issue 32439. self-hosting doesn't work with CFE yet.
diff --git a/tests/web/internal/object_members_test.dart b/tests/web/internal/object_members_test.dart
new file mode 100644
index 0000000..02092eb
--- /dev/null
+++ b/tests/web/internal/object_members_test.dart
@@ -0,0 +1,80 @@
+// Copyright (c) 2022, 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.
+
+// Make sure `Object` methods work as expected with `dart:html` and interop
+// types. The expectations here aren't guarantees that they should work a
+// particular way, but rather a way to monitor regressions/changes.
+
+@JS()
+library object_members_test;
+
+import 'package:js/js.dart';
+import 'package:expect/minitest.dart';
+
+import 'dart:html';
+import 'dart:_interceptors' show JSObject;
+
+@JS()
+external void eval(String code);
+
+@JS()
+class JSClass {
+ external JSClass();
+}
+
+void main() {
+ eval(r'''
+ function JSClass() {}
+ ''');
+
+ // `dart:html` type.
+ var div = document.createElement('div');
+ expect(div == div, true);
+ expect(div == DomPointReadOnly(), false);
+ // Ensure that we get a random hash for each new instance. It should be
+ // improbable for this to fail across many runs if the hash is
+ // non-deterministic.
+ var hashCode = div.hashCode;
+ var attempts = 0;
+ var maxAttempts = 1000;
+ while (div.hashCode == hashCode && attempts < maxAttempts) {
+ div = document.createElement('div');
+ attempts++;
+ }
+ expect(attempts > 0 && attempts != maxAttempts, isTrue);
+ expect(div.toString, isNotNull);
+ expect(div.toString(), 'div');
+ expect(div.noSuchMethod, isNotNull);
+ var noSuchMethodErrorThrown = true;
+ try {
+ (div as dynamic).triggerNoSuchMethod();
+ noSuchMethodErrorThrown = false;
+ } catch (_) {}
+ expect(noSuchMethodErrorThrown, isTrue);
+ expect(div.runtimeType, DivElement);
+
+ // `toString` for `dart:html` types that do not have an overridden `toString`
+ // should look up the type through the proto.
+ expect(window.navigator.toString(), "Instance of 'Navigator'");
+
+ // Interop type.
+ var js = JSClass();
+ expect(js == js, true);
+ expect(js == JSClass(), false);
+ // TODO(srujzs): Modify this once interop has random hash codes.
+ hashCode = js.hashCode;
+ expect(hashCode, 0);
+ expect(hashCode, js.hashCode);
+ expect(js.toString, isNotNull);
+ // Should forward to underlying `toString` call.
+ expect(js.toString(), '[object Object]');
+ expect(js.noSuchMethod, isNotNull);
+ noSuchMethodErrorThrown = true;
+ try {
+ (js as dynamic).triggerNoSuchMethod();
+ noSuchMethodErrorThrown = false;
+ } catch (_) {}
+ expect(noSuchMethodErrorThrown, isTrue);
+ expect(js.runtimeType, JSObject);
+}
diff --git a/tests/web_2/dart2js_2.status b/tests/web_2/dart2js_2.status
index 9faa92a..826b784 100644
--- a/tests/web_2/dart2js_2.status
+++ b/tests/web_2/dart2js_2.status
@@ -24,12 +24,16 @@
[ $compiler == dart2js && $runtime == chrome && $csp ]
deferred/load_in_correct_order_test: SkipByDesign # Purposely uses `eval`
+[ $compiler == dart2js && $runtime == d8 ]
+internal/object_members_test: SkipByDesign # Browser test
+
[ $compiler == dart2js && $runtime == ff && $system == windows ]
consistent_index_error_string_test: Slow, Pass # Issue 25940
[ $compiler == dart2js && $csp ]
deferred_custom_loader_test: SkipByDesign # Issue 25683
deferred_fail_and_retry_test: SkipByDesign # Uses eval to simulate failed loading.
+internal/object_members_test: SkipByDesign # Uses eval for interop
[ $compiler == dart2js && !$host_checked ]
dummy_compiler_test: Slow, Pass # Issue 32439. self-hosting doesn't work with CFE yet.
diff --git a/tests/web_2/internal/object_members_test.dart b/tests/web_2/internal/object_members_test.dart
new file mode 100644
index 0000000..02092eb
--- /dev/null
+++ b/tests/web_2/internal/object_members_test.dart
@@ -0,0 +1,80 @@
+// Copyright (c) 2022, 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.
+
+// Make sure `Object` methods work as expected with `dart:html` and interop
+// types. The expectations here aren't guarantees that they should work a
+// particular way, but rather a way to monitor regressions/changes.
+
+@JS()
+library object_members_test;
+
+import 'package:js/js.dart';
+import 'package:expect/minitest.dart';
+
+import 'dart:html';
+import 'dart:_interceptors' show JSObject;
+
+@JS()
+external void eval(String code);
+
+@JS()
+class JSClass {
+ external JSClass();
+}
+
+void main() {
+ eval(r'''
+ function JSClass() {}
+ ''');
+
+ // `dart:html` type.
+ var div = document.createElement('div');
+ expect(div == div, true);
+ expect(div == DomPointReadOnly(), false);
+ // Ensure that we get a random hash for each new instance. It should be
+ // improbable for this to fail across many runs if the hash is
+ // non-deterministic.
+ var hashCode = div.hashCode;
+ var attempts = 0;
+ var maxAttempts = 1000;
+ while (div.hashCode == hashCode && attempts < maxAttempts) {
+ div = document.createElement('div');
+ attempts++;
+ }
+ expect(attempts > 0 && attempts != maxAttempts, isTrue);
+ expect(div.toString, isNotNull);
+ expect(div.toString(), 'div');
+ expect(div.noSuchMethod, isNotNull);
+ var noSuchMethodErrorThrown = true;
+ try {
+ (div as dynamic).triggerNoSuchMethod();
+ noSuchMethodErrorThrown = false;
+ } catch (_) {}
+ expect(noSuchMethodErrorThrown, isTrue);
+ expect(div.runtimeType, DivElement);
+
+ // `toString` for `dart:html` types that do not have an overridden `toString`
+ // should look up the type through the proto.
+ expect(window.navigator.toString(), "Instance of 'Navigator'");
+
+ // Interop type.
+ var js = JSClass();
+ expect(js == js, true);
+ expect(js == JSClass(), false);
+ // TODO(srujzs): Modify this once interop has random hash codes.
+ hashCode = js.hashCode;
+ expect(hashCode, 0);
+ expect(hashCode, js.hashCode);
+ expect(js.toString, isNotNull);
+ // Should forward to underlying `toString` call.
+ expect(js.toString(), '[object Object]');
+ expect(js.noSuchMethod, isNotNull);
+ noSuchMethodErrorThrown = true;
+ try {
+ (js as dynamic).triggerNoSuchMethod();
+ noSuchMethodErrorThrown = false;
+ } catch (_) {}
+ expect(noSuchMethodErrorThrown, isTrue);
+ expect(js.runtimeType, JSObject);
+}
