[mojo] Don't inline unions with string fields

Inline allocations for unions have been allowed as long as the union
doesn't contain any non-string reference-type fields (i.e. non-handle,
non-POD fields). Exceptions were made for strings since they're copyable
and don't introduce a possibility of recursive types.

InlinedStructPtr however is incompatible with a union's generated
wrapper class when the union's first field is a reference type like a
string. This results in difficult to diagnose crashes in cases that are
meant to be allowed by the bindings.

This change prohibits unions from being allocated inline unless they
contain only POD fields.

Inline allocations could be allowed in these (and other) cases, but
that would require more fundamental refactoring of the wrapper types
and/or StructPtr/InlinedStructPtr.

Fixed: 1114366
Change-Id: I623779f067e777a9240aef67a40decc884948247
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347167Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#796660}

mojo/public/cpp/bindings/tests/union_unittest.cc

@@ -1229,5 +1229,11 @@ TEST(UnionTest, UnionInInterface) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST(UnionTest, InlineUnionAllocationWithNonPODFirstField) {
+  // Regression test for https://crbug.com/1114366. Should not crash.
+  UnionWithStringForFirstFieldPtr u;
+  u = UnionWithStringForFirstField::NewS("hey");
+}
+
 }  // namespace test
 }  // namespace mojo

mojo/public/interfaces/bindings/tests/test_unions.mojom

@@ -114,3 +114,8 @@ struct ImportedUnionStruct {
 union ImportedUnionUnion {
   imported.PointOrShape point_or_shape;
 };
+
+union UnionWithStringForFirstField {
+  string s;
+  int32 i;
+};

mojo/public/tools/bindings/generators/mojom_cpp_generator.py

@@ -180,9 +180,7 @@ def ShouldInlineStruct(struct):
 
 
 def ShouldInlineUnion(union):
-  return not any(
-      mojom.IsReferenceKind(field.kind) and not mojom.IsStringKind(field.kind)
-           for field in union.fields)
+  return not any(mojom.IsReferenceKind(field.kind) for field in union.fields)
 
 
 def HasPackedMethodOrdinals(interface):