[Extensions Bindings] Add bindings support for serializable functions

Add support for serializing JS functions to strings, rather than to
empty base::DictionaryValues. This will only be done for functions that
are marked with the trait serializableFunction in the schema.

Add testing for the same.

Bug: 1144841
Change-Id: I3990447ff336143a38eb97bf91a90503b0ee5c3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518451
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825765}

extensions/renderer/bindings/argument_spec.cc

@@ -4,6 +4,7 @@
 
 #include "extensions/renderer/bindings/argument_spec.h"
 
+#include "base/check.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -190,6 +191,9 @@ void ArgumentSpec::InitializeType(const base::DictionaryValue* dict) {
         enum_values_.insert(std::move(enum_value));
       }
     }
+  } else if (type_ == ArgumentType::FUNCTION) {
+    serialize_function_ =
+        dict->FindBoolKey("serializableFunction").value_or(false);
   }
 
   // Check if we should preserve null in objects. Right now, this is only used
@@ -301,19 +305,8 @@ bool ArgumentSpec::ParseArgument(v8::Local<v8::Context> context,
     case ArgumentType::BINARY:
       return ParseArgumentToAny(context, value, out_value, v8_out_value, error);
     case ArgumentType::FUNCTION:
-      if (out_value) {
-        // Certain APIs (contextMenus) have functions as parameters other than
-        // the callback (contextMenus uses it for an onclick listener). Our
-        // generated types have adapted to consider functions "objects" and
-        // serialize them as dictionaries.
-        // TODO(devlin): It'd be awfully nice to get rid of this eccentricity.
-        *out_value = std::make_unique<base::DictionaryValue>();
-      }
-
-      if (v8_out_value)
-        *v8_out_value = value;
-
-      return true;
+      return ParseArgumentToFunction(context, value, out_value, v8_out_value,
+                                     error);
     case ArgumentType::REF: {
       DCHECK(ref_);
       const ArgumentSpec* reference = refs.GetSpec(ref_.value());
@@ -754,6 +747,43 @@ bool ArgumentSpec::ParseArgumentToAny(v8::Local<v8::Context> context,
   return true;
 }
 
+bool ArgumentSpec::ParseArgumentToFunction(
+    v8::Local<v8::Context> context,
+    v8::Local<v8::Value> value,
+    std::unique_ptr<base::Value>* out_value,
+    v8::Local<v8::Value>* v8_out_value,
+    std::string* error) const {
+  DCHECK_EQ(ArgumentType::FUNCTION, type_);
+  DCHECK(value->IsFunction());
+  if (out_value) {
+    if (serialize_function_) {
+      v8::Local<v8::String> serialized_function;
+      if (!value.As<v8::Function>()->FunctionProtoToString(context).ToLocal(
+              &serialized_function)) {
+        *error = api_errors::ScriptThrewError();
+        return false;
+      }
+      std::string str;
+      // If ToLocal() succeeds, this should always be a string.
+      CHECK(gin::Converter<std::string>::FromV8(context->GetIsolate(),
+                                                serialized_function, &str));
+      *out_value = std::make_unique<base::Value>(std::move(str));
+    } else {  // Not a serializable function.
+      // Certain APIs (contextMenus) have functions as parameters other than
+      // the callback (contextMenus uses it for an onclick listener). Our
+      // generated types have adapted to consider functions "objects" and
+      // serialize them as dictionaries.
+      // TODO(devlin): It'd be awfully nice to get rid of this eccentricity.
+      *out_value = std::make_unique<base::DictionaryValue>();
+    }
+  }
+
+  if (v8_out_value)
+    *v8_out_value = value;
+
+  return true;
+}
+
 std::string ArgumentSpec::GetInvalidTypeError(
     v8::Local<v8::Value> value) const {
   return api_errors::InvalidType(GetTypeName().c_str(),

extensions/renderer/bindings/argument_spec.h

@@ -107,6 +107,9 @@ class ArgumentSpec {
     instance_of_ = std::move(instance_of);
   }
   void set_preserve_null(bool preserve_null) { preserve_null_ = preserve_null; }
+  void set_serialize_function(bool serialize_function) {
+    serialize_function_ = serialize_function;
+  }
 
  private:
   // Initializes this object according to |type_string| and |dict|.
@@ -136,6 +139,11 @@ class ArgumentSpec {
                           std::unique_ptr<base::Value>* out_value,
                           v8::Local<v8::Value>* v8_out_value,
                           std::string* error) const;
+  bool ParseArgumentToFunction(v8::Local<v8::Context> context,
+                               v8::Local<v8::Value> value,
+                               std::unique_ptr<base::Value>* out_value,
+                               v8::Local<v8::Value>* v8_out_value,
+                               std::string* error) const;
 
   // Returns an error message indicating the type of |value| does not match the
   // expected type.
@@ -156,6 +164,10 @@ class ArgumentSpec {
   // Whether to preserve null properties found in objects.
   bool preserve_null_ = false;
 
+  // Whether to serialize (by stringifying) a function argument. Only valid for
+  // arguments of type FUNCTION.
+  bool serialize_function_ = false;
+
   // The reference the argument points to, if any. Note that if this is set,
   // none of the following fields describing the argument will be.
   base::Optional<std::string> ref_;

extensions/renderer/bindings/argument_spec_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "extensions/renderer/bindings/api_binding_test_util.h"
 #include "extensions/renderer/bindings/api_invocation_errors.h"
@@ -1016,4 +1017,37 @@ TEST_F(ArgumentSpecUnitTest, TestV8ValuePassedThrough) {
   }
 }
 
+// Tests the serialization of functions that are explicitly marked as
+// serializable (otherwise, they are represented as empty DictionaryValues).
+TEST_F(ArgumentSpecUnitTest, SerializableFunctions) {
+  constexpr char kFunctionSpec[] =
+      R"({
+           "type": "function",
+           "serializableFunction": true
+         })";
+  ArgumentSpec spec(*ValueFromString(kFunctionSpec));
+
+  constexpr char kExpectedSerialization[] = R"("function() { }")";
+  ExpectSuccess(spec, "(function() { })", kExpectedSerialization);
+
+  {
+    constexpr char kNonTrivialFunction[] =
+        R"(function(foo, bar, baz) { let alpha = baz; })";
+    ExpectSuccess(spec, base::StringPrintf("(%s)", kNonTrivialFunction),
+                  base::StringPrintf(R"("%s")", kNonTrivialFunction));
+  }
+
+  // Test a couple tricky values with custom toString() implementations.
+  ExpectSuccess(spec,
+                R"(var f = function() { };
+         f.toString = function() { throw new Error('haha!'); };
+         f;)",
+                kExpectedSerialization);
+  ExpectSuccess(spec,
+                R"(var g = function() { };
+         g.toString = function() { return 'function() { return 3; }'; };
+         g;)",
+                kExpectedSerialization);
+}
+
 }  // namespace extensions