Remove _WorkerStub hack to make benefit closing an isolate.

dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart

 // Copyright (c) 2012, 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 _isolate_helper;
 
 import 'dart:async';
 import 'dart:collection' show Queue, HashMap;
 import 'dart:isolate';
 import 'dart:_js_helper' show convertDartClosureToJS,
@@ skipping 89 matching lines @@
  * "manager" - A manager contains one or more isolates, schedules their
  * execution, and performs other plumbing on their behalf.  The isolate
  * present at the creation of the manager is designated as its "root isolate".
  * A manager may, for example, be implemented on a web Worker.
  *
  * [_Manager] - State present within a manager (exactly once, as a global).
  *
  * [_ManagerStub] - A handle held within one manager that allows interaction
  * with another manager.  A target manager may be addressed by zero or more
  * [_ManagerStub]s.
- *
+ * TODO(ahe): The _ManagerStub concept is broken.  It was an attempt
+ * to create a common interface between the native Worker class and
+ * _MainManagerStub.
  */
 
 /**
  * A native object that is shared across isolates. This object is visible to all
  * isolates running under the same manager (either UI or background web worker).
  *
  * This is code that is intended to 'escape' the isolate boundaries in order to
  * implement the semantics of isolates in JavaScript. Without this we would have
  * been forced to implement more code (including the top-level event loop) in
  * JavaScript itself.
@@ skipping 55 matching lines @@
   bool get needSerialization => useWorkers;
 
   /**
    * Registry of isolates. Isolates must be registered if, and only if, receive
    * ports are alive.  Normally no open receive-ports means that the isolate is
    * dead, but DOM callbacks could resurrect it.
    */
   Map<int, _IsolateContext> isolates;
 
   /** Reference to the main [_Manager].  Null in the main [_Manager] itself. */
-  _ManagerStub mainManager;
+  _MainManagerStub mainManager;
 
-  /** Registry of active [_ManagerStub]s.  Only used in the main [_Manager]. */
-  Map<int, _ManagerStub> managers;
+  /// Registry of active Web Workers.  Only used in the main [_Manager].
+  Map<int, dynamic /* Worker */> managers;
 
   /** The entry point given by [startRootIsolate]. */
   final Function entry;
 
   _Manager(this.entry) {
     _nativeDetectEnvironment();
     topEventLoop = new _EventLoop();
     isolates = new Map<int, _IsolateContext>();
-    managers = new Map<int, _ManagerStub>();
+    managers = new Map<int, dynamic>();
     if (isWorker) {  // "if we are not the main manager ourself" is the intent.
       mainManager = new _MainManagerStub();
       _nativeInitWorkerMessageHandler();
     }
   }
 
   void _nativeDetectEnvironment() {
     bool isWindowDefined = globalWindow != null;
     bool isWorkerDefined = globalWorker != null;
 
@@ skipping 174 matching lines @@
   Function fn;
   String message;
 
   _IsolateEvent(this.isolate, this.fn, this.message);
 
   void process() {
     isolate.eval(fn);
   }
 }
 
-/** An interface for a stub used to interact with a manager. */
-abstract class _ManagerStub {
-  get id;
-  void set id(int i);
-  void set onmessage(Function f);
-  void postMessage(msg);
-  void terminate();
-}
-
 /** A stub for interacting with the main manager. */
