[Extensions Bindings] Add argument parsing errors

extensions/renderer/api_invocation_errors.cc

+// 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/api_invocation_errors.h"
+
+#include "base/strings/stringprintf.h"
+
+namespace extensions {
+namespace api_errors {
+
+const char kTypeString[] = "string";
+const char kTypeDouble[] = "number";
+const char kTypeBoolean[] = "boolean";
+const char kTypeInteger[] = "integer";
+const char kTypeObject[] = "object";
+const char kTypeList[] = "array";
+const char kTypeBinary[] = "binary";
+const char kTypeFunction[] = "function";
+const char kTypeUndefined[] = "undefined";
+const char kTypeNull[] = "null";
+
+const char kInvalidEnumValue[] = "Value must be one of %s.";
+const char kMissingRequiredProperty[] = "Missing required property '%s'.";
+const char kUnexpectedProperty[] = "Unexpected property: '%s'.";
+const char kTooFewArrayItems[] = "Array must have at least %d items.";
+const char kTooManyArrayItems[] = "Array must have at most %d items.";
+const char kTooFewStringChars[] = "String must have at least %d characters.";
+const char kTooManyStringChars[] = "String must have at most %d characters.";
+const char kNumberTooSmall[] = "Value must be at least %d.";
+const char kNumberTooLarge[] = "Value must be at most %d.";
+const char kInvalidType[] = "Invalid type: expected %s, found %s.";
+const char kNotAnInstance[] = "Value must be an instance of %s.";
+const char kInvalidChoice[] = "Value did not match any choice.";
+const char kUnserializableValue[] = "Value is unserializable.";
+const char kScriptThrewError[] = "Script threw an error.";
+
+std::string GetError(const char* error, int placeholder) {

[jbroman, 2017/04/25 15:44:55]

Hmm, my main question about this is that it's not obvious without inspecting the
source file which messages require one placeholder, which require two, etc. It
might be slightly less error-prone if these either had the format string at
their call site (where this can be seen by inspection), or this source file had
functions for each message, like:

std::string InvalidEnumValue(const char* valid_enumerators) {
  return base::StringPrintf("Value must be one of %s.", valid_enumerators);
}

[Devlin, 2017/04/25 22:47:59]

Good idea!  I was a little worried about the excess code, but, in retrospect,
this makes them all much more readable.  Done.

+  return base::StringPrintf(error, placeholder);
+}
+
+std::string GetError(const char* error, const char* placeholder) {
+  return base::StringPrintf(error, placeholder);
+}
+
+std::string GetError(const char* error,
+                     const char* placeholder1,
+                     const char* placeholder2) {
+  return base::StringPrintf(error, placeholder1, placeholder2);
+}
+
+std::string GetIndexError(uint32_t index, base::StringPiece error) {
+  return base::StringPrintf("Error at index %u: %s", index, error.data());

[jbroman, 2017/04/25 15:44:55]

You can't pass base::StringPiece to fill %s in printf, because it is not
null-terminated. (There's a comment warning about this in string_piece.h.)

You can kinda do it like this, with two caveats:
- convert the size to int, which would cause a problem if it were ever over 2G
- an embedded null will end the string early (rather than including the embedded
null in the result)

return base::StringPrintf("Error at index %u: %.*s", index, error.size(),
error.data());

Alternatively, you could use another approach, like operator+, or converting to
an std::string and then calling c_str.

[Devlin, 2017/04/25 22:47:59]

Good catch!  Opted for using std::strings for the errors, since they should be
constructed from other error calls anyway.

+}
+
+std::string GetPropertyError(base::StringPiece property_name,
+                             base::StringPiece error) {
+  return base::StringPrintf("Error at property '%s': %s", property_name.data(),
+                            error.data());
+}
+
+}  // namespace api_errors
+}  // namespace extensions