Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit d172051

Browse files
srujzsCommit Bot
authored andcommitted
[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 <[email protected]> Commit-Queue: Srujan Gaddam <[email protected]>
1 parent 47eff41 commit d172051

File tree

8 files changed

+203
-8
lines changed

8 files changed

+203
-8
lines changed

pkg/compiler/lib/src/js_emitter/native_emitter.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ class NativeEmitter {
9494

9595
Class objectClass = null;
9696
Class jsInterceptorClass = null;
97+
Class jsJavaScriptObjectClass = null;
9798

9899
void walk(Class cls) {
99100
if (cls.element == _commonElements.objectClass) {
@@ -104,6 +105,11 @@ class NativeEmitter {
104105
jsInterceptorClass = cls;
105106
return;
106107
}
108+
// Native classes may inherit either `Interceptor` e.g. `JSBool` or
109+
// `JavaScriptObject` e.g. `dart:html` classes.
110+
if (cls.element == _commonElements.jsJavaScriptObjectClass) {
111+
jsJavaScriptObjectClass = cls;
112+
}
107113
if (seen.contains(cls)) return;
108114
seen.add(cls);
109115
walk(cls.superclass);
@@ -215,6 +221,9 @@ class NativeEmitter {
215221
// by getNativeInterceptor and custom elements.
216222
if (_nativeCodegenEnqueuer.hasInstantiatedNativeClasses) {
217223
fillNativeInfo(jsInterceptorClass);
224+
if (jsJavaScriptObjectClass != null) {
225+
fillNativeInfo(jsJavaScriptObjectClass);
226+
}
218227
for (Class cls in classes) {
219228
if (!cls.isNative || neededClasses.contains(cls)) {
220229
fillNativeInfo(cls);

pkg/compiler/test/jsinterop/internal_annotations_test.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,15 @@ $mainSource
184184
"Expected $name to be indirectly instantiated in `${mainSource}`:"
185185
"\n${world.classHierarchy.dump(cls)}");
186186
}
187-
if (!isInstantiated && (name != 'Object' && name != 'Interceptor')) {
187+
// Classes that are expected to be instantiated by default. `Object` and
188+
// `Interceptor` are base types for non-native and native types, and
189+
// `JavaScriptObject` is the base type for `dart:html` types.
190+
var insantiatedBaseClasses = [
191+
'Object',
192+
'Interceptor',
193+
'JavaScriptObject'
194+
];
195+
if (!isInstantiated && !insantiatedBaseClasses.contains(name)) {
188196
Expect.isFalse(
189197
world.classHierarchy.isInstantiated(cls),
190198
"Expected $name to be uninstantiated in `${mainSource}`:"

pkg/compiler/test/jsinterop/world_test.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,15 @@ $mainSource
178178
"Expected $name to be indirectly instantiated in `${mainSource}`:"
179179
"\n${world.classHierarchy.dump(cls)}");
180180
}
181-
if (!isInstantiated && (name != 'Object' && name != 'Interceptor')) {
181+
// Classes that are expected to be instantiated by default. `Object` and
182+
// `Interceptor` are base types for non-native and native types, and
183+
// `JavaScriptObject` is the base type for `dart:html` types.
184+
var insantiatedBaseClasses = [
185+
'Object',
186+
'Interceptor',
187+
'JavaScriptObject'
188+
];
189+
if (!isInstantiated && !insantiatedBaseClasses.contains(name)) {
182190
Expect.isFalse(
183191
world.classHierarchy.isInstantiated(cls),
184192
"Expected $name to be uninstantiated in `${mainSource}`:"

sdk/lib/_internal/js_runtime/lib/js_helper.dart

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -415,19 +415,21 @@ class Primitives {
415415

416416
var interceptor = getInterceptor(object);
417417
if (identical(interceptor, JS_INTERCEPTOR_CONSTANT(Interceptor)) ||
418+
identical(interceptor, JS_INTERCEPTOR_CONSTANT(JavaScriptObject)) ||
418419
object is UnknownJavaScriptObject) {
419420
// Try to do better. If we do not find something better, fallthrough to
420-
// Dart-type based name that leave the name as 'UnknownJavaScriptObject'
421-
// or 'Interceptor' (or the minified versions thereof).
421+
// Dart-type based name that leave the name as 'UnknownJavaScriptObject',
422+
// 'Interceptor', or 'JavaScriptObject' (or their minified versions).
422423
//
423424
// When we get here via the UnknownJavaScriptObject test (for JavaScript
424425
// objects from outside the program), the object's constructor has a
425426
// better name that 'UnknownJavaScriptObject'.
426427
//
427-
// When we get here the Interceptor test (for Native classes that are
428-
// declared in the Dart program but have been 'folded' into Interceptor),
429-
// the native class's constructor name is better than the generic
430-
// 'Interceptor' (an abstract class).
428+
// When we get here via either the Interceptor or JavaScriptObject test
429+
// (for Native classes that are declared in the Dart program but have been
430+
// 'folded' into one of those interceptors), the native class's
431+
// constructor name is better than the generic 'Interceptor' or
432+
// 'JavaScriptObject'.
431433

432434
// Try the [constructorNameFallback]. This gets the constructor name for
433435
// any browser (used by [getNativeInterceptor]).
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Make sure `Object` methods work as expected with `dart:html` and interop
6+
// types. The expectations here aren't guarantees that they should work a
7+
// particular way, but rather a way to monitor regressions/changes.
8+
9+
@JS()
10+
library object_members_test;
11+
12+
import 'package:js/js.dart';
13+
import 'package:expect/minitest.dart';
14+
15+
import 'dart:html';
16+
import 'dart:_interceptors' show JSObject;
17+
18+
@JS()
19+
external void eval(String code);
20+
21+
@JS()
22+
class JSClass {
23+
external JSClass();
24+
}
25+
26+
void main() {
27+
eval(r'''
28+
function JSClass() {}
29+
''');
30+
31+
// `dart:html` type.
32+
var div = document.createElement('div');
33+
expect(div == div, true);
34+
expect(div == DomPointReadOnly(), false);
35+
// Ensure that we get a random hash for each new instance. It should be
36+
// improbable for this to fail across many runs if the hash is
37+
// non-deterministic.
38+
var hashCode = div.hashCode;
39+
var attempts = 0;
40+
var maxAttempts = 1000;
41+
while (div.hashCode == hashCode && attempts < maxAttempts) {
42+
div = document.createElement('div');
43+
attempts++;
44+
}
45+
expect(attempts > 0 && attempts != maxAttempts, isTrue);
46+
expect(div.toString, isNotNull);
47+
expect(div.toString(), 'div');
48+
expect(div.noSuchMethod, isNotNull);
49+
var noSuchMethodErrorThrown = true;
50+
try {
51+
(div as dynamic).triggerNoSuchMethod();
52+
noSuchMethodErrorThrown = false;
53+
} catch (_) {}
54+
expect(noSuchMethodErrorThrown, isTrue);
55+
expect(div.runtimeType, DivElement);
56+
57+
// `toString` for `dart:html` types that do not have an overridden `toString`
58+
// should look up the type through the proto.
59+
expect(window.navigator.toString(), "Instance of 'Navigator'");
60+
61+
// Interop type.
62+
var js = JSClass();
63+
expect(js == js, true);
64+
expect(js == JSClass(), false);
65+
// TODO(srujzs): Modify this once interop has random hash codes.
66+
hashCode = js.hashCode;
67+
expect(hashCode, 0);
68+
expect(hashCode, js.hashCode);
69+
expect(js.toString, isNotNull);
70+
// Should forward to underlying `toString` call.
71+
expect(js.toString(), '[object Object]');
72+
expect(js.noSuchMethod, isNotNull);
73+
noSuchMethodErrorThrown = true;
74+
try {
75+
(js as dynamic).triggerNoSuchMethod();
76+
noSuchMethodErrorThrown = false;
77+
} catch (_) {}
78+
expect(noSuchMethodErrorThrown, isTrue);
79+
expect(js.runtimeType, JSObject);
80+
}

tests/web/web.status

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,16 @@ deferred_split_test: Slow, Pass # Issue 25940
2727
[ $compiler == dart2js && $runtime == chrome && $csp ]
2828
deferred/load_in_correct_order_test: SkipByDesign # Purposely uses `eval`
2929

30+
[ $compiler == dart2js && $runtime == d8 ]
31+
internal/object_members_test: SkipByDesign # Browser test
32+
3033
[ $compiler == dart2js && $runtime == ff && $system == windows ]
3134
consistent_index_error_string_test: Slow, Pass # Issue 25940
3235

3336
[ $compiler == dart2js && $csp ]
3437
deferred_custom_loader_test: SkipByDesign # Issue 25683
3538
deferred_fail_and_retry_test: SkipByDesign # Uses eval to simulate failed loading.
39+
internal/object_members_test: SkipByDesign # Uses eval for interop
3640

3741
[ $compiler == dart2js && !$host_checked ]
3842
dummy_compiler_test: Slow, Pass # Issue 32439. self-hosting doesn't work with CFE yet.
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Make sure `Object` methods work as expected with `dart:html` and interop
6+
// types. The expectations here aren't guarantees that they should work a
7+
// particular way, but rather a way to monitor regressions/changes.
8+
9+
@JS()
10+
library object_members_test;
11+
12+
import 'package:js/js.dart';
13+
import 'package:expect/minitest.dart';
14+
15+
import 'dart:html';
16+
import 'dart:_interceptors' show JSObject;
17+
18+
@JS()
19+
external void eval(String code);
20+
21+
@JS()
22+
class JSClass {
23+
external JSClass();
24+
}
25+
26+
void main() {
27+
eval(r'''
28+
function JSClass() {}
29+
''');
30+
31+
// `dart:html` type.
32+
var div = document.createElement('div');
33+
expect(div == div, true);
34+
expect(div == DomPointReadOnly(), false);
35+
// Ensure that we get a random hash for each new instance. It should be
36+
// improbable for this to fail across many runs if the hash is
37+
// non-deterministic.
38+
var hashCode = div.hashCode;
39+
var attempts = 0;
40+
var maxAttempts = 1000;
41+
while (div.hashCode == hashCode && attempts < maxAttempts) {
42+
div = document.createElement('div');
43+
attempts++;
44+
}
45+
expect(attempts > 0 && attempts != maxAttempts, isTrue);
46+
expect(div.toString, isNotNull);
47+
expect(div.toString(), 'div');
48+
expect(div.noSuchMethod, isNotNull);
49+
var noSuchMethodErrorThrown = true;
50+
try {
51+
(div as dynamic).triggerNoSuchMethod();
52+
noSuchMethodErrorThrown = false;
53+
} catch (_) {}
54+
expect(noSuchMethodErrorThrown, isTrue);
55+
expect(div.runtimeType, DivElement);
56+
57+
// `toString` for `dart:html` types that do not have an overridden `toString`
58+
// should look up the type through the proto.
59+
expect(window.navigator.toString(), "Instance of 'Navigator'");
60+
61+
// Interop type.
62+
var js = JSClass();
63+
expect(js == js, true);
64+
expect(js == JSClass(), false);
65+
// TODO(srujzs): Modify this once interop has random hash codes.
66+
hashCode = js.hashCode;
67+
expect(hashCode, 0);
68+
expect(hashCode, js.hashCode);
69+
expect(js.toString, isNotNull);
70+
// Should forward to underlying `toString` call.
71+
expect(js.toString(), '[object Object]');
72+
expect(js.noSuchMethod, isNotNull);
73+
noSuchMethodErrorThrown = true;
74+
try {
75+
(js as dynamic).triggerNoSuchMethod();
76+
noSuchMethodErrorThrown = false;
77+
} catch (_) {}
78+
expect(noSuchMethodErrorThrown, isTrue);
79+
expect(js.runtimeType, JSObject);
80+
}

tests/web_2/web_2.status

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,16 @@ deferred_split_test: Slow, Pass # Issue 25940
2727
[ $compiler == dart2js && $runtime == chrome && $csp ]
2828
deferred/load_in_correct_order_test: SkipByDesign # Purposely uses `eval`
2929

30+
[ $compiler == dart2js && $runtime == d8 ]
31+
internal/object_members_test: SkipByDesign # Browser test
32+
3033
[ $compiler == dart2js && $runtime == ff && $system == windows ]
3134
consistent_index_error_string_test: Slow, Pass # Issue 25940
3235

3336
[ $compiler == dart2js && $csp ]
3437
deferred_custom_loader_test: SkipByDesign # Issue 25683
3538
deferred_fail_and_retry_test: SkipByDesign # Uses eval to simulate failed loading.
39+
internal/object_members_test: SkipByDesign # Uses eval for interop
3640

3741
[ $compiler == dart2js && !$host_checked ]
3842
dummy_compiler_test: Slow, Pass # Issue 32439. self-hosting doesn't work with CFE yet.

0 commit comments

Comments
 (0)