- Refactor the isolate callback mechanism to also include creation of the

include/dart_api.h

Index: include/dart_api.h
===================================================================
--- include/dart_api.h	(revision 1771)
+++ include/dart_api.h	(working copy)
@@ -226,25 +226,31 @@
 // --- Initialization and Globals ---
 
 /**
- * An isolate initialization callback function.
+ * A type for passing error messages back when creation and initialization
+ * of isolate fails.
+ */
+typedef struct {
+  char* buffer;
+  int length;
+} Dart_IsolateError;

[turnidge, 2011/11/23 18:05:12]

We are always using snprintf to fill this error.  We could provide a helper
function to help with this.  Something like:

  Dart_PrintIsolateError(Dart_IsolateError* error, format, ...)

It could use OS::VSNPrint.

If we had this, we might be able to hide the internal structure from the user if
we want to.  We could also considering allocating on demand when the error is
generated rather than pre-allocating, if that works out with the zones.

If we don't hide the internals, it would be good to comment about how/where the
buffer is allocated.  The buffer is allocated before the callback is called. 
Length is an incoming parameter indicating the buffer length, etc.  (Otherwise,
they might think that they should allocate the buffer themselves.)

[siva, 2011/11/23 22:37:27]

As discussed offline we will resolve the snprintf stuff in another CL if needed.
I have added documentation about the buffer.

On 2011/11/23 18:05:12, turnidge wrote:

+
+/**
+ * An isolate create and initialization callback function.

[turnidge, 2011/11/23 18:05:12]

s/create/creation/

[siva, 2011/11/23 22:37:27]

Done.

  *
- * This callback, provided by the embedder, is called during isolate
- * creation. It is called for all isolates, regardless of whether they
- * are created via Dart_CreateIsolate or directly from Dart code.
+ * This callback, provided by the embedder, is called when an isolate needs
+ * to be created. The callback should create an isolate and load up the
+ * required scripts for execution.
  *
- * \param data Embedder-specific data used during isolate initialization.
+ * \param error A structure into which the embedder can place a
+ *   C string containing an error message in the case of failures.
  *
- * \return If the embedder returns NULL, then the isolate being
- *   initialized will be shut down without executing any Dart code.
- *   Otherwise, the embedder should return a pointer to
- *   embedder-specific data created during the initialization of this
- *   isolate. This data will, in turn, be passed by the VM to all
- *   isolates spawned from the isolate currently being initialized.
+ * \return the embedder returns false if the creation and initialization was not
+ *   successful and true if successful. The embedder is responsible for
+ *   maintaining consistency in the case of errors (e.g: isolate is created,
+ *   but loading of scripts fails then the embedder should ensure that the
+ *   isolate is deleted).

[turnidge, 2011/11/23 18:05:12]

Instead of saying "deleted" maybe frame this in terms of Dart_ShutdownIsolate.

[siva, 2011/11/23 22:37:27]

Done.

  */
-typedef void* (*Dart_IsolateInitCallback)(void* embedder_data);
-// TODO(iposva): Pass a specification of the app file being spawned.
-// TODO(turnidge): We don't actually shut down the isolate on NULL yet.
-// TODO(turnidge): Should we separate the two return values?
+typedef bool (*Dart_IsolateCreateAndInitCallback)(Dart_IsolateError error);

[turnidge, 2011/11/23 18:05:12]

Dart_IsolateCreateAndInitCallback is a bit long.  Do you think we could get away
with calling this Dart_IsolateCreateCallback?

[siva, 2011/11/23 22:37:27]

Done.

 
 /**
  * Initializes the VM with the given commmand line flags.
@@ -252,12 +258,12 @@
  * \param argc The length of the arguments array.
  * \param argv An array of arguments.
  * \param callback A function to be called during isolate creation.
- *   See Dart_IsolateInitCallback.
+ *   See Dart_IsolateCreateAndInitCallback.
  *
  * \return True if initialization is successful.
  */
 DART_EXPORT bool Dart_Initialize(int argc, const char** argv,
-                                 Dart_IsolateInitCallback callback);
+                                 Dart_IsolateCreateAndInitCallback callback);
 
 /**
  * Returns true if the named VM flag is set.
@@ -296,13 +302,11 @@
  *
  * \param snapshot A buffer containing a VM snapshot or NULL if no
  *   snapshot is provided.
- * \param data Embedder-specific data. See Dart_IsolateInitCallback.
  *
  * \return The new isolate is returned. May be NULL if an error
  *   occurs duing isolate initialization.
  */
-DART_EXPORT Dart_Isolate Dart_CreateIsolate(const Dart_Snapshot* snapshot,
-                                            void* data);
+DART_EXPORT Dart_Isolate Dart_CreateIsolate(const Dart_Snapshot* snapshot);

[turnidge, 2011/11/23 18:05:12]

Consider taking a Dart_IsolateError parameter.  I guess we would need to do
allocation of the error string differently in that case.

[siva, 2011/11/23 22:37:27]

Done.

 // TODO(turnidge): Document behavior when there is already a current
 // isolate.