Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(67)

Unified Diff: pkg/compiler/lib/src/native/behavior.dart

Issue 1457383003: Alternative fix for the js-interop crash. (Closed) Base URL: git@github.com:dart-lang/sdk.git@master
Patch Set: Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « pkg/compiler/lib/src/js_backend/js_interop_analysis.dart ('k') | pkg/compiler/lib/src/ssa/builder.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: pkg/compiler/lib/src/native/behavior.dart
diff --git a/pkg/compiler/lib/src/native/behavior.dart b/pkg/compiler/lib/src/native/behavior.dart
index 7dbab30fe5e6f1e7f29b4a3b0715474e6abca661..5a03ca015c54f41294b23cef3691e78d089dcba4 100644
--- a/pkg/compiler/lib/src/native/behavior.dart
+++ b/pkg/compiler/lib/src/native/behavior.dart
@@ -645,12 +645,26 @@ class NativeBehavior {
static NativeBehavior ofMethod(FunctionElement method, Compiler compiler) {
FunctionType type = method.computeType(compiler.resolution);
var behavior = new NativeBehavior();
- behavior.typesReturned.add(type.returnType);
+ var returnType = type.returnType;
+ bool isInterop = compiler.backend.isJsInterop(method);
+ // Note: For dart:html and other internal libraries we maintain, we can
+ // trust the return type and use it to limit what we enqueue. We have to
+ // be more conservative about JS interop types and assume they can return
+ // anything (unless the user provides the experimental flag to trust the
+ // type of js-interop APIs). We do restrict the allocation effects and say
+ // that interop calls create only interop types (which may be unsound if
+ // an interop call returns a DOM type and declares a dynamic return type,
+ // but otherwise we would include a lot of code by default).
+ // TODO(sigmund,sra): consider doing something better for numeric types.
+ behavior.typesReturned.add(
+ !isInterop || compiler.trustJSInteropTypeAnnotations ? returnType
+ : const DynamicType());
if (!type.returnType.isVoid) {
// Declared types are nullable.
behavior.typesReturned.add(compiler.coreTypes.nullType);
}
- behavior._capture(type, compiler.resolution);
+ behavior._capture(type, compiler.resolution,
+ isInterop: isInterop, compiler: compiler);
// TODO(sra): Optional arguments are currently missing from the
// DartType. This should be fixed so the following work-around can be
@@ -668,10 +682,15 @@ class NativeBehavior {
Resolution resolution = compiler.resolution;
DartType type = field.computeType(resolution);
var behavior = new NativeBehavior();
- behavior.typesReturned.add(type);
+ bool isInterop = compiler.backend.isJsInterop(field);
+ // TODO(sigmund,sra): consider doing something better for numeric types.
+ behavior.typesReturned.add(
+ !isInterop || compiler.trustJSInteropTypeAnnotations ? type
+ : const DynamicType());
// Declared types are nullable.
behavior.typesReturned.add(resolution.coreTypes.nullType);
- behavior._capture(type, resolution);
+ behavior._capture(type, resolution,
+ isInterop: isInterop, compiler: compiler);
behavior._overrideWithAnnotations(field, compiler);
return behavior;
}
@@ -765,17 +784,49 @@ class NativeBehavior {
/// Models the behavior of Dart code receiving instances and methods of [type]
/// from native code. We usually start the analysis by capturing a native
/// method that has been used.
- void _capture(DartType type, Resolution resolution) {
+ ///
+ /// We assume that JS-interop APIs cannot instantiate Dart types or
+ /// non-JSInterop native types.
+ void _capture(DartType type, Resolution resolution,
+ {bool isInterop: false, Compiler compiler}) {
type.computeUnaliased(resolution);
type = type.unaliased;
if (type is FunctionType) {
FunctionType functionType = type;
- _capture(functionType.returnType, resolution);
+ _capture(functionType.returnType, resolution,
+ isInterop: isInterop, compiler: compiler);
for (DartType parameter in functionType.parameterTypes) {
_escape(parameter, resolution);
}
} else {
- typesInstantiated.add(type);
+ DartType instantiated = null;
+ JavaScriptBackend backend = compiler?.backend;
+ if (!isInterop) {
+ typesInstantiated.add(type);
+ } else {
+ if (type.element != null && backend.isNative(type.element)) {
+ // Any declared native or interop type (isNative implies isJsInterop)
+ // is assumed to be allocated.
+ typesInstantiated.add(type);
+ }
+
+ if (!compiler.trustJSInteropTypeAnnotations ||
+ type.isDynamic || type.isObject) {
+ // By saying that only JS-interop types can be created, we prevent
+ // pulling in every other native type (e.g. all of dart:html) when a
+ // JS interop API returns dynamic or when we don't trust the type
+ // annotations. This means that to some degree we still use the return
+ // type to decide whether to include native types, even if we don't
+ // trust the type annotation.
+ typesInstantiated.add(
+ backend.helpers.jsJavaScriptObjectClass.thisType);
+ } else {
+ // Otherwise, when the declared type is a Dart type, we do not
+ // register an allocation because we assume it cannot be instantiated
+ // from within the JS-interop code. It must have escaped from another
+ // API.
+ }
+ }
}
}
« no previous file with comments | « pkg/compiler/lib/src/js_backend/js_interop_analysis.dart ('k') | pkg/compiler/lib/src/ssa/builder.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698