Decode the Dart message into a Dart_CMessage structure before calling the native port callback

runtime/vm/native_message_handler.cc

Index: runtime/vm/native_message_handler.cc
diff --git a/runtime/vm/native_message_handler.cc b/runtime/vm/native_message_handler.cc
index ebc7121c84895ddd193c3ed90128fdbca84e8eb9..6b3f143346526800ccc2b5cd90d91dc124db3df8 100644
--- a/runtime/vm/native_message_handler.cc
+++ b/runtime/vm/native_message_handler.cc
@@ -6,6 +6,7 @@
 
 #include "vm/isolate.h"
 #include "vm/message.h"
+#include "vm/snapshot.h"
 #include "vm/thread.h"
 
 namespace dart {
@@ -47,12 +48,18 @@ static void RunWorker(uword parameter) {
         // dispatched to special vm code.  Implement.
         UNIMPLEMENTED();
       }
-      // TODO(sgjesse): Once CMessageReader::ReadObject is committed,
-      // use that here and pass the resulting data object to the
-      // handler instead.
+      // Create a new zone for allocating all the structures for the
+      // decoded message.
+      Zone zone;

[siva, 2012/02/04 01:55:43]

Instead of modifying zone to accept a version without an Isolate why not use
ApiZone here. This is native code and should theoratically work with just the
dart_api stuff.

[Søren Gjesse, 2012/02/06 16:25:52]

The problem with using an ApiZone here is that the passed-in zone is also used
for the allocation of the GrowableArray used for backwards references by the
CMessageReader.

It makes sense to use the ApiZone here as this callback is part of the API.
However I would then have to modify the GrowableArray to take an allocator
function instead of a zone - which might be the right thing to do. I will try to
see how that looks.

[siva, 2012/02/07 01:03:24]

You could make BaseGrowableArray use BaseZone* as the field in it instead of
Zone* and create a ApiGrowableArray structure in dart_api_state.h which passes
in the appropriate BaseZone from ApiZone got from the thread local storage.

If you think ApiGrowableArray won't be used a lot in the native code you could
pull the CMessageReader and CMessageWriter structure into this file and make
ApiGrowableArray a private class here.


On 2012/02/06 16:25:52, Søren Gjesse wrote:

+      int32_t length = reinterpret_cast<int32_t*>(
+          message->data())[Snapshot::kLengthIndex];
+      CMessageReader message_reader(message->data() + Snapshot::kHeaderSize,
+                                    length,
+                                    &zone);

[siva, 2012/02/04 01:55:43]

Do you really want a version of the reader with a Zone parameter, why not have
an allocator function which would use the Zone similar to the one in
ApiAllocator (dart_api_impl.cc) and it can access the zone wither from a static
variable if we have only one handler thread or use a thread local variable for
storing zone.

[Søren Gjesse, 2012/02/06 16:25:52]

We do need a thread local if we are not passing in a zone. I have prepared CL
http://codereview.chromium.org/9328042/ to make thread locals platform
independent. I will then add a thread local key for the NativeMessageHandler and
pass in an allocator which uses the thread local for finding the zone.

[siva, 2012/02/07 01:03:24]

I have reviewed that change, it seems fine to abstract out the thread local
stuff into platform as you might at some time want to use thread local data in
your thread pool implementation also.

On 2012/02/06 16:25:52, Søren Gjesse wrote:

+      Dart_CMessage* cmessage = message_reader.ReadMessage();
       (*handler->func())(message->dest_port(),
                          message->reply_port(),
-                         message->data());
+                         cmessage);
       delete message;
     }
   }