mojo::Serialize*_() calls now propogate/return validation errors.

mojo/public/cpp/bindings/lib/validation_errors.h

Index: mojo/public/cpp/bindings/lib/validation_errors.h
diff --git a/mojo/public/cpp/bindings/lib/validation_errors.h b/mojo/public/cpp/bindings/lib/validation_errors.h
index 43059c2e22b8ee6c18a089b1381bbdf4df66edb2..0364bae2ae1842e8bc116ecad9e3c5f3c6c5af37 100644
--- a/mojo/public/cpp/bindings/lib/validation_errors.h
+++ b/mojo/public/cpp/bindings/lib/validation_errors.h
@@ -119,4 +119,16 @@ class SerializationWarningObserverForTesting {
       << ValidationErrorToString(error) << " at the receiving side ("    \
       << description << ").";

[viettrungluu, 2015/10/06 16:40:06]

Drive-by: It's rather odd that this macro ends with a ';'.

[vardhan, 2015/10/06 22:57:14]

Done.

 
+// If condition is true, we either report or make a fatal log message, and then
+// return.
+#define MOJO_INTERNAL_SERIALIZATION_CHECK_AND_RETURN(condition, return_code,  \

[viettrungluu, 2015/10/06 16:40:06]

(This is not a well-written macro, for a number of reasons[1][2].)

[1] People will probably use it with a trailing semicolon, in which case there
ends up being an extra semicolon.

This may seem like a trivial point, but it breaks expectations if used in, e.g.:

if (something)
  MOJO_INTERNAL_SERIALIZATION_CHECK_AND_RETURN(...);
else
  ...;

[2] The following is "valid":

MOJO_INTERNAL_SERIALIZATION_CHECK_AND_RETURN(...)
else
  ...;

This may seem unlikely, but removing the semicolon in the example in [1] makes
it compile, but do the wrong thing!

[vardhan, 2015/10/06 22:57:14]

Ah, I didn't think about this enough, that's a good point.
But I changed the way I do this in response to your comment below.

+                                                     description)             \
+  if (condition) {                                                            \
+    MOJO_DLOG_IF(FATAL, !ReportSerializationWarning(return_code))             \
+        << "The outgoing message will trigger "                               \
+        << ValidationErrorToString(return_code) << " at the receiving side (" \
+        << description << ").";                                               \
+    return return_code;                                                       \

[viettrungluu, 2015/10/06 16:40:06]

But in general, I don't think it's a good idea to have a |return| inside a
macro.

I suggest either living with some redundancy[1] (for the sake of clarity) or
factoring out the MOJO_DLOG_IF in MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING() and
using it[2].

[1]:

MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING(my condition, ...);
if (my condition)
  return return_code;

[2]:

if (my condition) {
  MOJO_INTERNAL_WHATEVER_YOU_CALL_IT(...);
  return return_code;
}

where MOJO_INTERNAL_WHATEVER_YOU_CALL_IT() is:

MOJO_DLOG_IF(FATAL, !ReportSerializationWarning(error)) << ...

In which case you should probably rewrite
MOJO_INTERNAL_SERIALIZATION_CHECK_AND_RETURN() as:

do { \
  if (condition) \
    MOJO_INTERNAL_WHATEVER_YOU_CALL_IT(...); \
} while (false)

[vardhan, 2015/10/06 22:57:14]

I did something like [2]. PTAL!

+  }
+
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATION_ERRORS_H_