frog isolate fixes: minor changes to the isolate library + architecture.py

frog/world.dart

Index: frog/world.dart
diff --git a/frog/world.dart b/frog/world.dart
index 9b6bcfe43307d8c66b52c31db3670434b3c430a5..7f4b2e44707b8ff2b4cd6695cd9f3babc6413e58 100644
--- a/frog/world.dart
+++ b/frog/world.dart
@@ -215,9 +215,11 @@ class World {
       } else if (namedPri > existingPri) {
         // New one takes priority over existing
         _renameJavascriptTopName(existing);
-      } else {
+      } else if ((!_isDomWorker(named) || !_isIsolateWorker(existing))
+          && (!_isDomWorker(existing) || !_isIsolateWorker(named))) {

[mattsh, 2011/12/22 01:55:15]

is there a way to simplify this?  (Read a few times and can't quite follow it.)

[Siggi Cherem (dart-lang), 2011/12/22 18:51:15]

How about this (not-normalized form is a bit easier to read):

if (!((_isDomWorker(named) && _isIsolateWorker(existing)
|| (_isDomWorker(existing) && _isIsolateWorker(named))) {
 ...
}

[Jennifer Messerly, 2011/12/22 19:49:44]

How about:

} else if (!named.isWebWorker && !existing.isWebWorker) {
...
}

we shouldn't ever get in the state where DOM Worker is conflicting with DOM
Worker ... so that collapses the two cases

         // Two conflicting native names or names in corelib. Libraries need
-        // to be fixed.
+        // to be fixed. We exclude the case when isolate defines Worker, so that
+        // corelib doesn't depend on the dom lib.
         world.internalError('conflicting JS name "$name" of same '
             + 'priority $existingPri: (already defined in) '
             + '${existing.span.locationText} with priority $namedPri)',
@@ -229,6 +231,16 @@ class World {
     }
   }
 
+  /** Whether element is the dom definition of Worker. */
+  _isDomWorker(Element e) {

[Jennifer Messerly, 2011/12/22 19:49:44]

personally I'd put this as a getter on Element. Even though we only use it from
world right now: "named.isWebWorker" reads better imho (or "isWorker" -- but
Worker is kind of generic, and most people are familiar with the "web worker"
term)

[Siggi Cherem (dart-lang), 2011/12/22 21:17:23]

Seems really strange to add this on Element. I don't know if this will be the
only type conflict that we will ever have though. An alternative I was
discussing with Matt was to change the if test above to be:

if (!_isConflictHarmless(named, element)) {
  ...
}

and define here:
_isConflictHarmless(e1, e2) {
  if (e1.library != dom && e1.library != corelib) return false;
  if (e2.library != dom && e2.library != corelib) return false;
  return e1.nativeName == 'Worker' && e2.nativeName == 'Worker';
}

+    return e.library == dom && e.nativeName == 'Worker';
+  }
+
+  /** Whether element is the isolate (corelib) definition of Worker. */
+  _isIsolateWorker(Element e) {
+    return e.library == coreimpl && e.nativeName == 'Worker';
+  }
+
   /** Renames an [Element] that had a name conflict in the generated JS. */
   _renameJavascriptTopName(Element named) {
     named._jsname = '${named.library.jsname}_${named.jsname}';