Remove _WorkerStub hack to make benefit closing an isolate.

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

Index: dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
diff --git a/dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart b/dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
index ebbb9fd02a566663cd58f7f630b8ba1387d653ff..9c76c01739b7b0bfa6b102bf4088b6d66059b3e7 100644
--- a/dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
+++ b/dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
@@ -107,7 +107,9 @@ void startRootIsolate(entry) {
  * [_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.
  */
 
 /**
@@ -183,10 +185,10 @@ class _Manager {
   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;
@@ -195,7 +197,7 @@ class _Manager {
     _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();
@@ -390,41 +392,11 @@ class _IsolateEvent {
   }
 }
 
-/** 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"#.postMessage(#)", globalThis, 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 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";
@@ -439,6 +411,9 @@ 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.
@@ -488,9 +463,6 @@ class IsolateNatives {
 
   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
@@ -502,7 +474,7 @@ class IsolateNatives {
    * 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':
@@ -537,9 +509,8 @@ class IsolateNatives {
         _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':
@@ -675,24 +646,26 @@ class IsolateNatives {
    */
   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 }));
   }
 }
 
@@ -807,9 +780,9 @@ class _WorkerSendPort extends _BaseSendPort implements SendPort {
         _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);
         }
       }
     });