[Mojo] Improve JavaScript API for MojoReadMessage

Originally, I tried to make the JavaScript bindings for the core Mojo APIs a
literal translation of the C APIs, but that makes MojoReadMessage a bit awkward
because C needs to worry much more about buffer allocation than JavaScript. In
building towards using the hello_world_service, I found that it was much easier
to push the MojoReadMessage pattern from connector.cc into C++ and make the
JavaScript API a bit more idiomatic by just returning an ArrayBuffer for the
message and an Array of handles.

BUG=317398

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236999 0039d316-1c4b-4281-b951-d872f2087c98

gin/converter.cc

@@ -6,6 +6,7 @@
 
 #include "v8/include/v8.h"
 
+using v8::ArrayBuffer;
 using v8::Boolean;
 using v8::External;
 using v8::Function;
@@ -125,6 +126,19 @@ bool Converter<Handle<Object> >::FromV8(Handle<Value> val,
   return true;
 }
 
+Handle<Value> Converter<Handle<ArrayBuffer> >::ToV8(v8::Isolate* isolate,
+                                                    Handle<ArrayBuffer> val) {
+  return val.As<Value>();
+}
+
+bool Converter<Handle<ArrayBuffer> >::FromV8(Handle<Value> val,
+                                             Handle<ArrayBuffer>* out) {
+  if (!val->IsArrayBuffer())
+    return false;
+  *out = Handle<ArrayBuffer>::Cast(val);
+  return true;
+}
+
 Handle<Value> Converter<Handle<External> >::ToV8(v8::Isolate* isolate,
                                                  Handle<External> val) {
   return val.As<Value>();

gin/converter.h

@@ -87,6 +87,14 @@ struct Converter<v8::Handle<v8::Object> > {
                      v8::Handle<v8::Object>* out);
 };
 
