Implement gin::ObjectTemplateBuilder

Implement gin::ObjectTemplateBuilder

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=237867

[abarth-chromium, 2013-11-28 06:27:08]

LGTM

This is fantastic.

https://codereview.chromium.org/93813002/diff/1/gin/function_template.h
File gin/function_template.h (right):

https://codereview.chromium.org/93813002/diff/1/gin/function_template.h#newcode8
gin/function_template.h:8: #define GIN_FUNCTION_TEMPLATE_H_
!

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.cc
File gin/object_template_builder.cc (right):

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.c...
gin/object_template_builder.cc:24: return template_;
Should we clear out template_ at this point?

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.h
File gin/object_template_builder.h (right):

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.h...
gin/object_template_builder.h:20: ObjectTemplateBuilder(v8::Isolate* isolate);
Please mark one argument constructors explicit

https://codereview.chromium.org/93813002/diff/1/gin/test/gtest.cc
File gin/test/gtest.cc (right):

https://codereview.chromium.org/93813002/diff/1/gin/test/gtest.cc#newcode52
gin/test/gtest.cc:52: templ = builder
We could even use a temporary and avoid giving the builder a name:

templ = ObjectTemplateBuilder(isolate)
             .SetMethod(...)
             .Build();

https://codereview.chromium.org/93813002/diff/1/mojo/apps/js/threading.cc
File mojo/apps/js/threading.cc (right):

https://codereview.chromium.org/93813002/diff/1/mojo/apps/js/threading.cc#new...
mojo/apps/js/threading.cc:36: DCHECK(g_initial_handle != MOJO_HANDLE_INVALID);
This is gone now.  You can delete these lines.

https://codereview.chromium.org/93813002/diff/1/mojo/public/bindings/js/core.cc
File mojo/public/bindings/js/core.cc (right):

https://codereview.chromium.org/93813002/diff/1/mojo/public/bindings/js/core....
mojo/public/bindings/js/core.cc:99: .SetMethod("wait", mojo::Wait)
Oh, wow.  The base::Bind is implicit?

[Aaron Boodman, 2013-11-28 18:25:22]

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.cc
File gin/object_template_builder.cc (right):

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.c...
gin/object_template_builder.cc:24: return template_;
On 2013/11/28 06:27:09, abarth wrote:
> Should we clear out template_ at this point?

Done.

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.h
File gin/object_template_builder.h (right):

https://codereview.chromium.org/93813002/diff/1/gin/object_template_builder.h...
gin/object_template_builder.h:20: ObjectTemplateBuilder(v8::Isolate* isolate);
On 2013/11/28 06:27:09, abarth wrote:
> Please mark one argument constructors explicit

Whoops, thanks.

https://codereview.chromium.org/93813002/diff/1/gin/test/gtest.cc
File gin/test/gtest.cc (right):

https://codereview.chromium.org/93813002/diff/1/gin/test/gtest.cc#newcode52
gin/test/gtest.cc:52: templ = builder
On 2013/11/28 06:27:09, abarth wrote:
> We could even use a temporary and avoid giving the builder a name:
> 
> templ = ObjectTemplateBuilder(isolate)
>              .SetMethod(...)
>              .Build();

Done.

https://codereview.chromium.org/93813002/diff/1/mojo/apps/js/threading.cc
File mojo/apps/js/threading.cc (right):

https://codereview.chromium.org/93813002/diff/1/mojo/apps/js/threading.cc#new...
mojo/apps/js/threading.cc:36: DCHECK(g_initial_handle != MOJO_HANDLE_INVALID);
On 2013/11/28 06:27:09, abarth wrote:
> This is gone now.  You can delete these lines.

Done.

https://codereview.chromium.org/93813002/diff/1/mojo/public/bindings/js/core.cc
File mojo/public/bindings/js/core.cc (right):

https://codereview.chromium.org/93813002/diff/1/mojo/public/bindings/js/core....
mojo/public/bindings/js/core.cc:99: .SetMethod("wait", mojo::Wait)
On 2013/11/28 06:27:09, abarth wrote:
> Oh, wow.  The base::Bind is implicit?

Yeah, just a little sugar. There's an override that allows a pre-bound thing
still.

