Mojo C++ bindings: better log message for serialization warnings.

BUG=324170
TEST=None

Review URL: https://codereview.chromium.org/473793004

Cr-Commit-Position: refs/heads/master@{#291058}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291058 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 62 insertions, 15 deletions

diff --git a/mojo/public/cpp/bindings/lib/array_internal.cc b/mojo/public/cpp/bindings/lib/array_internal.cc
index 835b78a..9f62aa2 100644
--- a/mojo/public/cpp/bindings/lib/array_internal.cc
+++ b/mojo/public/cpp/bindings/lib/array_internal.cc
@@ -4,9 +4,28 @@
 
 #include "mojo/public/cpp/bindings/lib/array_internal.h"
 
+#include <sstream>
+
 namespace mojo {
 namespace internal {
 
+std::string MakeMessageWithArrayIndex(const char* message,
+                                      size_t size,
+                                      size_t index) {
+  std::ostringstream stream;
+  stream << message << ": array size - " << size << "; index - " << index;
+  return stream.str();
+}
+
+std::string MakeMessageWithExpectedArraySize(const char* message,
+                                             size_t size,
+                                             size_t expected_size) {
+  std::ostringstream stream;
+  stream << message << ": array size - " << size << "; expected size - "
+         << expected_size;
+  return stream.str();
+}
+
 ArrayDataTraits<bool>::BitRef::~BitRef() {
 }
 
diff --git a/mojo/public/cpp/bindings/lib/array_internal.h b/mojo/public/cpp/bindings/lib/array_internal.h
index a047db2..6b0baa0 100644
--- a/mojo/public/cpp/bindings/lib/array_internal.h
+++ b/mojo/public/cpp/bindings/lib/array_internal.h
@@ -27,6 +27,14 @@ namespace internal {
 // C++11).
 const uint32_t kMaxUint32 = 0xFFFFFFFF;
 
+std::string MakeMessageWithArrayIndex(const char* message,
+                                      size_t size,
+                                      size_t index);
+
+std::string MakeMessageWithExpectedArraySize(const char* message,
+                                             size_t size,
+                                             size_t expected_size);
+
 template <typename T>
 struct ArrayDataTraits {
   typedef T StorageType;
diff --git a/mojo/public/cpp/bindings/lib/array_serialization.h b/mojo/public/cpp/bindings/lib/array_serialization.h
index 2c399c89..fc62a88 100644
--- a/mojo/public/cpp/bindings/lib/array_serialization.h
+++ b/mojo/public/cpp/bindings/lib/array_serialization.h
@@ -103,7 +103,10 @@ template <typename H> struct ArraySerializer<ScopedHandleBase<H>, H, true> {
       output->at(i) = input[i].release();  // Transfer ownership of the handle.
       MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
           !element_is_nullable && !output->at(i).is_valid(),
-          VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE);
+          VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE,
+          MakeMessageWithArrayIndex(
+              "invalid handle in array expecting valid handles",
+              input.size(), i));
     }
   }
   static void DeserializeElements(
@@ -134,7 +137,9 @@ template <typename S> struct ArraySerializer<S, typename S::Data_*, true> {
       output->at(i) = element;
       MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
           !element_is_nullable && !element,
-          VALIDATION_ERROR_UNEXPECTED_NULL_POINTER);
+          VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
+          MakeMessageWithArrayIndex(
+              "null in array expecting valid pointers", input.size(), i));
     }
   }
   static void DeserializeElements(
@@ -193,7 +198,9 @@ template <> struct ArraySerializer<String, String_Data*, false> {
       output->at(i) = element;
       MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
           !element_is_nullable && !element,
-          VALIDATION_ERROR_UNEXPECTED_NULL_POINTER);
+          VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
+          MakeMessageWithArrayIndex(
+              "null in array expecting valid strings", input.size(), i));
     }
   }
   static void DeserializeElements(
@@ -222,7 +229,10 @@ inline void SerializeArray_(Array<E> input, internal::Buffer* buf,
     MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
         ValidateParams::expected_num_elements != 0 &&
             input.size() != ValidateParams::expected_num_elements,
-        internal::VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER);
+        internal::VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER,
+        internal::MakeMessageWithExpectedArraySize(
+            "fixed-size array has wrong number of elements",
+            input.size(), ValidateParams::expected_num_elements));
 
     internal::Array_Data<F>* result =
         internal::Array_Data<F>::New(input.size(), buf);
