[Extensions Bindings] Add support for custom property types

extensions/renderer/storage_area.cc

Index: extensions/renderer/storage_area.cc
diff --git a/extensions/renderer/storage_area.cc b/extensions/renderer/storage_area.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b5e0bbd9643c785cd729e5f9868e7004e6cf8ecd
--- /dev/null
+++ b/extensions/renderer/storage_area.cc
@@ -0,0 +1,243 @@
+// 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 "extensions/renderer/storage_area.h"
+
+#include "base/memory/ptr_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/values.h"
+#include "extensions/common/api/storage.h"
+#include "extensions/common/extension_api.h"
+#include "extensions/renderer/api_request_handler.h"
+#include "extensions/renderer/api_signature.h"
+#include "gin/arguments.h"
+#include "gin/handle.h"
+#include "gin/object_template_builder.h"
+#include "gin/wrappable.h"
+
+namespace extensions {
+
+namespace {
+
+template <typename T>
+void DecorateTemplateBuilder(gin::ObjectTemplateBuilder* builder) {
+  auto handler = [](const char* method_name, gin::Arguments* arguments) {
+    T* holder = nullptr;
+    if (!arguments->GetHolder(&holder)) {
+      arguments->ThrowTypeError("Invalid receiver for StorageArea method.");
+      return;
+    }
+    holder->storage_area().HandleFunctionCall(method_name, arguments);
+  };
+
+  for (const char* key : {"get", "set", "remove", "clear", "getBytesInUse"})

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

These comes from "js_module" key in current JS bindings, right? I'd mention
"js_module" in the CL description.

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

Done.

+    builder->SetMethod(key, base::Bind(handler, key));
+}
+
+// gin::Wrappables for each of the storage areas. Since each has slightly
+// different properties, and the object template is shared between all
+// instances, this is a little verbose.
+class LocalStorageArea final : public gin::Wrappable<LocalStorageArea> {
+ public:
+  LocalStorageArea(APIRequestHandler* request_handler,
+                   const APITypeReferenceMap* type_refs)
+      : storage_area_(request_handler, type_refs, "local") {}
+  ~LocalStorageArea() override = default;
+
+  static gin::WrapperInfo kWrapperInfo;
+
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override {
+    gin::ObjectTemplateBuilder builder =
+        Wrappable<LocalStorageArea>::GetObjectTemplateBuilder(isolate);
+    DecorateTemplateBuilder<LocalStorageArea>(&builder);
+    builder.SetValue("QUOTA_BYTES", api::storage::local::QUOTA_BYTES);

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

What's the plan for documentation generation for these as they won't be coming
from json schema anymore?

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

Right now, they're still half-coming-from-the-json, in that
api::storage::local::FOO is a generated file from the json, and that's what
we're using.

I thought about passing around the properties dictionary and using that, but
StorageArea is, AFAICT, the *only* implementation that does this.  So I figured
that it was better to have explicit code than create more spaghetti for the
one-off case.

I'm hoping that since we still link for the values, people will update both at
the same time (because adding another property for "FOO", <magic_number> would
look weird, and to add the constant to api::storage::local::FOO, you have to add
it to the json, and thus the documentation).

[lazyboy, 2017/02/22 22:15:24]

Yes. Agreed.
Acknowledged.

+    return builder;
+  }
+
+  StorageArea& storage_area() { return storage_area_; }

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

optional: Should storage_area_ be part of a common base class for all
XXXStorageArea classes?

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

Unfortunately, we can't.  gin::Wrappable<> does not allow for further
inheritance (i.e., we don't allow Foo : gin::Wrappable<>, CustomFoo : Foo), and
by style we only allow a single non-pure-interface inheritance (so having a Foo,
CustomFoo : Foo, gin::Wrappable<> is also out).

This was frustrating, because it *is* a lot of extra code, but I couldn't think
of a better way.

On the upside, we only need these for the cases where there are different
properties/methods for the different objects.

+
+ private:
+  StorageArea storage_area_;
+
+  DISALLOW_COPY_AND_ASSIGN(LocalStorageArea);
+};
+
+gin::WrapperInfo LocalStorageArea::kWrapperInfo = {gin::kEmbedderNativeGin};
+
+class SyncStorageArea final : public gin::Wrappable<SyncStorageArea> {
+ public:
+  SyncStorageArea(APIRequestHandler* request_handler,
+                  const APITypeReferenceMap* type_refs)
+      : storage_area_(request_handler, type_refs, "sync") {}
+  ~SyncStorageArea() override = default;
+
+  static gin::WrapperInfo kWrapperInfo;
+
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override {
+    gin::ObjectTemplateBuilder builder =
+        Wrappable<SyncStorageArea>::GetObjectTemplateBuilder(isolate);
+    DecorateTemplateBuilder<SyncStorageArea>(&builder);
+    builder.SetValue("QUOTA_BYTES", api::storage::sync::QUOTA_BYTES);
+    builder.SetValue("QUOTA_BYTES_PER_ITEM",
+                     api::storage::sync::QUOTA_BYTES_PER_ITEM);
+    builder.SetValue("MAX_ITEMS", api::storage::sync::MAX_ITEMS);
+    builder.SetValue("MAX_WRITE_OPERATIONS_PER_HOUR",
+                     api::storage::sync::MAX_WRITE_OPERATIONS_PER_HOUR);
+    builder.SetValue("MAX_WRITE_OPERATIONS_PER_MINUTE",
+                     api::storage::sync::MAX_WRITE_OPERATIONS_PER_MINUTE);
+    builder.SetValue(
+        "MAX_SUSTAINED_WRITE_OPERATIONS_PER_MINUTE",
+        api::storage::sync::MAX_SUSTAINED_WRITE_OPERATIONS_PER_MINUTE);
+    return builder;
+  }
+
+  StorageArea& storage_area() { return storage_area_; }
+
+ private:
+  StorageArea storage_area_;
+
+  DISALLOW_COPY_AND_ASSIGN(SyncStorageArea);
+};
+
+gin::WrapperInfo SyncStorageArea::kWrapperInfo = {gin::kEmbedderNativeGin};
+
+class ManagedStorageArea final : public gin::Wrappable<ManagedStorageArea> {
+ public:
+  ManagedStorageArea(APIRequestHandler* request_handler,
+                     const APITypeReferenceMap* type_refs)
+      : storage_area_(request_handler, type_refs, "managed") {}
+  ~ManagedStorageArea() override = default;
+
+  static gin::WrapperInfo kWrapperInfo;
+
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override {
+    gin::ObjectTemplateBuilder builder =
+        Wrappable<ManagedStorageArea>::GetObjectTemplateBuilder(isolate);
+    DecorateTemplateBuilder<ManagedStorageArea>(&builder);
+    return builder;
+  }
+
+  StorageArea& storage_area() { return storage_area_; }
+
+ private:
+  StorageArea storage_area_;
+
+  DISALLOW_COPY_AND_ASSIGN(ManagedStorageArea);
+};
+
+gin::WrapperInfo ManagedStorageArea::kWrapperInfo = {gin::kEmbedderNativeGin};
+
+}  // namespace
+
+StorageArea::StorageArea(APIRequestHandler* request_handler,
+                         const APITypeReferenceMap* type_refs,
+                         const std::string& name)
+    : request_handler_(request_handler), type_refs_(type_refs), name_(name) {}
+StorageArea::~StorageArea() = default;
+
+// static
+v8::Local<v8::Object> StorageArea::CreateStorageArea(
+    v8::Local<v8::Context> context,
+    const std::string& property_name,
+    APIRequestHandler* request_handler,
+    APITypeReferenceMap* type_refs) {
+  v8::Context::Scope context_scope(context);
+  v8::Local<v8::Object> object;
+  if (property_name == "local") {
+    gin::Handle<LocalStorageArea> handle =
+        gin::CreateHandle(context->GetIsolate(),
+                          new LocalStorageArea(request_handler, type_refs));
+    object = handle.ToV8().As<v8::Object>();
+  } else if (property_name == "sync") {
+    gin::Handle<SyncStorageArea> handle = gin::CreateHandle(
+        context->GetIsolate(), new SyncStorageArea(request_handler, type_refs));
+    object = handle.ToV8().As<v8::Object>();
+  } else {
+    CHECK_EQ("managed", property_name);
+    gin::Handle<ManagedStorageArea> handle =
+        gin::CreateHandle(context->GetIsolate(),
+                          new ManagedStorageArea(request_handler, type_refs));
+    object = handle.ToV8().As<v8::Object>();
+  }
+  return object;
+}
+
+void StorageArea::HandleFunctionCall(const std::string& method_name,
+                                     gin::Arguments* arguments) {
+  v8::Isolate* isolate = arguments->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();

[jbroman, 2017/02/21 20:22:07]

nit: I think we've been using holder context elsewhere for non-static functions
(as Blink does)?

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

Ah, yes, we have.  It's a little messier here, because in order to get at it we
need to do:
v8::Local<v8::Object> holder;
arguments->GetHolder(&holder);
holder->CreationContext()

Which isn't terrible, but not quite as clean.  In a followup, what do you think
of exposing something on gin::Arguments?  Either a Holder() that returns a
v8::Value<> or an info accessor to get at the raw v8::FunctionCallbackInfo.

[jbroman, 2017/02/22 21:05:28]

I have no objection to something along those lines, but I don't feel strongly.

+
+  std::vector<v8::Local<v8::Value>> argument_list;
+  if (arguments->Length() > 0) {
+    // Just copying handles should never fail.
+    CHECK(arguments->GetRemaining(&argument_list));
+  }
+
+  std::unique_ptr<base::ListValue> converted_arguments;
+  v8::Local<v8::Function> callback;
+  std::string error;
+  if (!GetFunctionSchema("storage", "storage.StorageArea", method_name)
+           .ParseArgumentsToJSON(context, argument_list, *type_refs_,
+                                 &converted_arguments, &callback, &error)) {
+    arguments->ThrowTypeError("Invalid invocation");
+    return;
+  }
+
+  converted_arguments->Insert(0u, base::MakeUnique<base::Value>(name_));
+  request_handler_->StartRequest(context, "storage." + method_name,
+                                 std::move(converted_arguments), callback,
+                                 v8::Local<v8::Function>());
+}
+
+const APISignature& StorageArea::GetFunctionSchema(
+    base::StringPiece api_name,
+    base::StringPiece type_name,
+    base::StringPiece function_name) {
+  std::string full_name = base::StringPrintf(
+      "%s.%s.%s", api_name.data(), type_name.data(), function_name.data());
+  auto iter = signatures_.find(full_name);
+  if (iter != signatures_.end())
+    return *iter->second;
+
+  const base::DictionaryValue* full_schema =
+      ExtensionAPI::GetSharedInstance()->GetSchema(api_name.as_string());
+  const base::ListValue* types = nullptr;
+  CHECK(full_schema->GetList("types", &types));
+  const base::DictionaryValue* type_schema = nullptr;
+  for (const auto& type : *types) {
+    const base::DictionaryValue* type_dict = nullptr;
+    CHECK(type->GetAsDictionary(&type_dict));
+    std::string id;
+    CHECK(type_dict->GetString("id", &id));
+    if (id == type_name) {
+      type_schema = type_dict;
+      break;
+    }
+  }
+  CHECK(type_schema);
+  const base::ListValue* type_functions = nullptr;
+  CHECK(type_schema->GetList("functions", &type_functions));
+  const base::ListValue* parameters = nullptr;
+  for (const auto& function : *type_functions) {
+    const base::DictionaryValue* function_dict = nullptr;
+    CHECK(function->GetAsDictionary(&function_dict));
+    std::string name;
+    CHECK(function_dict->GetString("name", &name));
+    if (name == function_name) {
+      CHECK(function_dict->GetList("parameters", &parameters));
+      break;
+    }
+  }
+  CHECK(parameters);
+  auto signature = base::MakeUnique<APISignature>(*parameters);
+  auto raw_signature = signature.get();

[jbroman, 2017/02/21 20:22:07]

nit: use auto* (or for this use, even "const auto*") when deducing pointer types

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

Done.

+  signatures_[full_name] = std::move(signature);
+  return *raw_signature;
+}
+
+}  // namespace extensions