[Extensions Bindings] Add support for updateArgumentsPreValidate

extensions/renderer/api_signature.cc

Index: extensions/renderer/api_signature.cc
diff --git a/extensions/renderer/api_signature.cc b/extensions/renderer/api_signature.cc
index 6d83c38ccdb41d690dce05e09c187619c281a7d1..bed2e9a40f2a7dab8bd5c145a7e2e5324f6b02fb 100644
--- a/extensions/renderer/api_signature.cc
+++ b/extensions/renderer/api_signature.cc
@@ -17,11 +17,12 @@ namespace {
 // should *only* be used directly on the stack!
 class ArgumentParser {
  public:
-  ArgumentParser(const std::vector<std::unique_ptr<ArgumentSpec>>& signature,
-                 gin::Arguments* arguments,
+  ArgumentParser(v8::Local<v8::Context> context,
+                 const std::vector<std::unique_ptr<ArgumentSpec>>& signature,
+                 const std::vector<v8::Local<v8::Value>>& arguments,
                  const ArgumentSpec::RefMap& type_refs,
                  std::string* error)
-    : context_(arguments->isolate()->GetCurrentContext()),
+    : context_(context),
       signature_(signature),
       arguments_(arguments),
       type_refs_(type_refs),
@@ -35,6 +36,15 @@ class ArgumentParser {
   v8::Isolate* GetIsolate() { return context_->GetIsolate(); }
 
  private:
+  v8::Local<v8::Value> next_argument() {
+    return current_index_ < arguments_.size() ?
+        arguments_[current_index_] : v8::Local<v8::Value>();
+  }
+
+  void consume_argument() {

[jbroman, 2016/12/23 22:37:06]

nit: This doesn't look like a simple getter/setter to me, so I think this ought
to be named ConsumeArgument.

[Devlin, 2016/12/28 19:02:39]

Not entirely sure I agree (it's basically just incrementing the internal member,
and I think incrementers are allowed to be hacker_case since they're largely
setters) - but given it's a private API in a .cc file, I don't feel strongly at
all :)

Done.

+    current_index_ = std::min(arguments_.size(), current_index_ + 1);
+  }
+
   // Attempts to match the next argument to the given |spec|.
   // If the next argument does not match and |spec| is optional, uses a null
   // value.
@@ -56,21 +66,23 @@ class ArgumentParser {
 
   v8::Local<v8::Context> context_;
   const std::vector<std::unique_ptr<ArgumentSpec>>& signature_;
-  gin::Arguments* arguments_;
+  const std::vector<v8::Local<v8::Value>>& arguments_;
   const ArgumentSpec::RefMap& type_refs_;
   std::string* error_;
+  size_t current_index_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(ArgumentParser);
 };
 
 class V8ArgumentParser : public ArgumentParser {
  public:
-  V8ArgumentParser(const std::vector<std::unique_ptr<ArgumentSpec>>& signature,
-                   gin::Arguments* arguments,
+  V8ArgumentParser(v8::Local<v8::Context> context,
+                   const std::vector<std::unique_ptr<ArgumentSpec>>& signature,
+                   const std::vector<v8::Local<v8::Value>>& arguments,
                    const ArgumentSpec::RefMap& type_refs,
                    std::string* error,
                    std::vector<v8::Local<v8::Value>>* values)
-      : ArgumentParser(signature, arguments, type_refs, error),
+      : ArgumentParser(context, signature, arguments, type_refs, error),
         values_(values) {}
 
  private:
@@ -91,12 +103,13 @@ class V8ArgumentParser : public ArgumentParser {
 class BaseValueArgumentParser : public ArgumentParser {
  public:
   BaseValueArgumentParser(
+      v8::Local<v8::Context> context,
       const std::vector<std::unique_ptr<ArgumentSpec>>& signature,
-      gin::Arguments* arguments,
+      const std::vector<v8::Local<v8::Value>>& arguments,
       const ArgumentSpec::RefMap& type_refs,
       std::string* error,
       base::ListValue* list_value)
-      : ArgumentParser(signature, arguments, type_refs, error),
+      : ArgumentParser(context, signature, arguments, type_refs, error),
         list_value_(list_value) {}
 
   v8::Local<v8::Function> callback() { return callback_; }
@@ -143,14 +156,14 @@ bool ArgumentParser::ParseArguments() {
   if (signature_has_callback && !ParseCallback(*signature_.back()))
     return false;
 
-  if (!arguments_->PeekNext().IsEmpty())
+  if (current_index_ != arguments_.size())
     return false;  // Extra arguments aren't allowed.
 
   return true;
 }
 
 bool ArgumentParser::ParseArgument(const ArgumentSpec& spec) {
-  v8::Local<v8::Value> value = arguments_->PeekNext();
+  v8::Local<v8::Value> value = next_argument();
   if (value.IsEmpty() || value->IsNull() || value->IsUndefined()) {
     if (!spec.optional()) {
       *error_ = "Missing required argument: " + spec.name();
@@ -158,7 +171,7 @@ bool ArgumentParser::ParseArgument(const ArgumentSpec& spec) {
     }
     // This is safe to call even if |arguments| is at the end (which can happen
     // if n optional arguments are omitted at the end of the signature).
-    arguments_->Skip();
+    consume_argument();
 
     AddNull();
     return true;
@@ -174,19 +187,19 @@ bool ArgumentParser::ParseArgument(const ArgumentSpec& spec) {
     return true;
   }
 
-  arguments_->Skip();
+  consume_argument();
   AddParsedArgument(value);
   return true;
 }
 
 bool ArgumentParser::ParseCallback(const ArgumentSpec& spec) {
-  v8::Local<v8::Value> value = arguments_->PeekNext();
+  v8::Local<v8::Value> value = next_argument();
   if (value.IsEmpty() || value->IsNull() || value->IsUndefined()) {
     if (!spec.optional()) {
       *error_ = "Missing required argument: " + spec.name();
       return false;
     }
-    arguments_->Skip();
+    consume_argument();
     return true;
   }
 
@@ -195,7 +208,7 @@ bool ArgumentParser::ParseCallback(const ArgumentSpec& spec) {
     return false;
   }
 
-  arguments_->Skip();
+  consume_argument();
   SetCallback(value.As<v8::Function>());
   return true;
 }
@@ -213,13 +226,16 @@ APISignature::APISignature(const base::ListValue& specification) {
 
 APISignature::~APISignature() {}
 
-bool APISignature::ParseArgumentsToV8(gin::Arguments* arguments,
-                                      const ArgumentSpec::RefMap& type_refs,
-                                      std::vector<v8::Local<v8::Value>>* v8_out,
-                                      std::string* error) const {
+bool APISignature::ParseArgumentsToV8(
+    v8::Local<v8::Context> context,
+    const std::vector<v8::Local<v8::Value>>& arguments,
+    const ArgumentSpec::RefMap& type_refs,
+    std::vector<v8::Local<v8::Value>>* v8_out,
+    std::string* error) const {
   DCHECK(v8_out);
   std::vector<v8::Local<v8::Value>> v8_values;
-  V8ArgumentParser parser(signature_, arguments, type_refs, error, &v8_values);
+  V8ArgumentParser parser(
+      context, signature_, arguments, type_refs, error, &v8_values);
   if (!parser.ParseArguments())
     return false;
   *v8_out = std::move(v8_values);
@@ -227,7 +243,8 @@ bool APISignature::ParseArgumentsToV8(gin::Arguments* arguments,
 }
 
 bool APISignature::ParseArgumentsToJSON(
-    gin::Arguments* arguments,
+    v8::Local<v8::Context> context,
+    const std::vector<v8::Local<v8::Value>>& arguments,
     const ArgumentSpec::RefMap& type_refs,
     std::unique_ptr<base::ListValue>* json_out,
     v8::Local<v8::Function>* callback_out,
@@ -236,7 +253,7 @@ bool APISignature::ParseArgumentsToJSON(
   DCHECK(callback_out);
   std::unique_ptr<base::ListValue> json = base::MakeUnique<base::ListValue>();
   BaseValueArgumentParser parser(
-      signature_, arguments, type_refs, error, json.get());
+      context, signature_, arguments, type_refs, error, json.get());
   if (!parser.ParseArguments())
     return false;
   *json_out = std::move(json);