[Extensions Bindings] Add errors to signature parsing

extensions/renderer/argument_spec.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/argument_spec.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_piece.h"
 #include "base/values.h"
 #include "content/public/child/v8_value_converter.h"
@@ skipping 402 matching lines @@
 
     v8::Local<v8::Value> prop_value;
     // Fun: It's possible that a previous getter has removed the property from
     // the object. This isn't that big of a deal, since it would only manifest
     // in the case of some reasonably-crazy script objects, and it's probably
     // not worth optimizing for the uncommon case to the detriment of the
     // common (and either should be totally safe). We can always add a
     // HasOwnProperty() check here in the future, if we desire.
     // See also comment in ParseArgumentToArray() about passing in custom
     // crazy values here.
-    if (!object->Get(context, key).ToLocal(&prop_value))
+    if (!object->Get(context, key).ToLocal(&prop_value)) {
+      // Technically, in the case of the property being removed before access,

[jbroman, 2017/04/27 19:37:20]

Hmm? Getting a nonexistent property will successfully produce undefined. This
branch is only for cases where some exception happened (either deliberated by
script, or due to another issue like an access check).

[Devlin, 2017/05/01 22:22:09]

Ah, good point.  Comment removed.

+      // this error message will be wrong. But since that can only happen if
+      // someone removes a property after the initial GetOwnPropertyNames()
+      // check, this seems reasonable enough - these error messages are meant
+      // to be helpful, but don't need 100% accuracy in deliberately-crazy
+      // cases.
+      *error = api_errors::ScriptThrewError();
       return false;
+    }
 
     // Note: We don't serialize undefined or null values.
     // TODO(devlin): This matches current behavior, but it is correct?
     if (prop_value->IsUndefined() || prop_value->IsNull()) {
       if (!property_spec->optional_) {
         *error = api_errors::MissingRequiredProperty(*utf8_key);
         return false;
       }
       continue;
     }
@@ skipping 160 matching lines @@
       break;
     case ArgumentType::CHOICES:
     case ArgumentType::ANY:
       NOTREACHED();
   }
 
   return api_errors::InvalidType(expected_type, GetV8ValueTypeString(value));
 }
 
 }  // namespace extensions