Use System.Text.Json's source generated code to deserialize WebEvents

[pranavkm]

Also address missing "type" property on TouchEvent

Fixes #32357

[eerhardt]

Why the double wrapped JsonSerializerOptions?

[pranavkm]

It looks like if you call new JsonContext(jsonSerializerOptions) it's tied to that instance of JsonContext. We use these serializer options extensively for other serialization and I wanted to leave the default one "untouched".

[eerhardt]

Might be good to add that as a comment.

[eerhardt]

cc @layomia - any concerns here that the JsonSerializerOptions object gets mutated when passed to a new JsonContext?

[layomia]

We have discussed allowing a one to many relationship between one options instance and multiple context instances, which would make using the jsonSerializerOptions itself possible. I'll bring it up in the next review so we can close on it.

However, it would still be optimal to make an options instance immutable (wrt serialization configuration, e.g. PropertyNameCaseInsensitive) once it has been bound with one or more context instances so that we can initialize the metadata reliably (knowing that the options won't change).

[eerhardt]

This looks good to me.

[eerhardt]

@layomia - using JsonSourceGenerationMode.Serialization means we only generate the "fast path" and not the "metadata".

What does this mean when we only support serialization right now, and not deserialization? Does it mean for deserialization it falls back to Reflection.Emit based deserialization?

[layomia]

To use generated code for deserialization and avoiding Ref.Emit today, the setting would need to be JsonSourceGenerationMode.MetadataAndSerialization. When deserializing, the serializer would fallback to generated code that references setters directly.

To make configuration here cleaner, we could change the enum to a flags enum:

[Flags]
public enum JsonSourceGenerationMode
{
  Unspecified = 0,
  Metadata = 1,
  Serialization = 2,
  Deserialization = 4, // Future
}

When we have fast-path logic for both read and write, the preferred setting (as needed) for simple scenarios would be Serialization | Deserialization.

[eerhardt]

Do we need this file?

[SteveSandersonMS]

Can we move the ReadJsonSerializerOptions call inside this if block so it's only done once? Super minor detail I know, but helps to clarify that we only respect its value the first time, and it can't change after.

[SteveSandersonMS]

Nice!

[SteveSandersonMS]

@pranavkm Is there already an issue filed about handling this for custom event args?

[layomia]

nit: this is a duplicate of the LOC above. Probably an error case the generator should catch since the mode could be different and the intention would be unclear. We'd throw or use last-one-wins..

[layomia]

It looks like it would be cleaner to have a global JsonSourceGenerationMode option per context:

public class JsonSourceGenerationModeAttribute : JsonAttribute
{
    public JsonSourceGenerationModeAttribute(JsonSourceGenerationMode mode) { }
}

Building on #32374 (comment), we'd have

[JsonSourceGenerationMode(JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
[JsonSerializable(typeof(WebEventDescriptor)]
[JsonSerializable(typeof(EventArgs)]
internal sealed partial class WebEventJsonContext : JsonSerializerContext
{
}

[JsonSerializable] instances with an Unspecified mode would fallback to the context-global mode.

[pranavkm]

That does seem like a better alternative. It feels unusual to want fine-grained control over the serialization behavior on a per-type basis. That said, I imagine most users wouldn't really configure this since the extra IL for serialization isn't really noticeable.

[eerhardt]

Is there a reason you aren't just letting it default the "GenerationMode"?

Suggested change

	[JsonSerializable(typeof(WebEventDescriptor), GenerationMode = JsonSourceGenerationMode.MetadataAndSerialization)]
	[JsonSerializable(typeof(WebEventDescriptor)]