Special-case top-level final JS fields to make them non-lazy

(follow-up to John's suggestion in https://codereview.chromium.org/1486473002/)

BUG=
[email protected]

Review URL: https://codereview.chromium.org/1487213002 .

Author: Olivier Chafik

lib/runtime/dart/_isolate_helper.js

@@ -914,14 +914,10 @@ dart_library.library('dart/_isolate_helper', null, /* Imports */[
     constructors: () => ({_IsolateEvent: [_IsolateEvent, [_IsolateContext, core.Function, core.String]]}),
     methods: () => ({process: [dart.void, []]})
   });
-  dart.defineLazyProperties(exports, {
-    get _global() {
-      return typeof global == 'undefined' ? self : global;
-    }
-  });
+  const _global = typeof global == 'undefined' ? self : global;
   class _MainManagerStub extends core.Object {
     postMessage(msg) {
-      exports._global.postMessage(msg);
+      _global.postMessage(msg);
     }
   }
   dart.setSignature(_MainManagerStub, {
@@ -931,13 +927,13 @@ dart_library.library('dart/_isolate_helper', null, /* Imports */[
   const _SPAWN_FAILED_SIGNAL = "spawn failed";
   dart.copyProperties(exports, {
     get globalWindow() {
-      return exports._global.window;
+      return _global.window;
     },
     get globalWorker() {
-      return exports._global.Worker;
+      return _global.Worker;
     },
     get globalPostMessageDefined() {
-      return !!exports._global.postMessage;
+      return !!_global.postMessage;
     }
   });
   const _MainFunction = dart.typedef('_MainFunction', () => dart.functionType(dart.dynamic, []));

lib/runtime/dart/_js_mirrors.js

@@ -28,24 +28,18 @@ dart_library.library('dart/_js_mirrors', null, /* Imports */[
     return new JsClassMirror._(key);
   }
   dart.fn(reflectType, mirrors.TypeMirror, [core.Type]);
-  dart.defineLazyProperties(exports, {
-    get _dart() {
-      return dart;
-    },
-    get _metadata() {
-      return exports._dart.metadata;
-    }
-  });
+  const _dart = dart;
+  const _metadata = _dart.metadata;
   function _dload(obj, name) {
-    return exports._dart.dload(obj, name);
+    return _dart.dload(obj, name);
   }
   dart.fn(_dload, dart.dynamic, [dart.dynamic, core.String]);
   function _dput(obj, name, val) {
-    exports._dart.dput(obj, name, val);
+    _dart.dput(obj, name, val);
   }
   dart.fn(_dput, dart.void, [dart.dynamic, core.String, dart.dynamic]);
   function _dsend(obj, name, args) {
-    return exports._dart.dsend(obj, name, ...args);
+    return _dart.dsend(obj, name, ...args);
   }
   dart.fn(_dsend, dart.dynamic, [dart.dynamic, core.String, core.List]);
   const _toJsMap = Symbol('_toJsMap');
@@ -103,23 +97,23 @@ dart_library.library('dart/_js_mirrors', null, /* Imports */[
       [_toJsMap]: [dart.dynamic, [core.Map$(core.Symbol, dart.dynamic)]]
     })
   });
-  const _metadata = Symbol('_metadata');
+  const _metadata$ = Symbol('_metadata');
   const _declarations = Symbol('_declarations');
   const _cls = Symbol('_cls');
   class JsClassMirror extends core.Object {
     get metadata() {
-      return this[_metadata];
+      return this[_metadata$];
     }
     get declarations() {
       return this[_declarations];
     }
     _(cls) {
       this[_cls] = cls;
       this.simpleName = core.Symbol.new(cls.name);
-      this[_metadata] = null;
+      this[_metadata$] = null;
       this[_declarations] = null;
       let fn = this[_cls][dart.metadata];
-      this[_metadata] = fn == null ? dart.list([], mirrors.InstanceMirror) : core.List$(mirrors.InstanceMirror).from(dart.as(dart.dsend(dart.dcall(fn), 'map', dart.fn(i => new JsInstanceMirror._(i), JsInstanceMirror, [dart.dynamic])), core.Iterable));
+      this[_metadata$] = fn == null ? dart.list([], mirrors.InstanceMirror) : core.List$(mirrors.InstanceMirror).from(dart.as(dart.dsend(dart.dcall(fn), 'map', dart.fn(i => new JsInstanceMirror._(i), JsInstanceMirror, [dart.dynamic])), core.Iterable));
       this[_declarations] = core.Map$(core.Symbol, mirrors.MethodMirror).new();
       this[_declarations].set(this.simpleName, new JsMethodMirror._(this, this[_cls]));
     }
@@ -344,7 +338,7 @@ dart_library.library('dart/_js_mirrors', null, /* Imports */[
       this[_method] = method;
       this[_name] = getName(cls.simpleName);
       this[_params] = null;
-      let ftype = exports._dart.classGetConstructorType(cls[_cls]);
+      let ftype = _dart.classGetConstructorType(cls[_cls]);
       this[_params] = this[_createParameterMirrorList](ftype);
     }
     get constructorName() {

lib/runtime/dart/js.js

@@ -334,9 +334,9 @@ dart_library.library('dart/js', null, /* Imports */[
     } else if (dart.is(o, JsObject)) {
       return dart.dload(o, _jsObject);
     } else if (dart.is(o, core.Function)) {
-      return _putIfAbsent(exports._jsProxies, o, _wrapDartFunction);
+      return _putIfAbsent(_jsProxies, o, _wrapDartFunction);
     } else {
-      return _putIfAbsent(exports._jsProxies, o, dart.fn(o => new _DartObject(o), _DartObject, [dart.dynamic]));
+      return _putIfAbsent(_jsProxies, o, dart.fn(o => new _DartObject(o), _DartObject, [dart.dynamic]));
     }
   }
   dart.fn(_convertToJS);
@@ -345,7 +345,7 @@ dart_library.library('dart/js', null, /* Imports */[
       let args = Array.prototype.map.call(arguments, _convertToDart);
       return _convertToJS(f(...args));
     };
-    dart.dsetindex(exports._dartProxies, wrapper, f);
+    dart.dsetindex(_dartProxies, wrapper, f);
     return wrapper;
   }
   dart.fn(_wrapDartFunction);
@@ -358,7 +358,7 @@ dart_library.library('dart/js', null, /* Imports */[
     } else if (dart.is(o, _DartObject) && dart.jsobject != dart.realRuntimeType(o)) {
       return dart.dload(o, _dartObj);
     } else {
-      return _putIfAbsent(exports._dartProxies, o, _wrapToDart);
+      return _putIfAbsent(_dartProxies, o, _wrapToDart);
     }
   }
   dart.fn(_convertToDart, core.Object, [dart.dynamic]);
@@ -372,14 +372,8 @@ dart_library.library('dart/js', null, /* Imports */[
     return new JsObject._fromJs(o);
   }
   dart.fn(_wrapToDart, JsObject, [dart.dynamic]);
-  dart.defineLazyProperties(exports, {
-    get _dartProxies() {
-      return new WeakMap();
-    },
-    get _jsProxies() {
-      return new WeakMap();
-    }
-  });
+  const _dartProxies = new WeakMap();
+  const _jsProxies = new WeakMap();
   function _putIfAbsent(weakMap, o, getValue) {
     let value = weakMap.get(o);
     if (value == null) {

lib/src/codegen/js_codegen.dart

@@ -1325,23 +1325,23 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
   }
 
   bool _isInlineJSFunction(FunctionExpression functionExpression) {
-    bool isJsInvocation(Expression expr) =>
-        expr is MethodInvocation && isInlineJS(expr.methodName.staticElement);
-
     var body = functionExpression.body;
     if (body is ExpressionFunctionBody) {
-      return isJsInvocation(body.expression);
+      return _isJSInvocation(body.expression);
     } else if (body is BlockFunctionBody) {
       if (body.block.statements.length == 1) {
         var stat = body.block.statements.single;
         if (stat is ReturnStatement) {
-          return isJsInvocation(stat.expression);
+          return _isJSInvocation(stat.expression);
         }
       }
     }
     return false;
   }
 
+  bool _isJSInvocation(Expression expr) =>
+      expr is MethodInvocation && isInlineJS(expr.methodName.staticElement);
+
   // Simplify `(args) => ((x, y) => { ... })(x, y)` to `(args) => { ... }`.
   // Note: we don't check if the top-level args match the ones passed through
   // the arrow function, which allows silently passing args through to the
@@ -1753,7 +1753,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
     // Always qualify:
     // * mutable top-level fields
     // * elements from other libraries
-    bool mutableTopLevel = e is TopLevelVariableElement && !e.isConst;
+    bool mutableTopLevel = e is TopLevelVariableElement &&
+        !e.isConst &&
+        !_isFinalJSDecl(e.computeNode());
     bool fromAnotherLibrary = e.library != currentLibrary;
     if (mutableTopLevel || fromAnotherLibrary) {
       return new JS.PropertyAccess(libName, nameExpr);
@@ -2160,6 +2162,10 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
     return new JS.VariableInitialization(name, _visitInitializer(node));
   }
 
+  bool _isFinalJSDecl(AstNode field) => field is VariableDeclaration &&
+      field.isFinal &&
+      _isJSInvocation(field.initializer);
+
   /// Emits a static or top-level field.
   JS.Statement _emitStaticField(VariableDeclaration field) {
     PropertyInducingElement element = field.element;
@@ -2178,8 +2184,14 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
       eagerInit = false;
     }
 
+    // Treat `final x = JS('', '...')` as a const (non-lazy) to help compile
+    // runtime helpers.
+    var isJSTopLevel = field.isFinal && _isFinalJSDecl(field);
+    if (isJSTopLevel) eagerInit = true;
+
     var fieldName = field.name.name;
-    if (field.isConst && eagerInit && element is TopLevelVariableElement) {
+    if ((field.isConst && eagerInit && element is TopLevelVariableElement) ||
+        isJSTopLevel) {
       // constant fields don't change, so we can generate them as `let`
       // but add them to the module's exports. However, make sure we generate
       // anything they depend on first.

tool/input_sdk/private/utils.dart

@@ -10,17 +10,15 @@ import 'dart:_foreign_helper' show JS;
 /// by the Dart runtime.
 // TODO(ochafik): Rewrite some of these in Dart when possible.
 
-// TODO(ochafik): Make these final + special-case them in js_codegen to fix
-// Analyzer errors.
-const defineProperty = JS('', 'Object.defineProperty');
-const getOwnPropertyDescriptor = JS('', 'Object.getOwnPropertyDescriptor');
-const getOwnPropertyNames = JS('', 'Object.getOwnPropertyNames');
-const getOwnPropertySymbols = JS('', 'Object.getOwnPropertySymbols');
+final defineProperty = JS('', 'Object.defineProperty');
+final getOwnPropertyDescriptor = JS('', 'Object.getOwnPropertyDescriptor');
+final getOwnPropertyNames = JS('', 'Object.getOwnPropertyNames');
+final getOwnPropertySymbols = JS('', 'Object.getOwnPropertySymbols');
 
-const hasOwnProperty = JS('', 'Object.prototype.hasOwnProperty');
+final hasOwnProperty = JS('', 'Object.prototype.hasOwnProperty');
 
 // TODO(ochafik): Add ES6 class syntax support to JS intrinsics to avoid this.
-const StrongModeError = JS('', '''(function() {
+final StrongModeError = JS('', '''(function() {
   function StrongModeError(message) {
     Error.call(this);
     this.message = message;

tool/sdk_expected_errors.txt

@@ -1,12 +1,6 @@
 severe: [AnalyzerMessage] The redirected constructor '(dynamic) → JSArray<dynamic>' has incompatible parameters with '(dynamic) → JSArray<E>' (dart:_interceptors/js_array.dart, line 29, col 46)
 severe: [AnalyzerMessage] Missing concrete implementation of 'num.==' (dart:_interceptors/js_number.dart, line 12, col 7)
 severe: [AnalyzerMessage] Missing concrete implementation of 'String.==' (dart:_interceptors/js_string.dart, line 14, col 7)
-severe: [AnalyzerMessage] 'const' variables must be constant value (dart:_utils, line 15, col 24)
-severe: [AnalyzerMessage] 'const' variables must be constant value (dart:_utils, line 16, col 34)
-severe: [AnalyzerMessage] 'const' variables must be constant value (dart:_utils, line 17, col 29)
-severe: [AnalyzerMessage] 'const' variables must be constant value (dart:_utils, line 18, col 31)
-severe: [AnalyzerMessage] 'const' variables must be constant value (dart:_utils, line 20, col 24)
-severe: [AnalyzerMessage] 'const' variables must be constant value (dart:_utils, line 23, col 25)
 severe: [AnalyzerMessage] The redirected constructor '((Stream<dynamic>, bool) → StreamSubscription<dynamic>) → _StreamSubscriptionTransformer<dynamic, dynamic>' has incompatible parameters with '((Stream<S>, bool) → StreamSubscription<T>) → StreamTransformer<S, T>' (dart:async/stream.dart, line 1571, col 9)
 severe: [AnalyzerMessage] The redirected constructor '({handleData: (dynamic, EventSink<dynamic>) → void, handleError: (Object, StackTrace, EventSink<dynamic>) → void, handleDone: (EventSink<dynamic>) → void}) → _StreamHandlerTransformer<dynamic, dynamic>' has incompatible parameters with '({handleData: (S, EventSink<T>) → void, handleError: (Object, StackTrace, EventSink<T>) → void, handleDone: (EventSink<T>) → void}) → StreamTransformer<S, T>' (dart:async/stream.dart, line 1588, col 13)
 severe: [AnalyzerMessage] The type '_NoCallbackSyncStreamController' is declared with 0 type parameters, but 1 type arguments were given (dart:async/stream_controller.dart, line 84, col 17)