-class _MainManagerStub implements _ManagerStub {
-  get id => 0;
-  void set id(int i) { throw new UnimplementedError(); }
-  void set onmessage(f) {
-    throw new Exception("onmessage should not be set on MainManagerStub");
+class _MainManagerStub {
+  void postMessage(msg) {
+    JS("void", r"self.postMessage(#)", msg);

[ngeoffray, 2013/05/15 08:33:10]

What's 'self'?

[ahe, 2013/05/15 09:06:30]

self is a way to refer to the global context object that works in HTML pages and
in Web Workers.  It does not work in d8 and Firefox jsshell, because that would
have been too easy.

See: http://www.w3.org/TR/workers/#the-global-scope
and: http://www.w3.org/TR/Window/#dfn-self-attribute

[ngeoffray, 2013/05/15 09:09:56]

Thanks for the explanation, could you add it to the code. And why can't you use
globalThis then?

[ahe, 2013/05/15 09:19:08]

I'd like to get rid of globalThis, and I'd like to educate people on the self
property that works well across DOM and web workers.

I have this idea about adding a JS command-line support shim, for example:

(function() {
  this.self = this;
  function Console() {
  }
  Console.prototype.log = function(message) { print(message); }
  this.console = new Console();
  this.importScripts = function(/* Array */ scripts) {
    for (var i = 0; i < scripts.length; i++) {
      load(scripts[i]);
    }
  }
})()

This way we can simplify how we check for presence of features making
hello-world much smaller.

   }
-  void postMessage(msg) {
-    JS("void", r"#.postMessage(#)", globalThis, msg);
-  }
-  void terminate() {}  // Nothing useful to do here.
-}
-
-/**
- * A stub for interacting with a manager built on a web worker. This
- * definition uses a 'hidden' type (* prefix on the native name) to
- * enforce that the type is defined dynamically only when web workers
- * are actually available.
- */
-// @Native("*Worker");
-class _WorkerStub implements _ManagerStub {
-  get id => JS("", "#.id", this);
-  void set id(i) { JS("void", "#.id = #", this, i); }
-  void set onmessage(f) { JS("void", "#.onmessage = #", this, f); }
-  void postMessage(msg) { JS("void", "#.postMessage(#)", this, msg); }
-  void terminate() { JS("void", "#.terminate()", this); }
 }
 
 const String _SPAWNED_SIGNAL = "spawned";
 
 var globalThis = IsolateNatives.computeGlobalThis();
 var globalWindow = JS('', "#.window", globalThis);
 var globalWorker = JS('', "#.Worker", globalThis);
 bool globalPostMessageDefined =
     JS('', "#.postMessage !== (void 0)", globalThis);
 
 class IsolateNatives {
 
   static String thisScript = computeThisScript();
 
+  /// Associates an ID with a native worker object.
+  static final Expando<int> workerIds = new Expando<int>();
+
   /**
    * The src url for the script tag that loaded this code. Used to create
    * JavaScript workers.
    */
   static String computeThisScript() {
     var currentScript = JS('', r'$.$currentScript');
     if (currentScript != null) {
       return JS('String', 'String(#.src)', currentScript);
     }
 
@@ skipping 29 matching lines @@
     pattern = JS('', r'new RegExp("^[^@]*@(.*):[0-9]*$", "m")');
 
     matches = JS('=List|Null', '#.match(#)', stack, pattern);
     if (matches != null) return JS('String', '#[1]', matches);
 
     throw new UnsupportedError('Cannot extract URI from "$stack"');
   }
 
   static computeGlobalThis() => JS('', 'function() { return this; }()');
 
-  /** Starts a new worker with the given URL. */
-  static _WorkerStub _newWorker(url) => JS("_WorkerStub", r"new Worker(#)", url);
-
   /**
    * Assume that [e] is a browser message event and extract its message data.
    * We don't import the dom explicitly so, when workers are disabled, this
    * library can also run on top of nodejs.
    */
   static _getEventData(e) => JS("", "#.data", e);
 
   /**
    * Process messages on a worker, either to control the worker instance or to
    * pass messages along to the isolate running in the worker.
    */
-  static void _processWorkerMessage(sender, e) {
+  static void _processWorkerMessage(/* Worker */ sender, e) {
     var msg = _deserializeMessage(_getEventData(e));
     switch (msg['command']) {
       case 'start':
         _globalState.currentManagerId = msg['id'];
         String functionName = msg['functionName'];
         Function entryPoint = (functionName == null)
             ? _globalState.entry
             : _getJSFunctionFromName(functionName);
         var replyTo = _deserializeMessage(msg['replyTo']);
         var context = new _IsolateContext();
@@ skipping 14 matching lines @@
         break;
       case 'message':
         SendPort port = msg['port'];
         // If the port has been closed, we ignore the message.
         if (port != null) {
           msg['port'].send(msg['msg'], msg['replyTo']);
         }
         _globalState.topEventLoop.run();
         break;
       case 'close':
-        _log("Closing Worker");
-        _globalState.managers.remove(sender.id);
-        sender.terminate();
+        _globalState.managers.remove(workerIds[sender]);
+        JS('void', '#.terminate()', sender);
         _globalState.topEventLoop.run();
         break;
       case 'log':
         _log(msg['msg']);
         break;
       case 'print':
         if (_globalState.isWorker) {
           _globalState.mainManager.postMessage(
               _serializeMessage({'command': 'print', 'msg': msg}));
         } else {
@@ skipping 115 matching lines @@
     replyTo.send(_SPAWNED_SIGNAL, port.toSendPort());
     topLevel();
   }
 
   /**
    * Spawns an isolate in a worker. [factoryName] is the Javascript constructor
    * name for the isolate entry point class.
    */
   static void _spawnWorker(functionName, uri, replyPort) {
     if (uri == null) uri = thisScript;
-    final worker = _newWorker(uri);
-    worker.onmessage = JS('',
-                          'function(e) { #(#, e); }',
-                          DART_CLOSURE_TO_JS(_processWorkerMessage),
-                          worker);
+    final worker = JS('var', 'new Worker(#)', uri);
+
+    var processWorkerMessageTrampoline =
+      JS('', 'function(e) { #(#, e); }',
+         DART_CLOSURE_TO_JS(_processWorkerMessage),
+         worker);
+    JS('void', '#.onmessage = #', worker, processWorkerMessageTrampoline);
     var workerId = _globalState.nextManagerId++;
     // We also store the id on the worker itself so that we can unregister it.

[ngeoffray, 2013/05/15 08:33:10]

Update comment?

[ahe, 2013/05/15 09:06:30]

I actually think the comment still applies, I'm just using an expando.  What you
you say?

[ngeoffray, 2013/05/15 09:09:56]

Fine by me.

-    worker.id = workerId;
+    workerIds[worker] = workerId;
     _globalState.managers[workerId] = worker;
-    worker.postMessage(_serializeMessage({
-      'command': 'start',
-      'id': workerId,
-      // Note: we serialize replyPort twice because the child worker needs to
-      // first deserialize the worker id, before it can correctly deserialize
-      // the port (port deserialization is sensitive to what is the current
-      // workerId).
-      'replyTo': _serializeMessage(replyPort),
-      'functionName': functionName }));
+    JS('void', '#.postMessage(#)', worker, _serializeMessage({
+        'command': 'start',
+        'id': workerId,
+        // Note: we serialize replyPort twice because the child worker needs to
+        // first deserialize the worker id, before it can correctly deserialize
+        // the port (port deserialization is sensitive to what is the current
+        // workerId).
+        'replyTo': _serializeMessage(replyPort),
+        'functionName': functionName }));
   }
 }
 
 /********************************************************
   Inserted from lib/isolate/dart2js/ports.dart
  ********************************************************/
 
 /** Common functionality to all send ports. */
 class _BaseSendPort implements SendPort {
   /** Id for the destination isolate. */
@@ skipping 94 matching lines @@
           'port': this,
           'msg': message,
           'replyTo': replyTo});
 
       if (_globalState.isWorker) {
         // Communication from one worker to another go through the
         // main worker.
         _globalState.mainManager.postMessage(workerMessage);
       } else {
         // Deliver the message only if the worker is still alive.
-        _ManagerStub manager = _globalState.managers[_workerId];
+        /* Worker */ var manager = _globalState.managers[_workerId];
         if (manager != null) {
-          manager.postMessage(workerMessage);
+          JS('void', '#.postMessage(#)', manager, workerMessage);
         }
       }
     });
   }
 
   bool operator ==(var other) {
     return (other is _WorkerSendPort) &&
         (_workerId == other._workerId) &&
         (_isolateId == other._isolateId) &&
         (_receivePortId == other._receivePortId);
@@ skipping 608 matching lines @@
         JS('void', '#.clearInterval(#)', globalThis, _handle);
       }
       _handle = null;
     } else {
       throw new UnsupportedError("Canceling a timer.");
     }
   }
 }
 
 bool hasTimer() => JS('', '#.setTimeout', globalThis) != null;