Fix VM not accepting static methods for Isolate.spawn.

BUG= http://dartbug.com/3011

Committed: https://code.google.com/p/dart/source/detail?r=30300

R=iposva@google.com

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

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@30442 260f80e4-7a28-3924-810f-c04153c831b5

runtime/lib/isolate.cc

@@ -215,30 +215,22 @@ static RawObject* Spawn(NativeArguments* arguments, IsolateSpawnState* state) {
 
 DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 1) {
   GET_NON_NULL_NATIVE_ARGUMENT(Instance, closure, arguments->NativeArgAt(0));
-  bool throw_exception = false;
-  Function& func = Function::Handle();
   if (closure.IsClosure()) {
+    Function& func = Function::Handle();
     func = Closure::function(closure);
-    const Class& cls = Class::Handle(func.Owner());
-    if (!func.IsClosureFunction() || !func.is_static() || !cls.IsTopLevel()) {
-      throw_exception = true;
-    }
-  } else {
-    throw_exception = true;
-  }
-  if (throw_exception) {
-    const String& msg = String::Handle(String::New(
-        "Isolate.spawn expects to be passed a top-level function"));
-    Exceptions::ThrowArgumentError(msg);
-  }
-
+    if (func.IsImplicitClosureFunction() && func.is_static()) {
 #if defined(DEBUG)
-  Context& ctx = Context::Handle();
-  ctx = Closure::context(closure);
-  ASSERT(ctx.num_variables() == 0);
+      Context& ctx = Context::Handle();
+      ctx = Closure::context(closure);
+      ASSERT(ctx.num_variables() == 0);
 #endif
-
-  return Spawn(arguments, new IsolateSpawnState(func));
+      return Spawn(arguments, new IsolateSpawnState(func));
+    }
+  }
+  const String& msg = String::Handle(String::New(
+      "Isolate.spawn expects to be passed a static or top-level function"));
+  Exceptions::ThrowArgumentError(msg);
+  return Object::null();
 }
 
 

runtime/vm/isolate.cc

@@ -1032,17 +1032,21 @@ IsolateSpawnState::IsolateSpawnState(const Function& func)
     : isolate_(NULL),
       script_url_(NULL),
       library_url_(NULL),
+      class_name_(NULL),
       function_name_(NULL),
       exception_callback_name_(NULL) {
   script_url_ = NULL;
   const Class& cls = Class::Handle(func.Owner());
-  ASSERT(cls.IsTopLevel());
   const Library& lib = Library::Handle(cls.library());
   const String& lib_url = String::Handle(lib.url());
   library_url_ = strdup(lib_url.ToCString());
 
   const String& func_name = String::Handle(func.name());
   function_name_ = strdup(func_name.ToCString());
+  if (!cls.IsTopLevel()) {
+    const String& class_name = String::Handle(cls.Name());
+    class_name_ = strdup(class_name.ToCString());
+  }
   exception_callback_name_ = strdup("_unhandledExceptionCallback");
 }
 
@@ -1050,6 +1054,7 @@ IsolateSpawnState::IsolateSpawnState(const Function& func)
 IsolateSpawnState::IsolateSpawnState(const char* script_url)
     : isolate_(NULL),
       library_url_(NULL),
+      class_name_(NULL),
       function_name_(NULL),
       exception_callback_name_(NULL) {
   script_url_ = strdup(script_url);
@@ -1063,6 +1068,7 @@ IsolateSpawnState::~IsolateSpawnState() {
   free(script_url_);
   free(library_url_);
   free(function_name_);
+  free(class_name_);
   free(exception_callback_name_);
 }
 
@@ -1084,13 +1090,36 @@ RawObject* IsolateSpawnState::ResolveFunction() {
   ASSERT(!lib.IsNull());
 
   // Resolve the function.
-  const String& func_name =
-      String::Handle(String::New(function_name()));
-  const Function& func = Function::Handle(lib.LookupLocalFunction(func_name));
+  const String& func_name = String::Handle(String::New(function_name()));
+
+  if (class_name() == NULL) {
+    const Function& func = Function::Handle(lib.LookupLocalFunction(func_name));
+    if (func.IsNull()) {
+      const String& msg = String::Handle(String::NewFormatted(
+          "Unable to resolve function '%s' in library '%s'.",
+          function_name(),
+          (library_url() != NULL ? library_url() : script_url())));
+      return LanguageError::New(msg);
+    }
+    return func.raw();
+  }
+
+  const String& cls_name = String::Handle(String::New(class_name()));
+  const Class& cls = Class::Handle(lib.LookupLocalClass(cls_name));
+  if (cls.IsNull()) {
+    const String& msg = String::Handle(String::NewFormatted(
+          "Unable to resolve class '%s' in library '%s'.",
+          class_name(),
+          (library_url() != NULL ? library_url() : script_url())));
+    return LanguageError::New(msg);
+  }
+  const Function& func =
+      Function::Handle(cls.LookupStaticFunctionAllowPrivate(func_name));
   if (func.IsNull()) {
     const String& msg = String::Handle(String::NewFormatted(
-        "Unable to resolve function '%s' in library '%s'.",
-        function_name(), (library_url() ? library_url() : script_url())));
+          "Unable to resolve static method '%s.%s' in library '%s'.",
+          class_name(), function_name(),
+          (library_url() != NULL ? library_url() : script_url())));
     return LanguageError::New(msg);
   }
   return func.raw();

runtime/vm/isolate.h

