Add support for async .NET methods #25967
Conversation
mattleibow
requested review from
rmarinho,
jsuarezruiz,
Eilon and
tj-devel709
and removed request for
rmarinho and
jsuarezruiz
| (var contentBytes, contentType) = InvokeDotNet(invokeQueryString); | ||
| var contentBytes = await InvokeDotNetAsync(invokeQueryString); |
There was a problem hiding this comment.
Windows is super easy, it is already async so I just added the await here.
| if (dotnetReturnValue is Task task) // Task and/or Task<T> | ||
| { | ||
| await task; | ||
|
|
||
| // Task<T> | ||
| if (dotnetMethod.ReturnType.IsGenericType) | ||
| { | ||
| var resultProperty = dotnetMethod.ReturnType.GetProperty(nameof(Task<object>.Result)); | ||
| return resultProperty?.GetValue(task); | ||
| } | ||
|
|
||
| // Task | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This is the new logic for detecting types.
| var responseData = GetResponseBytes(url); | ||
| var (bytes, contentType, statusCode) = await GetResponseBytesAsync(url); |
There was a problem hiding this comment.
iOS is also easy with async since the APIs are designed to be called in parts.
| var fullUri = new Uri(fullUrl!); | ||
| var invokeQueryString = HttpUtility.ParseQueryString(fullUri.Query); | ||
| var task = Handler.InvokeDotNetAsync(invokeQueryString); | ||
| return new WebResourceResponse("application/json", "UTF-8", 200, "OK", GetHeaders("application/json"), new DotNetInvokeAsyncStream(task)); |
There was a problem hiding this comment.
Android is the difficult one, but it seems that the recommendation is to not actually do work in ShouldInterceptRequest but instead do it as part of the response stream reading. I moved the await code into a new stream type: DotNetInvokeAsyncStream
| @@ -147,5 +148,116 @@ internal void Disconnect() | |||
| { | |||
| _handler.SetTarget(null); | |||
| } | |||
|
|
|||
| private class DotNetInvokeAsyncStream : Stream | |||
There was a problem hiding this comment.
I probably over engineered this, but I am using pipes to allow myself to read and write. I probably could simplify this and just block, but this appears to work.
|
|
||
| var bytesRead = 0; | ||
|
|
||
| var readResult = _pipe.Reader.ReadAsync().ConfigureAwait(false).GetAwaiter().GetResult(); |
| StartReading(); | ||
| } | ||
|
|
||
| private async void StartReading() |
There was a problem hiding this comment.
This async void concerns me if the stream throws. I'm imagining a scenario where the method being invoked throws an exception from a property getter and then this crashes the whole process or something?
| @@ -137,82 +138,90 @@ public SchemeHandler(HybridWebViewHandler webViewHandler) | |||
|
|
|||
| [Export("webView:startURLSchemeTask:")] | |||
| [SupportedOSPlatform("ios11.0")] | |||
| public void StartUrlSchemeTask(WKWebView webView, IWKUrlSchemeTask urlSchemeTask) | |||
| public async void StartUrlSchemeTask(WKWebView webView, IWKUrlSchemeTask urlSchemeTask) | |||
Description of Change
This PR makes the call from JS into .NET be able to support async methods (return
TaskorTask<T>)This PR has a fair bit of things moving around because of the async and the fact that I wanted to make the code flow the same on all the platforms. The code was getting a bit nested, so I broke things out into methods.
Issues Fixed
Fixes #25968
This pull request includes several changes to improve the handling of asynchronous operations and simplify the codebase for
HybridWebView. The most important changes include the addition of new methods for handling asynchronous tasks, refactoring existing methods to use these new async methods, and updating the test cases accordingly.Asynchronous Handling Improvements:
src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.cs: RefactoredInvokeDotNetmethod toInvokeDotNetAsyncand added support for async invocation of .NET methods from JavaScript.src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Windows.cs: AddedGetResponseStreamAsyncmethod to handle async web resource requests and updatedOnWebResourceRequestedmethod to use this new async method. [1] [2]src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.iOS.cs: UpdatedStartUrlSchemeTaskmethod to handle async responses and addedGetResponseBytesAsyncmethod for asynchronous byte retrieval. [1] [2]Test Case Updates:
src/Controls/tests/DeviceTests/Elements/HybridWebView/HybridWebViewTests.cs: Added new test methods to handle async invocations and updated existing test methods to use the newNewComplexResultproperty. [1] [2]src/Controls/tests/DeviceTests/Resources/Raw/HybridTestRoot/invokedotnettests.html: Simplified the test HTML file by removing specific cases and using a default case for invoking .NET methods. [1] [2]Codebase Simplification:
src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Windows.cs: Removed unnecessaryGetHeaderStringmethod and cleaned up theOnWebResourceRequestedmethod.src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.iOS.cs: Simplified the response handling in theStartUrlSchemeTaskmethod.