[Extensions Bindings] Add native declarativeContent verification

extensions/renderer/declarative_content_hooks_delegate.cc

Index: extensions/renderer/declarative_content_hooks_delegate.cc
diff --git a/extensions/renderer/declarative_content_hooks_delegate.cc b/extensions/renderer/declarative_content_hooks_delegate.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b79b8716293cf798a48a4a49c7f05506b54e3b39
--- /dev/null
+++ b/extensions/renderer/declarative_content_hooks_delegate.cc
@@ -0,0 +1,238 @@
+// 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/declarative_content_hooks_delegate.h"
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "extensions/renderer/api_type_reference_map.h"
+#include "extensions/renderer/argument_spec.h"
+#include "gin/arguments.h"
+#include "gin/converter.h"
+#include "third_party/WebKit/public/platform/WebString.h"
+#include "third_party/WebKit/public/web/WebSelector.h"
+
+namespace extensions {
+
+namespace {
+
+void CallbackHelper(const v8::FunctionCallbackInfo<v8::Value>& info) {
+  CHECK(info.Data()->IsExternal());
+  v8::Local<v8::External> external = info.Data().As<v8::External>();
+  auto* callback =
+      static_cast<DeclarativeContentHooksDelegate::HandlerCallback*>(
+          external->Value());
+  callback->Run(info);
+}
+
+// Copies the 'own' properties from src -> dst.
+bool V8Assign(v8::Local<v8::Context> context,
+              v8::Local<v8::Object> src,
+              v8::Local<v8::Object> dst) {
+  v8::Local<v8::Array> own_property_names;
+  if (!src->GetOwnPropertyNames(context).ToLocal(&own_property_names))
+    return false;
+
+  uint32_t length = own_property_names->Length();
+  for (uint32_t i = 0; i < length; ++i) {
+    v8::Local<v8::Value> key;
+    if (!own_property_names->Get(context, i).ToLocal(&key))
+      return false;
+    DCHECK(key->IsString() || key->IsUint32());
+
+    v8::Local<v8::Value> prop_value;
+    if (!src->Get(context, key).ToLocal(&prop_value))
+      return false;
+
+    v8::Maybe<bool> success =
+        key->IsString()
+            ? dst->CreateDataProperty(context, key.As<v8::String>(), prop_value)
+            : dst->CreateDataProperty(context, key.As<v8::Uint32>()->Value(),
+                                      prop_value);
+    if (!success.IsJust() || !success.FromJust())
+      return false;
+  }
+
+  return true;
+}
+
+// Canonicalizes any css selectors specified in a page state matcher, returning
+// true on success.
+bool CanonicalizeCssSelectors(v8::Local<v8::Context> context,
+                              v8::Local<v8::Object> object,
+                              std::string* error) {
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::Local<v8::String> key = gin::StringToSymbol(isolate, "css");

[lazyboy, 2017/05/03 18:47:00]

declarative_content_constants::kCss

[Devlin, 2017/05/03 22:55:02]

Done.  (Note: this meant moving the file, which was previously in
chrome/browser, but that seems like the right thing to do.)

+  v8::Maybe<bool> has_css = object->HasOwnProperty(context, key);
+  // Note: don't bother populating |error| if script threw an exception.
+  if (!has_css.IsJust())
+    return false;
+
+  if (!has_css.FromJust())
+    return true;
+
+  v8::Local<v8::Value> css;
+  if (!object->Get(context, key).ToLocal(&css))
+    return false;
+
+  if (css->IsUndefined())
+    return true;
+
+  if (!css->IsArray())
+    return false;
+
+  v8::Local<v8::Array> css_array = css.As<v8::Array>();
+  uint32_t length = css_array->Length();
+  for (uint32_t i = 0; i < length; ++i) {
+    v8::Local<v8::Value> val;
+    if (!css_array->Get(context, i).ToLocal(&val) || !val->IsString())
+      return false;
+    v8::String::Utf8Value selector(val.As<v8::String>());
+    std::string parsed =
+        blink::CanonicalizeSelector(

[lazyboy, 2017/05/03 18:47:00]

note: The blink::CanonicalizeSelector call from css_natives_handler has a vague
TODO saying "this API shouldn't exist", although I don't know what "this" refers
to. So pointing it out.

[Devlin, 2017/05/03 22:55:02]

Added a note.

+            blink::WebString::FromUTF8(*selector, selector.length()),
+            blink::kWebSelectorTypeCompound)
+            .Utf8();
+    if (parsed.empty()) {
+      *error =
+          "Invalid css selector: " + std::string(*selector, selector.length());

[lazyboy, 2017/05/03 18:47:00]

nit: s/css/CSS since we're not talking about the "css" key.

[Devlin, 2017/05/03 22:55:01]

Done.

+      return false;
+    }
+    v8::Maybe<bool> set_result =
+        css_array->Set(context, i, gin::StringToSymbol(isolate, parsed));
+    if (!set_result.IsJust() || !set_result.FromJust())
+      return false;
+  }
+
+  return true;
+}
+
+// Validates the source object against the expected spec, and copies over values
+// to |this_object|. Returns true on success.
+bool Validate(const ArgumentSpec* spec,
+              const APITypeReferenceMap& type_refs,
+              v8::Local<v8::Context> context,
+              v8::Local<v8::Object> this_object,
+              v8::Local<v8::Object> source_object,
+              const std::string& type_name,
+              std::string* error) {
+  if (!source_object.IsEmpty() &&
+      !V8Assign(context, source_object, this_object)) {
+    return false;
+  }
+
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::Maybe<bool> set_result = this_object->CreateDataProperty(
+      context, gin::StringToSymbol(isolate, "instanceType"),

[lazyboy, 2017/05/03 18:47:01]

declarative_content_constants::kInstanceType

[Devlin, 2017/05/03 22:55:01]

Done.

+      gin::StringToSymbol(isolate, type_name));
+  if (!set_result.IsJust() || !set_result.FromJust()) {
+    return false;
+  }
+
+  if (!spec->ParseArgument(context, this_object, type_refs, nullptr, error)) {
+    return false;
+  }
+
+  if (type_name == "declarativeContent.PageStateMatcher" &&

[lazyboy, 2017/05/03 18:47:00]

The full names match declarative_contents_constants::kPageStateMatcherType,
should we be using that instead? Same for line 160 below.

[Devlin, 2017/05/03 22:55:02]

Done.

+      !CanonicalizeCssSelectors(context, this_object, error)) {
+    return false;
+  }
+  return true;
+}
+
+}  // namespace
+
+DeclarativeContentHooksDelegate::DeclarativeContentHooksDelegate() {}
+DeclarativeContentHooksDelegate::~DeclarativeContentHooksDelegate() {}
+
+void DeclarativeContentHooksDelegate::InitializeTemplate(
+    v8::Isolate* isolate,
+    v8::Local<v8::ObjectTemplate> object_template,
+    const APITypeReferenceMap& type_refs) {
+  // Add constructors for the API types.
+  // TODO(devlin): We'll need to extract out common logic here and share it with
+  // declarativeWebRequest.
+  struct {
+    const char* full_name;
+    const char* exposed_name;
+  } kTypes[] = {
+      {"declarativeContent.PageStateMatcher", "PageStateMatcher"},
+      {"declarativeContent.ShowPageAction", "ShowPageAction"},
+      {"declarativeContent.SetIcon", "SetIcon"},
+      {"declarativeContent.RequestContentScript", "RequestContentScript"},
+  };
+  for (const auto& type : kTypes) {
+    const ArgumentSpec* spec = type_refs.GetSpec(type.full_name);
+    DCHECK(spec);
+    // This object should outlive any calls to the function, so this
+    // base::Unretained and the callback itself are safe. Similarly, the same
+    // bindings system owns all these objects, so the spec and type refs should
+    // also be safe.
+    callbacks_.push_back(base::MakeUnique<HandlerCallback>(

[lazyboy, 2017/05/03 18:47:01]

At size 4, the vector is going to most likely realloc. So reserve?

[Devlin, 2017/05/03 22:55:01]

Done.

+        base::Bind(&DeclarativeContentHooksDelegate::HandleCall,
+                   base::Unretained(this), spec, &type_refs, type.full_name)));
+    object_template->Set(
+        gin::StringToSymbol(isolate, type.exposed_name),
+        v8::FunctionTemplate::New(
+            isolate, &CallbackHelper,
+            v8::External::New(isolate, callbacks_.back().get())));
+  }
+}
+
+void DeclarativeContentHooksDelegate::HandleCall(
+    const ArgumentSpec* spec,
+    const APITypeReferenceMap* type_refs,
+    const std::string& type_name,
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  gin::Arguments arguments(info);
+  v8::Isolate* isolate = arguments.isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+
+  // TODO(devlin): It would be pretty nice to be able to throw an error if
+  // Arguments::IsConstructCall() is false. That would ensure that the caller
+  // used `new declarativeContent.Foo()`, which is a) the documented approach
+  // and b) allows us (more) confidence that the |this| object we receive is
+  // an unmodified instance. But we don't know how many extensions enforcing
+  // that may break, and it's also incompatible with SetIcon().
+
+  v8::Local<v8::Object> this_object = info.This();
+  if (this_object.IsEmpty()) {
+    // Crazy script (e.g. declarativeContent.Foo.apply(null, args);).
+    NOTREACHED();
+    return;
+  }
+
+  // TODO(devlin): Find a way to use APISignature here? It's a little awkward
+  // because of undocumented expected properties like instanceType and not
+  // requiring an argument at all. We may need a better way of expressing these
+  // in the JSON schema.
+  if (arguments.Length() > 1) {
+    arguments.ThrowTypeError("Invalid invocation.");
+    return;
+  }
+
+  v8::Local<v8::Object> properties;
+  if (arguments.Length() == 1 && !arguments.GetNext(&properties)) {
+    arguments.ThrowTypeError("Invalid invocation.");
+    return;
+  }
+
+  std::string error;
+  bool success = false;
+  {
+    v8::TryCatch try_catch(isolate);
+    success = Validate(spec, *type_refs, context, this_object, properties,
+                       type_name, &error);
+    if (try_catch.HasCaught()) {
+      try_catch.ReThrow();
+      return;
+    }
+  }
+
+  if (!success)
+    arguments.ThrowTypeError("Invalid invocation: " + error);
+}
+
+}  // namespace extensions