@@ -561,6 +561,7 @@ class IsolateSpawnState {
   void set_isolate(Isolate* value) { isolate_ = value; }
   char* script_url() const { return script_url_; }
   char* library_url() const { return library_url_; }
+  char* class_name() const { return class_name_; }
   char* function_name() const { return function_name_; }
   char* exception_callback_name() const { return exception_callback_name_; }
   bool is_spawn_uri() const { return library_url_ == NULL; }
@@ -572,6 +573,7 @@ class IsolateSpawnState {
   Isolate* isolate_;
   char* script_url_;
   char* library_url_;
+  char* class_name_;
   char* function_name_;
   char* exception_callback_name_;
 };

tests/isolate/isolate.status

@@ -111,6 +111,7 @@ spawn_function_custom_class_test: Fail
 spawn_function_test: Fail
 stacktrace_message_test: Fail
 unresolved_ports_test: Fail
+static_function_test: Fail
 
 [ $compiler == dartanalyzer || $compiler == dart2analyzer ]
 browser/typed_data_message_test: StaticWarning

tests/isolate/static_function_lib.dart

@@ -0,0 +1,27 @@
+// 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 static_function_testlib;
+import "dart:isolate" show SendPort;
+
+// Used by static_function_test.dart.
+
+void function(port) { port.send("LIBTOP"); }
+void _function(port) { port.send("_LIBTOP"); }
+
+Function get privateFunction => _function;
+
+class C {
+  static void function(SendPort port) { port.send("LIB"); }
+  static void _function(SendPort port) { port.send("LIBPRIVATE"); }
+  static Function get privateFunction => _function;
+}
+
+Function get privateClassFunction => _C.function;
+Function get privateClassAndFunction => _C._function;
+
+class _C {
+  static void function(SendPort port) { port.send("_LIB"); }
+  static void _function(SendPort port) { port.send("_LIBPRIVATE"); }
+}

tests/isolate/static_function_test.dart

@@ -0,0 +1,106 @@
+// Copyright (c) 2013, 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.
+
+// Test starting isolate with static functions (and toplevel ones, for sanity).
+
+library static_function_test;
+import 'dart:isolate';
+import 'dart:async';
+import 'static_function_lib.dart' as lib;
+import '../../pkg/unittest/lib/unittest.dart';
+
+void function(SendPort port) { port.send("TOP"); }
+void _function(SendPort port) { port.send("_TOP"); }
+
+// A closure created at top-level (not inside a method), but not by a top-level
+// function declaration.
+var staticClosure = (SendPort port) { port.send("WHAT?"); };
+
+// An unnamed closure created inside a function.
+get dynamicClosure => (SendPort port) { port.send("WHAT??"); };
+
+// A named closure created inside a function.
+get namedDynamicClosure {
+  void foo(SendPort port) { port.send("WHAT FOO??"); };
+  return foo;
+}
+
+class C {
+  // Unnamed closure created during object initialization, but not inside
+  // a method or constructor.
+  final Function instanceClosure = (SendPort port) { port.send("C WHAT?"); };
+  // Unnamed closure created during object initializer list evaluation.
+  final Function constructorInitializerClosure;
+  // Unnamed closure created inside constructor body.
+  Function constructorBodyClosure;
+  // Named closure created inside constructor body.
+  Function namedConstructorBodyClosure;
+
+  C() : constructorInitializerClosure =
+            ((SendPort port) { port.send("Init?"); }) {
+    constructorBodyClosure = (SendPort port) {
+      port.send("bodyClosure?");
+    };
+    void foo(SendPort port) {
+      port.send("namedBodyClosure?");
+    }
+    namedConstructorBodyClosure = foo;
+  }
+
+  static void function(SendPort port) { port.send("YES"); }
+  static void _function(SendPort port) { port.send("PRIVATE"); }
+  void instanceMethod(SendPort port) { port.send("INSTANCE WHAT?"); }
+}
+
+class _C {
+  static void function(SendPort port) { port.send("_YES"); }
+  static void _function(SendPort port) { port.send("_PRIVATE"); }
+}
+
+void spawnTest(name, function, response) {
+  test(name, () {
+    ReceivePort r = new ReceivePort();
+      Isolate.spawn(function, r.sendPort);
+      r.listen(expectAsync1((v) {
+        expect(v, response);
+        r.close();
+      }));
+  });
+}
+
+void throwsTest(name, function) {
+  test("throws on $name", () {
+    expect(() { Isolate.spawn(function, null); }, throws);
+  });
+}
+
+void main() {
+  // Sanity check.
+  spawnTest("function", function, "TOP");
+  spawnTest("_function", _function, "_TOP");
+  spawnTest("lib.function", lib.function, "LIBTOP");
+  spawnTest("lib._function", lib.privateFunction, "_LIBTOP");
+
+  // Local static functions.
+  spawnTest("class.function", C.function, "YES");
+  spawnTest("class._function", C._function, "PRIVATE");
+  spawnTest("_class._function", _C.function, "_YES");
+  spawnTest("_class._function", _C._function, "_PRIVATE");
+
+  // Imported static functions.
+  spawnTest("lib.class.function", lib.C.function, "LIB");
+  spawnTest("lib.class._function", lib.C.privateFunction, "LIBPRIVATE");
+  spawnTest("lib._class._function", lib.privateClassFunction, "_LIB");
+  spawnTest("lib._class._function", lib.privateClassAndFunction, "_LIBPRIVATE");
+
+  // Negative tests
+  throwsTest("static closure", staticClosure);
+  throwsTest("dynamic closure", dynamicClosure);
+  throwsTest("named dynamic closure", namedDynamicClosure);
+  throwsTest("instance closure", new C().instanceClosure);
+  throwsTest("initializer closure", new C().constructorInitializerClosure);
+  throwsTest("constructor closure", new C().constructorBodyClosure);
+  throwsTest("named constructor closure", new C().namedConstructorBodyClosure);
+  throwsTest("instance method", new C().instanceMethod);
+}