gin: Create a DataObjectBuilder class to help create simple data objects.

gin/data_object_builder_unittest.cc

Index: gin/data_object_builder_unittest.cc
diff --git a/gin/data_object_builder_unittest.cc b/gin/data_object_builder_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..068bf65a673634721d04055ec12bac7d3bda703d
--- /dev/null
+++ b/gin/data_object_builder_unittest.cc
@@ -0,0 +1,109 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "gin/data_object_builder.h"
+
+#include "gin/dictionary.h"
+#include "gin/public/isolate_holder.h"
+#include "gin/test/v8_test.h"
+
+namespace gin {
+namespace {
+
+using DataObjectBuilderTest = V8Test;
+
+// It should create ordinary data properties.
+TEST_F(DataObjectBuilderTest, CreatesDataProperties) {
+  v8::Isolate* isolate = instance_->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Context> context = context_.Get(isolate);
+
+  v8::Local<v8::Object> object =
+      DataObjectBuilder(isolate).Set("key", 42).Finish();
+  ASSERT_TRUE(object->HasOwnProperty(context, StringToSymbol(isolate, "key"))
+                  .ToChecked());
+
+  v8::Local<v8::Value> descriptor_object;
+  ASSERT_TRUE(
+      object->GetOwnPropertyDescriptor(context, StringToSymbol(isolate, "key"))
+          .ToLocal(&descriptor_object));
+  gin::Dictionary descriptor(isolate, descriptor_object.As<v8::Object>());
+
+  int32_t value;
+  ASSERT_TRUE(descriptor.Get("value", &value));
+  EXPECT_EQ(42, value);
+
+  bool writable;

[Devlin, 2017/04/26 15:50:32]

nit: initialize (here and below)  (I know that since we ASSERT below, we guard
against any uninitialized reads, but it's just easier to not have uninitialized
values. :))

[jbroman, 2017/04/26 17:47:31]

Done. I kinda like this use as it strongly implies that the next line must
initialize it, but I'm totally okay with changing.

+  ASSERT_TRUE(descriptor.Get("writable", &writable));
+  EXPECT_TRUE(writable);
+
+  bool enumerable;
+  ASSERT_TRUE(descriptor.Get("enumerable", &enumerable));
+  EXPECT_TRUE(enumerable);
+
+  bool configurable;
+  ASSERT_TRUE(descriptor.Get("configurable", &configurable));
+  EXPECT_TRUE(configurable);
+}
+
+// It should not invoke setters on the prototype chain.
+TEST_F(DataObjectBuilderTest, DoesNotInvokeSetters) {
+  v8::Isolate* isolate = instance_->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Context> context = context_.Get(isolate);
+
+  // Install a setter on the object prototype.
+  v8::Local<v8::Value> object_constructor;
+  ASSERT_TRUE(context->Global()
+                  ->Get(context, StringToSymbol(isolate, "Object"))
+                  .ToLocal(&object_constructor));
+  v8::Local<v8::Value> object_prototype;
+  ASSERT_TRUE(object_constructor.As<v8::Function>()
+                  ->Get(context, StringToSymbol(isolate, "prototype"))
+                  .ToLocal(&object_prototype));
+  ASSERT_TRUE(
+      object_prototype.As<v8::Object>()
+          ->SetAccessor(context, StringToSymbol(isolate, "key"),
+                        [](v8::Local<v8::Name>,
+                           const v8::PropertyCallbackInfo<v8::Value>&) {},
+                        [](v8::Local<v8::Name>, v8::Local<v8::Value>,
+                           const v8::PropertyCallbackInfo<void>&) {
+                          ADD_FAILURE() << "setter should not be invoked";
+                        })
+          .ToChecked());
+
+  // Create an object.
+  DataObjectBuilder(isolate).Set("key", 42).Finish();
+}
+
+// The internal handle is cleared when the builder is finished.
+// This makes the class harder to abuse, so that its methods cannot be used
+// after something may have modified the object in unexpected ways.

[Devlin, 2017/04/26 15:50:32]

Oh, interesting.  I suggested this be a DCHECK failure.  I'm fine with either.

[jbroman, 2017/04/26 17:47:31]

(shrug) Replied above, let me know what you think.

+TEST_F(DataObjectBuilderTest, ReleasesOnFinish) {
+  v8::Isolate* isolate = instance_->isolate();
+  v8::HandleScope handle_scope(isolate);
+
+  DataObjectBuilder builder(isolate);
+  EXPECT_FALSE(builder.Finish().IsEmpty());
+  EXPECT_TRUE(builder.Finish().IsEmpty());
+}
+
+// As is the normal behaviour of CreateDataProperty, new data properties should
+// replace existing ones. Since no non-configurable ones are present, nor should
+// the object be non-extensible, this should work.
+TEST_F(DataObjectBuilderTest, ReplacesExistingProperties) {
+  v8::Isolate* isolate = instance_->isolate();
+  v8::HandleScope handle_scope(isolate);
+
+  v8::Local<v8::Object> object =
+      DataObjectBuilder(isolate).Set("value", 42).Set("value", 55).Finish();
+
+  gin::Dictionary dictionary(isolate, object);
+  int32_t value;
+  ASSERT_TRUE(dictionary.Get("value", &value));
+  EXPECT_EQ(55, value);
+}
+
+}  // namespace
+}  // namespace gin