commit bebb5c663a5d32b21ab72a1599c036fb988390e6
author Emily Andrews <emiled@microsoft.com> Tue Jul 30 17:32:00 2019
committer Commit Bot <commit-bot@chromium.org> Tue Jul 30 17:32:00 2019
tree 63e3c152f1ff6a14b190b55eebf8caf6f0f3ed92
parent 79fab97a496818f1ac48f2212bc0451585b84a9b

Modify mojo::internal::Validate* functions for 48KB disk space savings

mojo::internal::ValidateStruct is not able to fold properly even though
almost 90% of the disassembly of the defined template specializations is
the same because the call to T::Validate is unique.

This change slightly modifies the internals of mojo::internal::Validate*
to call a new helper function which pulls out most of the similar
functionality. While the helper function is also templated, because the
compiled specializations are identical, the compiler is able to fold all
the calls to the same function.

This reduces the overall size of chrome.dll and chrome_child.dll by
24 KB each for a total of 48 KB in disk space savings. If there are
other dlls that compile in mojo, these will experience similar benefits.

Other approaches considered included:
- creating an interface which defined helper functions within
T:Validate. While this did allow all the functions to fold for 400 KB
savings, this also opened us to a security risk because the *_Data
objects are sent via IPC and could be coming from anywhere.
- Including the ScopedDepthTracker call in the helper. This resulted in
bugs since the ScopedDepthTracker takes advantage of deconstructors to
keep track of how far down we've recursed through Validate function.
- Generalize the header validation code generated in T:Validate. This
resulted in a size increase. The reason is that we know the number of
loops at compile time, so the compiler is able to take advantage of loop
unrolling.

Bug: 988151
Change-Id: I80e8d35005dc3ec1a8c09762bf14b0823af7c3bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1723023
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#682339}

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

diff --git a/mojo/public/cpp/bindings/lib/validation_util.h b/mojo/public/cpp/bindings/lib/validation_util.h
index 3b88956f..a8edae3a 100644
--- a/mojo/public/cpp/bindings/lib/validation_util.h
+++ b/mojo/public/cpp/bindings/lib/validation_util.h
@@ -147,16 +147,24 @@
     ValidationContext* validation_context);
 
 template <typename T>
-bool ValidateContainer(const Pointer<T>& input,
-                       ValidationContext* validation_context,
-                       const ContainerValidateParams* validate_params) {
-  ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
+bool ValidateParams(const Pointer<T>& input,
+                    ValidationContext* validation_context) {
   if (validation_context->ExceedsMaxDepth()) {
     ReportValidationError(validation_context,
                           VALIDATION_ERROR_MAX_RECURSION_DEPTH);
     return false;
   }
-  return ValidatePointer(input, validation_context) &&
+
+  return ValidatePointer(input, validation_context);
+}
+
+template <typename T>
+bool ValidateContainer(const Pointer<T>& input,
+                       ValidationContext* validation_context,
+                       const ContainerValidateParams* validate_params) {
+  ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
+
+  return ValidateParams(input, validation_context) &&
          T::Validate(input.Get(), validation_context, validate_params);
 }
 
@@ -164,12 +172,8 @@
 bool ValidateStruct(const Pointer<T>& input,
                     ValidationContext* validation_context) {
   ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
-  if (validation_context->ExceedsMaxDepth()) {
-    ReportValidationError(validation_context,
-                          VALIDATION_ERROR_MAX_RECURSION_DEPTH);
-    return false;
-  }
-  return ValidatePointer(input, validation_context) &&
+
+  return ValidateParams(input, validation_context) &&
          T::Validate(input.Get(), validation_context);
 }
 
@@ -189,12 +193,8 @@
 bool ValidateNonInlinedUnion(const Pointer<T>& input,
                              ValidationContext* validation_context) {
   ValidationContext::ScopedDepthTracker depth_tracker(validation_context);
-  if (validation_context->ExceedsMaxDepth()) {
-    ReportValidationError(validation_context,
-                          VALIDATION_ERROR_MAX_RECURSION_DEPTH);
-    return false;
-  }
-  return ValidatePointer(input, validation_context) &&
+
+  return ValidateParams(input, validation_context) &&
          T::Validate(input.Get(), validation_context, false);
 }