[Extensions Bindings] Accept null callbacks while ignoring schema

extensions/renderer/api_signature.cc

 // Copyright 2016 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/api_signature.h"
 
 #include <algorithm>
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
@@ skipping 299 matching lines @@
   // TODO(devlin): This is what the current bindings do, but it's quite terribly
   // incorrect. We only hit this flow when an API method has a hook to update
   // the arguments post-validation, and in some cases, the arguments returned by
   // that hook do *not* match the signature of the API method (e.g.
   // fileSystem.getDisplayPath); see also note in api_bindings.cc for why this
   // is bad. But then here, we *rely* on the signature to determine whether or
   // not the last parameter is a callback, even though the hooks may not return
   // the arguments in the signature. This is very broken.
   if (HasCallback(signature_)) {
     CHECK(!arguments.empty());
-    if (arguments[size - 1]->IsFunction()) {
-      callback = arguments[size - 1].As<v8::Function>();
-      --size;
-    }
+    v8::Local<v8::Value> value = arguments.back();
+    --size;
+    DCHECK(value->IsFunction() || value->IsUndefined() || value->IsNull());

[jbroman, 2017/06/05 18:16:32]

This DCHECK implies this value can only come from Chrome code, right? (it's not
provided by the extension author?)

[Devlin, 2017/06/09 20:26:31]

More-or-less. :/  In theory, we should only get here after the extension
bindings have validated the signature, so we should know that it's one of these.
 In practice, there's flaws with this that are described above in the TODO. 
I've added a brief comment here explaining that.

+    if (value->IsFunction())
+      callback = value.As<v8::Function>();
   }
 
   auto json = base::MakeUnique<base::ListValue>();
   std::unique_ptr<content::V8ValueConverter> converter(
       content::V8ValueConverter::create());
   converter->SetFunctionAllowed(true);
   for (size_t i = 0; i < size; ++i) {
     std::unique_ptr<base::Value> converted =
         converter->FromV8Value(arguments[i], context);
     if (!converted)
@@ skipping 17 matching lines @@
     pieces.push_back(
         base::StringPrintf("%s%s %s", spec->optional() ? kOptionalPrefix : "",
                            spec->GetTypeName().c_str(), spec->name().c_str()));
   }
   expected_signature_ = base::JoinString(pieces, ", ");
 
   return expected_signature_;
 }
 
 }  // namespace extensions