Change mojo JS bindings to expose a handle object, which is Closed when garbage

mojo/bindings/js/core.cc

Index: mojo/bindings/js/core.cc
diff --git a/mojo/bindings/js/core.cc b/mojo/bindings/js/core.cc
index 3aaf6298e8ffa715b83fffbb897d122569899037..cdd9455ee944ca359d8bd8e7d33475a9c4304745 100644
--- a/mojo/bindings/js/core.cc
+++ b/mojo/bindings/js/core.cc
@@ -11,9 +11,11 @@
 #include "gin/converter.h"
 #include "gin/dictionary.h"
 #include "gin/function_template.h"
+#include "gin/handle.h"
 #include "gin/object_template_builder.h"
 #include "gin/per_isolate_data.h"
 #include "gin/public/wrapper_info.h"
+#include "gin/wrappable.h"
 #include "mojo/bindings/js/handle.h"
 
 namespace mojo {
@@ -21,6 +23,46 @@ namespace js {
 
 namespace {
 
+class HandleWrapper : public gin::Wrappable<HandleWrapper> {
+ public:
+  static gin::WrapperInfo kWrapperInfo;
+
+  static gin::Handle<HandleWrapper> Create(v8::Isolate* isolate,
+                                           MojoHandle handle) {
+    return CreateHandle(isolate, new HandleWrapper(handle));
+  }
+
+  MojoHandle get() const { return handle_.get().value(); }
+  void Close() { handle_.reset(); }
+
+ protected:
+  HandleWrapper(MojoHandle handle) : handle_(mojo::Handle(handle)) {}
+  virtual ~HandleWrapper() {}
+  ScopedHandle handle_;
+};
+
+gin::WrapperInfo HandleWrapper::kWrapperInfo = { gin::kEmbedderNativeGin };
+
+MojoResult CloseHandle(gin::Handle<HandleWrapper> handle) {
+  handle->Close();
+  return MOJO_RESULT_OK;
+}
+
+MojoResult WaitHandle(gin::Handle<HandleWrapper> handle,
+                      MojoWaitFlags flags,
+                      MojoDeadline deadline) {
+  return MojoWait(handle->get(), flags, deadline);
+}
+
+MojoResult WaitMany(const std::vector<gin::Handle<HandleWrapper> >& handles,
+                    const std::vector<MojoWaitFlags>& flags,
+                    MojoDeadline deadline) {
+  std::vector<mojo::Handle> raw_handles;
+  for (size_t i = 0; i < handles.size(); ++i)
+    raw_handles[i].set_value(handles[i]->get());
+  return mojo::WaitMany(raw_handles, flags, deadline);
+}
+
 gin::Dictionary CreateMessagePipe(const gin::Arguments& args) {
   MojoHandle handle0 = MOJO_HANDLE_INVALID;
   MojoHandle handle1 = MOJO_HANDLE_INVALID;
@@ -28,30 +70,33 @@ gin::Dictionary CreateMessagePipe(const gin::Arguments& args) {
   CHECK(result == MOJO_RESULT_OK);
 
   gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(args.isolate());
-  dictionary.Set("handle0", handle0);
-  dictionary.Set("handle1", handle1);
+  dictionary.Set("handle0", HandleWrapper::Create(args.isolate(), handle0));
+  dictionary.Set("handle1", HandleWrapper::Create(args.isolate(), handle1));
   return dictionary;
 }
 
-MojoResult WriteMessage(MojoHandle handle,
+MojoResult WriteMessage(gin::Handle<HandleWrapper> handle,
                         const gin::ArrayBufferView& buffer,
-                        const std::vector<MojoHandle>& handles,
+                        const std::vector<gin::Handle<HandleWrapper> >& handles,
                         MojoWriteMessageFlags flags) {
-  return MojoWriteMessage(handle,
+  std::vector<MojoHandle> raw_handles;

[abarth-chromium, 2014/03/29 01:56:17]

Do you want to resize the vector to the final size so that there's only one
malloc?

[Matt Perry, 2014/03/31 19:34:17]

Done.

+  for (size_t i = 0; i < handles.size(); ++i)
+    raw_handles[i] = handles[i]->get();
+  return MojoWriteMessage(handle->get(),
                           buffer.bytes(),
                           static_cast<uint32_t>(buffer.num_bytes()),
-                          handles.empty() ? NULL : &handles[0],
+                          raw_handles.empty() ? NULL : &raw_handles[0],
                           static_cast<uint32_t>(handles.size()),
                           flags);
 }
 
 gin::Dictionary ReadMessage(const gin::Arguments& args,
-                            MojoHandle handle,
+                            gin::Handle<HandleWrapper> handle,
                             MojoReadMessageFlags flags) {
   uint32_t num_bytes = 0;
   uint32_t num_handles = 0;
   MojoResult result = MojoReadMessage(
-      handle, NULL, &num_bytes, NULL, &num_handles, flags);
+      handle->get(), NULL, &num_bytes, NULL, &num_handles, flags);
   if (result != MOJO_RESULT_RESOURCE_EXHAUSTED) {
     gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(args.isolate());
     dictionary.Set("result", result);
@@ -60,25 +105,29 @@ gin::Dictionary ReadMessage(const gin::Arguments& args,
 
   v8::Handle<v8::ArrayBuffer> array_buffer =
       v8::ArrayBuffer::New(args.isolate(), num_bytes);
-  std::vector<MojoHandle> handles(num_handles);
+  std::vector<MojoHandle> raw_handles(num_handles);
 
   gin::ArrayBuffer buffer;
   ConvertFromV8(args.isolate(), array_buffer, &buffer);
   CHECK(buffer.num_bytes() == num_bytes);
 
-  result = MojoReadMessage(handle,
+  result = MojoReadMessage(handle->get(),
                            buffer.bytes(),
                            &num_bytes,
-                           handles.empty() ? NULL : &handles[0],
+                           raw_handles.empty() ? NULL : &raw_handles[0],
                            &num_handles,
                            flags);
 
   CHECK(buffer.num_bytes() == num_bytes);
-  CHECK(handles.size() == num_handles);
+  CHECK(raw_handles.size() == num_handles);
 
   gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(args.isolate());
   dictionary.Set("result", result);
   dictionary.Set("buffer", array_buffer);
+
+  std::vector<gin::Handle<HandleWrapper> > handles;

[abarth-chromium, 2014/03/29 01:56:17]

Ditto.

[Matt Perry, 2014/03/31 19:34:17]

Done.

+  for (size_t i = 0; i < raw_handles.size(); ++i)
+    handles[i] = HandleWrapper::Create(args.isolate(), raw_handles[i]);
   dictionary.Set("handles", handles);
   return dictionary;
 }
@@ -116,17 +165,20 @@ gin::Dictionary CreateDataPipe(const gin::Arguments& args,
   CHECK_EQ(MOJO_RESULT_OK, result);
 
   dictionary.Set("result", result);
-  dictionary.Set("producerHandle", producer_handle);
-  dictionary.Set("consumerHandle", consumer_handle);
+  dictionary.Set("producerHandle",
+                 HandleWrapper::Create(args.isolate(), producer_handle));
+  dictionary.Set("consumerHandle",
+                 HandleWrapper::Create(args.isolate(), consumer_handle));
   return dictionary;
 }
 
 gin::Dictionary WriteData(const gin::Arguments& args,
-                          MojoHandle handle,
+                          gin::Handle<HandleWrapper> handle,
                           const gin::ArrayBufferView& buffer,
                           MojoWriteDataFlags flags) {
   uint32_t num_bytes = static_cast<uint32_t>(buffer.num_bytes());
-  MojoResult result = MojoWriteData(handle, buffer.bytes(), &num_bytes, flags);
+  MojoResult result =
+      MojoWriteData(handle->get(), buffer.bytes(), &num_bytes, flags);
   gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(args.isolate());
   dictionary.Set("result", result);
   dictionary.Set("numBytes", num_bytes);
@@ -134,11 +186,11 @@ gin::Dictionary WriteData(const gin::Arguments& args,
 }
 
 gin::Dictionary ReadData(const gin::Arguments& args,
-                         MojoHandle handle,
+                         gin::Handle<HandleWrapper> handle,
                          MojoReadDataFlags flags) {
   uint32_t num_bytes = 0;
   MojoResult result = MojoReadData(
-      handle, NULL, &num_bytes, MOJO_READ_DATA_FLAG_QUERY);
+      handle->get(), NULL, &num_bytes, MOJO_READ_DATA_FLAG_QUERY);
   if (result != MOJO_RESULT_OK) {
     gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(args.isolate());
     dictionary.Set("result", result);
@@ -151,7 +203,7 @@ gin::Dictionary ReadData(const gin::Arguments& args,
   ConvertFromV8(args.isolate(), array_buffer, &buffer);
   CHECK_EQ(num_bytes, buffer.num_bytes());
 
-  result = MojoReadData(handle, buffer.bytes(), &num_bytes, flags);
+  result = MojoReadData(handle->get(), buffer.bytes(), &num_bytes, flags);
   CHECK_EQ(num_bytes, buffer.num_bytes());
 
   gin::Dictionary dictionary = gin::Dictionary::CreateEmpty(args.isolate());
@@ -173,10 +225,11 @@ v8::Local<v8::Value> Core::GetModule(v8::Isolate* isolate) {
 
   if (templ.IsEmpty()) {
      templ = gin::ObjectTemplateBuilder(isolate)
-        .SetMethod("close", mojo::CloseRaw)
-        .SetMethod("wait", mojo::Wait)
-        .SetMethod("waitMany", mojo::WaitMany<std::vector<mojo::Handle>,
-                                              std::vector<MojoWaitFlags> >)
+        // TODO(mpcomplete): Should these just be methods on the JS Handle
+        // object?

[abarth-chromium, 2014/03/29 01:56:17]

Yeah, at some point we should probably make these APIs more idiomatic for JS.

+        .SetMethod("close", CloseHandle)
+        .SetMethod("wait", WaitHandle)
+        .SetMethod("waitMany", WaitMany)
         .SetMethod("createMessagePipe", CreateMessagePipe)
         .SetMethod("writeMessage", WriteMessage)
         .SetMethod("readMessage", ReadMessage)