+template<>
+struct Converter<v8::Handle<v8::ArrayBuffer> > {
+  static v8::Handle<v8::Value> ToV8(v8::Isolate* isolate,
+                                    v8::Handle<v8::ArrayBuffer> val);
+  static bool FromV8(v8::Handle<v8::Value> val,
+                     v8::Handle<v8::ArrayBuffer>* out);
+};
+
 template<>
 struct Converter<v8::Handle<v8::External> > {
   static v8::Handle<v8::Value> ToV8(v8::Isolate* isolate,

gin/modules/module_registry.cc

@@ -12,6 +12,7 @@
 #include "gin/converter.h"
 #include "gin/per_isolate_data.h"
 #include "gin/public/wrapper_info.h"
+#include "gin/try_catch.h"
 
 using v8::Context;
 using v8::External;
@@ -91,19 +92,6 @@ Handle<String> GetHiddenValueKey(Isolate* isolate) {
   return StringToSymbol(isolate, "::gin::ModuleRegistry");
 }
 
-std::string GetImplicitModuleName(const std::string& explicit_name) {
-  if (!explicit_name.empty())
-    return explicit_name;
-  std::string implicit_name;
-  Handle<StackTrace> trace = StackTrace::CurrentStackTrace(1);
-  if (!trace->GetFrameCount())
-    return implicit_name;
-  Handle<String> script_name = trace->GetFrame(0)->GetScriptName();
-  if (!script_name.IsEmpty())
-    ConvertFromV8(script_name, &implicit_name);
-  return implicit_name;
-}
-
 }  // namespace
 
 ModuleRegistry::ModuleRegistry(Isolate* isolate)
@@ -196,11 +184,17 @@ void ModuleRegistry::Load(Isolate* isolate, scoped_ptr<PendingModule> pending) {
   Handle<Function> factory;
   if (ConvertFromV8(module, &factory)) {
     Handle<Object> global = isolate->GetCurrentContext()->Global();
-    module = factory->Call(global, argc, argv.data());
-    // TODO(abarth): What should we do with exceptions?
+    {
+      gin::TryCatch try_catch;
+      module = factory->Call(global, argc, argv.data());
+      if (try_catch.HasCaught())
+        return;  // TODO(abarth): What should we do with the exception?
+    }
+    if (pending->id.empty())
+      ConvertFromV8(factory->GetScriptOrigin().ResourceName(), &pending->id);
   }
 
-  RegisterModule(isolate, GetImplicitModuleName(pending->id), module);
+  RegisterModule(isolate, pending->id, module);
 }
 
 bool ModuleRegistry::AttemptToLoad(Isolate* isolate,

mojo/public/bindings/js/DEPS

 include_rules = [
+  "+base",
   "+gin",
   "+testing",
   "+v8",

mojo/public/bindings/js/core.cc

@@ -4,6 +4,7 @@
 
 #include "mojo/public/bindings/js/core.h"
 
+#include "base/logging.h"
 #include "gin/arguments.h"
 #include "gin/array_buffer.h"
 #include "gin/converter.h"
@@ -62,22 +63,23 @@ void WaitMany(const v8::FunctionCallbackInfo<v8::Value>& info) {
 void CreateMessagePipe(const v8::FunctionCallbackInfo<v8::Value>& info) {
   gin::Arguments args(info);
 
-  mojo::ScopedMessagePipeHandle handle_0;
-  mojo::ScopedMessagePipeHandle handle_1;
-  mojo::CreateMessagePipe(&handle_0, &handle_1);
+  MojoHandle handle_0 = MOJO_HANDLE_INVALID;
+  MojoHandle handle_1 = MOJO_HANDLE_INVALID;
+  MojoResult result = MojoCreateMessagePipe(&handle_0, &handle_1);
+  CHECK(result == MOJO_RESULT_OK);
 
   gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(info.GetIsolate());
-  dictionary.Set("handle0", static_cast<mojo::Handle>(handle_0.release()));
-  dictionary.Set("handle1", static_cast<mojo::Handle>(handle_1.release()));
+  dictionary.Set("handle0", handle_0);
+  dictionary.Set("handle1", handle_1);
   args.Return(dictionary);
 }
 
 void WriteMessage(const v8::FunctionCallbackInfo<v8::Value>& info) {
   gin::Arguments args(info);
 
-  mojo::Handle handle;
+  MojoHandle handle = MOJO_HANDLE_INVALID;
   gin::ArrayBufferView buffer(args.isolate());
-  std::vector<mojo::Handle> handles;
+  std::vector<MojoHandle> handles;
   MojoWriteMessageFlags flags = MOJO_WRITE_MESSAGE_FLAG_NONE;
 
   if (!args.GetNext(&handle) ||
@@ -87,46 +89,61 @@ void WriteMessage(const v8::FunctionCallbackInfo<v8::Value>& info) {
     return args.ThrowError();
   }
 
-  args.Return(mojo::WriteMessageRaw(
-      MessagePipeHandle(handle.value()), buffer.bytes(),
-      static_cast<uint32_t>(buffer.num_bytes()),
-      handles.empty() ? NULL : reinterpret_cast<const MojoHandle*>(&handles[0]),
-      static_cast<uint32_t>(handles.size()), flags));
+  args.Return(MojoWriteMessage(handle,
+                               buffer.bytes(),
+                               static_cast<uint32_t>(buffer.num_bytes()),
+                               handles.empty() ? NULL : handles.data(),
+                               static_cast<uint32_t>(handles.size()),
+                               flags));
 }
 
 void ReadMessage(const v8::FunctionCallbackInfo<v8::Value>& info) {
   gin::Arguments args(info);
 
-  mojo::Handle handle;
-  gin::ArrayBufferView buffer(args.isolate());
-  uint32_t num_handles = 0;
+  MojoHandle handle = MOJO_HANDLE_INVALID;
   MojoReadMessageFlags flags = MOJO_READ_MESSAGE_FLAG_NONE;
 
   if (!args.GetNext(&handle) ||
-      !args.GetNext(&buffer) ||
-      !args.GetNext(&num_handles) ||
       !args.GetNext(&flags)) {
     return args.ThrowError();
   }
 
-  uint32_t num_bytes = static_cast<uint32_t>(buffer.num_bytes());
-  std::vector<mojo::Handle> handles(num_handles);
-  MojoResult result = mojo::ReadMessageRaw(
-      MessagePipeHandle(handle.value()), buffer.bytes(), &num_bytes,
-      handles.empty() ? NULL : reinterpret_cast<MojoHandle*>(&handles[0]),
-      &num_handles, flags);
-  handles.resize(num_handles);
+  uint32_t num_bytes = 0;
+  uint32_t num_handles = 0;
+  MojoResult result = MojoReadMessage(
+      handle, NULL, &num_bytes, NULL, &num_handles, flags);
+  if (result != MOJO_RESULT_RESOURCE_EXHAUSTED) {
+    gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(
+        info.GetIsolate());
+    dictionary.Set("result", result);
+    args.Return(dictionary);
+  }
+
+  v8::Handle<v8::ArrayBuffer> array_buffer = v8::ArrayBuffer::New(num_bytes);
+  std::vector<MojoHandle> handles(num_handles);
+
+  gin::ArrayBuffer buffer(args.isolate());
+  ConvertFromV8(array_buffer, &buffer);
+  CHECK(buffer.num_bytes() == num_bytes);
+
+  result = MojoReadMessage(handle,
+                           buffer.bytes(),
+                           &num_bytes,
+                           handles.empty() ? NULL : handles.data(),
+                           &num_handles,
+                           flags);
+
+  CHECK(buffer.num_bytes() == num_bytes);
+  CHECK(handles.size() == num_handles);
 
-  // TODO(abarth): We should benchmark this codepath to make sure it's ok to
-  // allocate all this memory on each read.
   gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(info.GetIsolate());
   dictionary.Set("result", result);
-  dictionary.Set("bytesRead", num_bytes);
+  dictionary.Set("buffer", array_buffer);
   dictionary.Set("handles", handles);
   args.Return(dictionary);
 }
 
-gin::WrapperInfo g_core_wrapper_info = { gin::kEmbedderNativeGin };
+gin::WrapperInfo g_wrapper_info = { gin::kEmbedderNativeGin };
 
 }  // namespace
 
@@ -135,7 +152,7 @@ const char Core::kModuleName[] = "mojo/public/bindings/js/core";
 v8::Local<v8::ObjectTemplate> Core::GetTemplate(v8::Isolate* isolate) {
   gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
   v8::Local<v8::ObjectTemplate> templ = data->GetObjectTemplate(
-      &g_core_wrapper_info);
+      &g_wrapper_info);
 
   if (templ.IsEmpty()) {
     templ = v8::ObjectTemplate::New();
@@ -211,7 +228,7 @@ v8::Local<v8::ObjectTemplate> Core::GetTemplate(v8::Isolate* isolate) {
     templ->Set(gin::StringToSymbol(isolate, "READ_MESSAGE_FLAG_MAY_DISCARD"),
                gin::ConvertToV8(isolate, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD));
 
-    data->SetObjectTemplate(&g_core_wrapper_info, templ);
+    data->SetObjectTemplate(&g_wrapper_info, templ);
   }
 
   return templ;

mojo/public/bindings/js/core_unittests.js

@@ -42,20 +42,20 @@ define([
 
     var receiverData = new Uint8Array(50);
 
-    var mesage = core.readMessage(
-      pipe.handle1, receiverData, 10,
-      core.READ_MESSAGE_FLAG_NONE)
-
-    gtest.expectEqual(mesage.result, core.RESULT_OK,
-        "mesage.result is " + mesage.result);
-    gtest.expectEqual(mesage.bytesRead, 42,
-        "mesage.bytesRead is " + mesage.bytesRead);
-    gtest.expectEqual(mesage.handles.length, 0,
-        "mesage.handles.length is " + mesage.handles.length);
-
-    for (var i = 0; i < mesage.bytesRead; ++i) {
-      gtest.expectEqual(receiverData[i], (i * i) & 0xFF,
-          "receiverData[" + i + "] is " + receiverData[i]);
+    var read = core.readMessage(
+      pipe.handle1, core.READ_MESSAGE_FLAG_NONE)
+
+    gtest.expectEqual(read.result, core.RESULT_OK,
+        "read.result is " + read.result);
+    gtest.expectEqual(read.buffer.byteLength, 42,
+        "read.buffer.byteLength is " + read.buffer.byteLength);
+    gtest.expectEqual(read.handles.length, 0,
+        "read.handles.length is " + read.handles.length);
+
+    var memory = new Uint8Array(read.buffer);
+    for (var i = 0; i < memory.length; ++i) {
+      gtest.expectEqual(memory[i], (i * i) & 0xFF,
+          "memory[" + i + "] is " + memory[i]);
     }
   }
 });

mojo/public/bindings/mojom_bindings_generator.gypi

@@ -29,9 +29,10 @@
         '<(DEPTH)/mojo/public/bindings/generators/template_expander.py',
       ],
       'outputs': [
+        '<(output_dir)/<(RULE_INPUT_ROOT).cc',
         '<(output_dir)/<(RULE_INPUT_ROOT).h',
+        '<(output_dir)/<(RULE_INPUT_ROOT).js',
         '<(output_dir)/<(RULE_INPUT_ROOT)_internal.h',
-        '<(output_dir)/<(RULE_INPUT_ROOT).cc',
       ],
       'action': [
         'python', '<@(mojom_bindings_generator)',