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

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

 // Copyright 2014 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.
 
 #ifndef MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATION_ERRORS_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_LIB_VALIDATION_ERRORS_H_
 
 #include "mojo/public/cpp/system/macros.h"
 
 namespace mojo {
@@ skipping 99 matching lines @@
 //   - otherwise, logs a fatal-level message.
 // |error| is the validation error that will be triggered by the receiver
 // of the serialzation result.
 //
 // In non-debug build, does nothing (not even compiling |condition|).
 #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 << ").";

[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_