[wasm] Initial addition of Browser WebAssembly support

[kjpou1]

Initial file support for Interop and Http handlers.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

[stephentoub]

Thanks for starting to work on this. I skimmed it and left a few comments. I'll comment in more depth when it's further along and the initial round of feedback has been addressed. Thanks!

[jkotas]

The CI failures seem to be related to the change. The CI needs to be green before this is merged.

All my concerns are addressed.

I would like to see somebody from the networking team to sign-off on this as well.

[stephentoub]

I'll take another pass through it today.

[stephentoub]

Now that _underlyingHandler could be something other than a SocketsHttpHandler, this set of checks will probably need to be updated to validate that the instance retrieved from the field actually is a SocketsHttpHandler and bailing if it's not. That could be left for later, but if we start running the System.Net.Http tests against the webassembly implementation, this will likely cause problems (then again, I expect a ton of tests wouldn't pass, anyway).

[stephentoub]

Doesn't need to be in this PR, but it might be worth refactoring this subsequently to avoid the explicit static cctor, e.g.

private static bool StreamingSupported { get; } = GetIsStreamingSupported();

private static bool GetIsStreamingSupported()
{
    using (var streamingSupported = new Function("return typeof Response !== 'undefined' && 'body' in Response.prototype && typeof ReadableStream === 'function'"))
        return (bool)streamingSupported.Call();
}

[stephentoub]

May not matter, but this can be made to work, e.g.

private Dictionary<string, object?>? _properties;
public IDictionary<string, object?> Properties => _properties ??= new Dictionary<string, object?>();

It's fine if you want to punt on that for now, given that most everything else throws PNSE and there's little reason right now for someone to be storing data into properties.

[stephentoub]

Nit: the sealed here isn't necessary, as the whole class is sealed

[stephentoub]

Nit: you might want to override BeginRead/EndRead as well; if you don't and someone uses them, they'll end up getting an exception about synchronous reads not being supported, as by default the base implementations just queue a work item that wraps the sync method. You can implement these pretty easily with an internal helper that should already be available in this library, e.g. something like:

public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsynCallback? callback, object? state) =>
    TaskToApm.Begin(ReadAsync(buffer, offset, count), callback, state);

public override int EndRead(IAsyncResult asyncResult) => TaskToApm.End<int>(asyncResult);

Search for BeginRead/EndRead and you should find other examples in this project.

[stephentoub]

Am I reading this correctly that if someone reads to the end but then calls read again (such that _position >= _bufferedBytes.Length), they'll end up starting to read again at the beginning because they'll fail this if check and proceed to the below try? If so, that seems wrong.

[stephentoub]

I still see all of these custom messages being passed to PNSE.

[stephentoub]

These ifdefs are ok for now. It'd be good to see if we can find a cleaner solution subsequently.

[stephentoub]

Nit: missing a newline at the end of the file

[stephentoub]

Nit:

Suggested change

[stephentoub]

Nit: some of these members have blank lines between them, some don't

[stephentoub]

Nit:

Suggested change

[stephentoub]

Nit:

Suggested change

[stephentoub]

Nit:
Another extra blank line. I'll stop commenting on these, but there look to be a few more.

Suggested change

[stephentoub]

Seems a little strange to be assigning a new object to Key here. Is that intentional?

[stephentoub]

What's the plan for all these TODOs? Is there an issue tracking whatever needs to be done here?

[kjpou1]

Will follow up with PR to address the other points.