[Extensions Bindings] Add support for custom property types

extensions/renderer/api_binding.cc

Index: extensions/renderer/api_binding.cc
diff --git a/extensions/renderer/api_binding.cc b/extensions/renderer/api_binding.cc
index d3364792157c05b5ca7ab93826f2fdaf21ad8c9a..dc786821f75d7d84474154e4c9fb9f18de90ba82 100644
--- a/extensions/renderer/api_binding.cc
+++ b/extensions/renderer/api_binding.cc
@@ -72,64 +72,6 @@ void CallbackHelper(const v8::FunctionCallbackInfo<v8::Value>& info) {
   callback->Run(&args);
 }
 
-// Decorates |object_template| with the properties specified by |properties|.
-void DecorateTemplateWithProperties(
-    v8::Isolate* isolate,
-    v8::Local<v8::ObjectTemplate> object_template,
-    const base::DictionaryValue& properties) {
-  static const char kValueKey[] = "value";
-  for (base::DictionaryValue::Iterator iter(properties); !iter.IsAtEnd();
-       iter.Advance()) {
-    const base::DictionaryValue* dict = nullptr;
-    CHECK(iter.value().GetAsDictionary(&dict));
-    bool optional = false;
-    if (dict->GetBoolean("optional", &optional)) {
-      // TODO(devlin): What does optional even mean here? It's only used, it
-      // seems, for lastError and inIncognitoContext, which are both handled
-      // with custom bindings. Investigate, and remove.
-      continue;
-    }
-    std::string ref;
-    if (dict->GetString("$ref", &ref)) {
-      // TODO(devlin): Handle refs. These are tricky(er), because they represent
-      // a type that's defined in (currently) JS.
-      continue;
-    }
-    std::string type;
-    CHECK(dict->GetString("type", &type));
-    if (type != "object" && !dict->HasKey(kValueKey)) {
-      // TODO(devlin): What does a fundamental property not having a value mean?
-      // It doesn't seem useful, and looks like it's only used by runtime.id,
-      // which is set by custom bindings. Investigate, and remove.
-      continue;
-    }
-    if (type == "integer") {
-      int val = 0;
-      CHECK(dict->GetInteger(kValueKey, &val));
-      object_template->Set(isolate, iter.key().c_str(),
-                           v8::Integer::New(isolate, val));
-    } else if (type == "boolean") {
-      bool val = false;
-      CHECK(dict->GetBoolean(kValueKey, &val));
-      object_template->Set(isolate, iter.key().c_str(),
-                           v8::Boolean::New(isolate, val));
-    } else if (type == "string") {
-      std::string val;
-      CHECK(dict->GetString(kValueKey, &val)) << iter.key();
-      object_template->Set(isolate, iter.key().c_str(),
-                           gin::StringToSymbol(isolate, val));
-    } else if (type == "object") {
-      v8::Local<v8::ObjectTemplate> property_template =
-          v8::ObjectTemplate::New(isolate);
-      const base::DictionaryValue* property_dict = nullptr;
-      CHECK(dict->GetDictionary("properties", &property_dict));
-      DecorateTemplateWithProperties(isolate, property_template,
-                                     *property_dict);
-      object_template->Set(isolate, iter.key().c_str(), property_template);
-    }
-  }
-}
-
 }  // namespace
 
 struct APIBinding::MethodData {
@@ -166,17 +108,32 @@ struct APIBinding::EventData {
   APIEventHandler* event_handler;
 };
 
+struct APIBinding::CustomPropertyData {
+  CustomPropertyData(const std::string& type_name,
+                     const std::string& property_name,
+                     const CreateCustomType& create_custom_type)
+      : type_name(type_name),
+        property_name(property_name),
+        create_custom_type(create_custom_type) {}
+
+  std::string type_name;

[lazyboy, 2017/02/22 03:37:03]

I find giving examples of these are helpful to navigate the code.
e.g. chrome.storage.local refers to type StorageArea, type_name would be
"storage.StorageArea", property_name would be "local"

[Devlin, 2017/02/22 20:56:45]

Done.

+  std::string property_name;
+  CreateCustomType create_custom_type;
+};
+
 APIBinding::APIBinding(const std::string& api_name,
                        const base::ListValue* function_definitions,
                        const base::ListValue* type_definitions,
                        const base::ListValue* event_definitions,
                        const base::DictionaryValue* property_definitions,
+                       const CreateCustomType& create_custom_type,
                        std::unique_ptr<APIBindingHooks> binding_hooks,
                        APITypeReferenceMap* type_refs,
                        APIRequestHandler* request_handler,
                        APIEventHandler* event_handler)
     : api_name_(api_name),
       property_definitions_(property_definitions),
+      create_custom_type_(create_custom_type),
       binding_hooks_(std::move(binding_hooks)),
       type_refs_(type_refs),
       request_handler_(request_handler),
@@ -327,6 +284,67 @@ void APIBinding::InitializeTemplate(v8::Isolate* isolate) {
   object_template_.Set(isolate, object_template);
 }
 
+void APIBinding::DecorateTemplateWithProperties(
+    v8::Isolate* isolate,
+    v8::Local<v8::ObjectTemplate> object_template,
+    const base::DictionaryValue& properties) {
+  static const char kValueKey[] = "value";
+  for (base::DictionaryValue::Iterator iter(properties); !iter.IsAtEnd();
+       iter.Advance()) {
+    const base::DictionaryValue* dict = nullptr;
+    CHECK(iter.value().GetAsDictionary(&dict));
+    bool optional = false;
+    if (dict->GetBoolean("optional", &optional)) {
+      // TODO(devlin): What does optional even mean here? It's only used, it
+      // seems, for lastError and inIncognitoContext, which are both handled
+      // with custom bindings. Investigate, and remove.
+      continue;
+    }
+
+    v8::Local<v8::String> v8_key = gin::StringToSymbol(isolate, iter.key());
+    std::string ref;
+    if (dict->GetString("$ref", &ref)) {

[lazyboy, 2017/02/22 03:37:03]

For storage, we would get "StorageArea" as $ref in JSON and since we registered
the custom type "storage.StorageArea" in native_extensions_bindings_system.cc,
we will be able to retrieve the custom type?

So the custom bindings need to be implemented in c++ but we need to express its
presence in JSON schema, right? I'd document that in
APIBindingsSystem::RegisterCustomType()

[Devlin, 2017/02/22 20:56:45]

In our json generation/compression, we fully specify all types, so the ref
becomes storage.StorageArea.  Check out
src/out/Debug/gen/extensions/common/api/generated_schemas.cc.
Sg; done.

+      auto property_data = base::MakeUnique<CustomPropertyData>(
+          ref, iter.key(), create_custom_type_);
+      object_template->SetLazyDataProperty(
+          v8_key, &APIBinding::GetCustomPropertyObject,
+          v8::External::New(isolate, property_data.get()));
+      custom_properties_.push_back(std::move(property_data));
+      continue;
+    }
+
+    std::string type;
+    CHECK(dict->GetString("type", &type));
+    if (type != "object" && !dict->HasKey(kValueKey)) {
+      // TODO(devlin): What does a fundamental property not having a value mean?
+      // It doesn't seem useful, and looks like it's only used by runtime.id,
+      // which is set by custom bindings. Investigate, and remove.
+      continue;
+    }
+    if (type == "integer") {
+      int val = 0;
+      CHECK(dict->GetInteger(kValueKey, &val));
+      object_template->Set(v8_key, v8::Integer::New(isolate, val));
+    } else if (type == "boolean") {
+      bool val = false;
+      CHECK(dict->GetBoolean(kValueKey, &val));
+      object_template->Set(v8_key, v8::Boolean::New(isolate, val));
+    } else if (type == "string") {
+      std::string val;
+      CHECK(dict->GetString(kValueKey, &val)) << iter.key();
+      object_template->Set(v8_key, gin::StringToSymbol(isolate, val));
+    } else if (type == "object" || !ref.empty()) {
+      v8::Local<v8::ObjectTemplate> property_template =
+          v8::ObjectTemplate::New(isolate);
+      const base::DictionaryValue* property_dict = nullptr;
+      CHECK(dict->GetDictionary("properties", &property_dict));
+      DecorateTemplateWithProperties(isolate, property_template,
+                                     *property_dict);
+      object_template->Set(v8_key, property_template);
+    }
+  }
+}
+
 // static
 void APIBinding::GetEventObject(
     v8::Local<v8::Name> property,
@@ -344,6 +362,27 @@ void APIBinding::GetEventObject(
       event_data->full_name, context));
 }
 
+void APIBinding::GetCustomPropertyObject(
+    v8::Local<v8::Name> property_name,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+  v8::Isolate* isolate = info.GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Context> context = info.Holder()->CreationContext();
+  if (!IsContextValid(context))
+    return;
+
+  CHECK(info.Data()->IsExternal());
+  auto* property_data =
+      static_cast<CustomPropertyData*>(info.Data().As<v8::External>()->Value());
+
+  v8::Local<v8::Object> property = property_data->create_custom_type.Run(
+      context, property_data->type_name, property_data->property_name);
+  if (property.IsEmpty())
+    return;
+
+  info.GetReturnValue().Set(property);
+}
+
 void APIBinding::HandleCall(const std::string& name,
                             const APISignature* signature,
                             gin::Arguments* arguments) {