[commit-bot: I haz the power, 2013-11-28 18:26:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/93813002/40001

[commit-bot: I haz the power, 2013-11-28 18:26:08]

Failed to apply patch for mojo/public/bindings/js/core.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file mojo/public/bindings/js/core.cc
  Hunk #2 FAILED at 93.
  1 out of 2 hunks FAILED -- saving rejects to file
mojo/public/bindings/js/core.cc.rej

Patch:       mojo/public/bindings/js/core.cc
Index: mojo/public/bindings/js/core.cc
diff --git a/mojo/public/bindings/js/core.cc b/mojo/public/bindings/js/core.cc
index
cb6a12ca7380c21c266e0d1bff5be81816720281..624eed5f66d70f70757c9675bdc49ab93ecf17f7
100644
--- a/mojo/public/bindings/js/core.cc
+++ b/mojo/public/bindings/js/core.cc
@@ -11,6 +11,7 @@
 #include "gin/converter.h"
 #include "gin/dictionary.h"
 #include "gin/function_template.h"
+#include "gin/object_template_builder.h"
 #include "gin/per_isolate_data.h"
 #include "gin/public/wrapper_info.h"
 #include "mojo/public/bindings/js/handle.h"
@@ -92,85 +93,49 @@ v8::Local<v8::ObjectTemplate> Core::GetTemplate(v8::Isolate*
isolate) {
       &g_wrapper_info);
 
   if (templ.IsEmpty()) {
-    templ = v8::ObjectTemplate::New();
-
-    templ->Set(gin::StringToSymbol(isolate, "close"),
-               gin::CreateFunctionTemplate(isolate,
-                   base::Bind(mojo::CloseRaw)));
-    templ->Set(gin::StringToSymbol(isolate, "wait"),
-               gin::CreateFunctionTemplate(isolate,
-                   base::Bind(mojo::Wait)));
-    templ->Set(gin::StringToSymbol(isolate, "waitMany"),
-               gin::CreateFunctionTemplate(isolate,
-                   base::Bind(mojo::WaitMany<std::vector<mojo::Handle>,
-                                             std::vector<MojoWaitFlags> >)));
-    templ->Set(gin::StringToSymbol(isolate, "createMessagePipe"),
-               gin::CreateFunctionTemplate(isolate,
-                   base::Bind(CreateMessagePipe)));
-    templ->Set(gin::StringToSymbol(isolate, "writeMessage"),
-               gin::CreateFunctionTemplate(isolate,
-                   base::Bind(WriteMessage)));
-    templ->Set(gin::StringToSymbol(isolate, "readMessage"),
-               gin::CreateFunctionTemplate(isolate,
-                   base::Bind(ReadMessage)));
-
-    // TODO(vtl): Change name of "kInvalidHandle", now that there's no such C++
-    // constant?
-    templ->Set(gin::StringToSymbol(isolate, "kInvalidHandle"),
-               gin::ConvertToV8(isolate, mojo::Handle()));
-
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_OK"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_OK));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_CANCELLED"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_CANCELLED));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_UNKNOWN"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_UNKNOWN));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_INVALID_ARGUMENT"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_INVALID_ARGUMENT));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_DEADLINE_EXCEEDED"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_DEADLINE_EXCEEDED));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_NOT_FOUND"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_NOT_FOUND));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_ALREADY_EXISTS"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_ALREADY_EXISTS));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_PERMISSION_DENIED"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_PERMISSION_DENIED));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_RESOURCE_EXHAUSTED"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_RESOURCE_EXHAUSTED));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_FAILED_PRECONDITION"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_FAILED_PRECONDITION));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_ABORTED"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_ABORTED));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_OUT_OF_RANGE"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_OUT_OF_RANGE));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_UNIMPLEMENTED"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_UNIMPLEMENTED));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_INTERNAL"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_INTERNAL));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_UNAVAILABLE"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_UNAVAILABLE));
-    templ->Set(gin::StringToSymbol(isolate, "RESULT_DATA_LOSS"),
-               gin::ConvertToV8(isolate, MOJO_RESULT_DATA_LOSS));
-
-    templ->Set(gin::StringToSymbol(isolate, "DEADLINE_INDEFINITE"),
-               gin::ConvertToV8(isolate, MOJO_DEADLINE_INDEFINITE));
-
-    templ->Set(gin::StringToSymbol(isolate, "WAIT_FLAG_NONE"),
-               gin::ConvertToV8(isolate, MOJO_WAIT_FLAG_NONE));
-    templ->Set(gin::StringToSymbol(isolate, "WAIT_FLAG_READABLE"),
-               gin::ConvertToV8(isolate, MOJO_WAIT_FLAG_READABLE));
-    templ->Set(gin::StringToSymbol(isolate, "WAIT_FLAG_READABLE"),
-               gin::ConvertToV8(isolate, MOJO_WAIT_FLAG_READABLE));
-    templ->Set(gin::StringToSymbol(isolate, "WAIT_FLAG_EVERYTHING"),
-               gin::ConvertToV8(isolate, MOJO_WAIT_FLAG_EVERYTHING));
-
-    templ->Set(gin::StringToSymbol(isolate, "WRITE_MESSAGE_FLAG_NONE"),
-               gin::ConvertToV8(isolate, MOJO_WRITE_MESSAGE_FLAG_NONE));
-
-    templ->Set(gin::StringToSymbol(isolate, "READ_MESSAGE_FLAG_NONE"),
-               gin::ConvertToV8(isolate, MOJO_READ_MESSAGE_FLAG_NONE));
-    templ->Set(gin::StringToSymbol(isolate, "READ_MESSAGE_FLAG_MAY_DISCARD"),
-               gin::ConvertToV8(isolate, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD));
+     templ = gin::ObjectTemplateBuilder(isolate)
+        .SetMethod("close", mojo::CloseRaw)
+        .SetMethod("wait", mojo::Wait)
+        .SetMethod("waitMany", mojo::WaitMany<std::vector<mojo::Handle>,
+                                              std::vector<MojoWaitFlags> >)
+        .SetMethod("createMessagePipe", CreateMessagePipe)
+        .SetMethod("writeMessage", WriteMessage)
+        .SetMethod("readMessage", ReadMessage)
+
+        // TODO(vtl): Change name of "kInvalidHandle", now that there's no such
+        // C++ constant?
+        .SetValue("kInvalidHandle", mojo::Handle())
+
+        .SetValue("RESULT_OK", MOJO_RESULT_OK)
+        .SetValue("RESULT_CANCELLED", MOJO_RESULT_CANCELLED)
+        .SetValue("RESULT_UNKNOWN", MOJO_RESULT_UNKNOWN)
+        .SetValue("RESULT_INVALID_ARGUMENT", MOJO_RESULT_INVALID_ARGUMENT)
+        .SetValue("RESULT_DEADLINE_EXCEEDED", MOJO_RESULT_DEADLINE_EXCEEDED)
+        .SetValue("RESULT_NOT_FOUND", MOJO_RESULT_NOT_FOUND)
+        .SetValue("RESULT_ALREADY_EXISTS", MOJO_RESULT_ALREADY_EXISTS)
+        .SetValue("RESULT_PERMISSION_DENIED", MOJO_RESULT_PERMISSION_DENIED)
+        .SetValue("RESULT_RESOURCE_EXHAUSTED", MOJO_RESULT_RESOURCE_EXHAUSTED)
+        .SetValue("RESULT_FAILED_PRECONDITION",
MOJO_RESULT_FAILED_PRECONDITION)
+        .SetValue("RESULT_ABORTED", MOJO_RESULT_ABORTED)
+        .SetValue("RESULT_OUT_OF_RANGE", MOJO_RESULT_OUT_OF_RANGE)
+        .SetValue("RESULT_UNIMPLEMENTED", MOJO_RESULT_UNIMPLEMENTED)
+        .SetValue("RESULT_INTERNAL", MOJO_RESULT_INTERNAL)
+        .SetValue("RESULT_UNAVAILABLE", MOJO_RESULT_UNAVAILABLE)
+        .SetValue("RESULT_DATA_LOSS", MOJO_RESULT_DATA_LOSS)
+
+        .SetValue("DEADLINE_INDEFINITE", MOJO_DEADLINE_INDEFINITE)
+
+        .SetValue("WAIT_FLAG_NONE", MOJO_WAIT_FLAG_NONE)
+        .SetValue("WAIT_FLAG_READABLE", MOJO_WAIT_FLAG_READABLE)
+        .SetValue("WAIT_FLAG_READABLE", MOJO_WAIT_FLAG_READABLE)
+        .SetValue("WAIT_FLAG_EVERYTHING", MOJO_WAIT_FLAG_EVERYTHING)
+
+        .SetValue("WRITE_MESSAGE_FLAG_NONE", MOJO_WRITE_MESSAGE_FLAG_NONE)
+
+        .SetValue("READ_MESSAGE_FLAG_NONE", MOJO_READ_MESSAGE_FLAG_NONE)
+        .SetValue("READ_MESSAGE_FLAG_MAY_DISCARD",
+                  MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)
+        .Build();
 
     data->SetObjectTemplate(&g_wrapper_info, templ);
   }

[commit-bot: I haz the power, 2013-11-28 18:45:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/93813002/60001

[commit-bot: I haz the power, 2013-11-28 19:26:10]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, cacheinvalidation_unittests, cc_unittests, check_deps,
chromedriver2_unittests, components_unittests, content_browsertests,
content_unittests, crypto_unittests, google_apis_unittests, gpu_unittests,
interactive_ui_tests, ipc_tests, jingle_unittests, media_unittests,
nacl_integration, net_unittests, ppapi_unittests, printing_unittests,
remoting_unittests, sandbox_linux_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-29 01:39:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/93813002/80001

[commit-bot: I haz the power, 2013-11-29 03:21:50]

Change committed as 237867