diff --git a/mojo/public/cpp/bindings/lib/validation_errors.h b/mojo/public/cpp/bindings/lib/validation_errors.h
index d7e651a..8ca0eae 100644
--- a/mojo/public/cpp/bindings/lib/validation_errors.h
+++ b/mojo/public/cpp/bindings/lib/validation_errors.h
@@ -100,9 +100,11 @@ class SerializationWarningObserverForTesting {
 // of the serialzation result.
 //
 // In non-debug build, does nothing (not even compiling |condition|).
-#define MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(condition, error) \
-    MOJO_DLOG_IF(FATAL, (condition) && !ReportSerializationWarning(error)) \
-        << "The outgoing message will trigger " \
-        << ValidationErrorToString(error) << " at the receiving side.";
+#define MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING( \
+    condition, error, description) \
+  MOJO_DLOG_IF(FATAL, (condition) && !ReportSerializationWarning(error)) \
+      << "The outgoing message will trigger " \
+      << ValidationErrorToString(error) << " at the receiving side (" \
+      << description << ").";
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATION_ERRORS_H_
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
index e1b82eb..998b6f3 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
@@ -44,7 +44,7 @@ params->{{param.name}}
 {%-   endfor %}
 {%- endmacro %}
 
-{%- macro build_message(params_name, parameters) -%}
+{%- macro build_message(params_name, parameters, params_description) -%}
   {# TODO(yzshen): Consider refactoring to share code with
      struct_serialization_definition.tmpl #}
   {{params_name}}* params =
@@ -61,7 +61,8 @@ params->{{param.name}}
 {%-       if not param.kind|is_nullable_kind %}
   MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
       !params->{{param.name}}.ptr,
-      mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER);
+      mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
+      "null {{param.name}} argument in {{params_description}}");
 {%-       endif %}
 {%-     elif param.kind|is_any_handle_kind %}
 {%-       if param.kind|is_interface_kind or
@@ -74,7 +75,8 @@ params->{{param.name}}
 {%-       if not param.kind|is_nullable_kind %}
   MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
       !params->{{param.name}}.is_valid(),
-      mojo::internal::VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE);
+      mojo::internal::VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE,
+      "invalid {{param.name}} argument in {{params_description}}");
 {%-       endif %}
 {%-     else %}
   params->{{param.name}} = in_{{param.name}};
@@ -132,6 +134,8 @@ bool {{class_name}}_{{method.name}}_ForwardToCallback::Accept(
           "internal::k%s_%s_Name"|format(interface.name, method.name) %}
 {%-   set params_name =
           "internal::%s_%s_Params_Data"|format(interface.name, method.name) %}
+{%-   set params_description =
+          "%s.%s request"|format(interface.name, method.name) %}
 void {{proxy_name}}::{{method.name}}(
     {{interface_macros.declare_request_params("in_", method)}}) {
   {{compute_payload_size(params_name, method.parameters)}}
@@ -142,7 +146,7 @@ void {{proxy_name}}::{{method.name}}(
   mojo::internal::MessageBuilder builder({{message_name}}, payload_size);
 {%- endif %}
 
-  {{build_message(params_name, method.parameters)}}
+  {{build_message(params_name, method.parameters, params_description)}}
 
 {%- if method.response_parameters != None %}
   mojo::MessageReceiver* responder =
@@ -164,6 +168,8 @@ void {{proxy_name}}::{{method.name}}(
             "internal::k%s_%s_Name"|format(interface.name, method.name) %}
 {%-     set params_name =
             "internal::%s_%s_ResponseParams_Data"|format(interface.name, method.name) %}
+{%-     set params_description =
+            "%s.%s response"|format(interface.name, method.name) %}
 class {{class_name}}_{{method.name}}_ProxyToResponder
     : public {{interface_macros.declare_callback(method)}}::Runnable {
  public:
@@ -190,7 +196,7 @@ void {{class_name}}_{{method.name}}_ProxyToResponder::Run(
   {{compute_payload_size(params_name, method.response_parameters)}}
   mojo::internal::ResponseMessageBuilder builder(
       {{message_name}}, payload_size, request_id_);
-  {{build_message(params_name, method.response_parameters)}}
+  {{build_message(params_name, method.response_parameters, params_description)}}
   bool ok MOJO_ALLOW_UNUSED = responder_->Accept(&message);
   // TODO(darin): !ok returned here indicates a malformed message, and that may
   // be good reason to close the connection. However, we don't have a way to do
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
index fb311cd..7a6d83d 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
@@ -24,7 +24,8 @@ void Serialize_({{struct.name}}Ptr input, mojo::internal::Buffer* buf,
 {%-     if not pf.field.kind|is_nullable_kind %}
     MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
         !result->{{pf.field.name}}.ptr,
-        mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER);
+        mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
+        "null {{pf.field.name}} field in {{struct.name}} struct");
 {%-     endif %}
 {%-   elif pf.field.kind|is_any_handle_kind %}
 {%-     if pf.field.kind|is_interface_kind %}
@@ -35,7 +36,8 @@ void Serialize_({{struct.name}}Ptr input, mojo::internal::Buffer* buf,
 {%-     if not pf.field.kind|is_nullable_kind %}
     MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(
         !result->{{pf.field.name}}.is_valid(),
-        mojo::internal::VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE);
+        mojo::internal::VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE,
+        "invalid {{pf.field.name}} field in {{struct.name}} struct");
 {%-     endif %}
 {%-   else %}
     result->{{pf.field.name}} = input->{{pf.field.name}};