Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Proxy requests and round trip JavaScript invocations #43

Merged
merged 10 commits into from
Mar 11, 2024
Merged

Proxy requests and round trip JavaScript invocations #43

merged 10 commits into from
Mar 11, 2024

Conversation

rbrundritt
Copy link
Contributor

This pull request is in relation to #42

All existing capabilities and API interface continue to work the same as before. This is an additive pull request.

New features this pull request adds:

  • Framework for intercepting web requests (proxy) from WebView in a .NET method and allow the user to send a response.
  • Round trip .NET method invoke capability (reverse of InvokeJsMethodAsync). You can now call a .NET method from JavaScript (with or without parameters) and get a response back from .NET to the calling JavaScript function. This is an async feature since you have to wait for the response. Due to WebView limitations of some platforms, input data is passed as an encoded query string parameter. This will likely mean there is a size limit on input data from the JavaScript side. The result is passed as a JSON stream in the response body, so that should allow massive amounts of data to be sent from .NET to JavaScript if needed.
  • Added a bunch of samples in the app that leverages these features. Including an interactive map that loads map tiles from an offline Sqlite DB.
  • Working and tested in Windows and Android. All features working well, with great performance.
  • Code added for iOS and Mac, but untested due to lack of dev environment. It looks like the code is similar to the Android implementation, so I'm fairly confident it works.

All code has been documented, and the Readme updated accordingly.

@rbrundritt
Add support for proxying
ae12965 
Added support in Windows and Android for proxying web requests from the browser through native code to allow for modifying the request, and creating custom responses.
@rbrundritt
Add proxy support for iOS and Mac
2173725 
Add proxy support for iOS and Mac. Cleaned up some of the previous proxy code for Android and Windows.
@rbrundritt
Add round trip invoke JS-.NET-JS
f400de0 
Add round trip invoke JS-.NET-JS by leveraging the proxy framework that was created.
@rbrundritt
Bug fix and code cleanup
a89ea61 
- Bug fix: expose method to get proxy origin in JS. iOS/Mac has a different URL scheme than Android/Windows. Simple helper method gives users JS code the correct URL to the proxy service for the platform the web view is running on.
- A few improvements to better handle errors.
- Document code
- Readme documentation
- Move async invoke samples to methodinvoke.html sample
@HelloooJoe
Copy link

HelloooJoe commented Feb 21, 2024 •
edited
Loading

I'm still very patiently waiting for this to be approved. lol @Eilon

p.s. after the merge conflicts are resolved. There weren't any 2 weeks ago when it was submitted.

@hansmbakker hansmbakker mentioned this pull request Feb 29, 2024
Closed
@Eilon
Copy link
Owner

Eilon commented Mar 7, 2024

Looking at this now... this is very cool!

@Eilon
Copy link
Owner

Eilon commented Mar 7, 2024

Here are my initial notes, and I pushed a change to the PR that addresses some of my comments:

  • Renamed event to ProxyRequestReceived to match .NET event naming patterns (the event names itself shouldn't have On in it)
  • The note about currently only hard-coding a few known extensions: definitely agree this needs to be better. I just added a few common extensions, but ultimately it needs to be a more complete built-in list (like BlazorWebView has), and ideally customizable.
  • Some tweaks around how the paths are processed: at least in some cases (e.g. Windows) the relativePath of the request is already known, so some code can be removed/simplified.
  • Various small code tweaks (removing some unnecessary casts/checks)
  • TODO: Rename the proxy prefix to _proxy. Usually an underscore is used to differentiate 'built-in' features, which this would be.
  • TODO: Figure out how to avoid calling .Wait() on Tasks. We're currently dealing with some bugs in BlazorWebView where this came back to bite us with hangs, so I want to avoid it here. I think it's easy to fix be delaying calls to .DidFinish() or equivalent APIs, but I haven't tested it yet.
  • TODO: Will try to replace new GetKeyValuePairs() method with a built-in .NET API for parsing query string values
  • TODO: Need to find out where pictures.zip is from so that we can add proper attribution for it
  • TODO: Fully test all 4 platforms (I've mostly been running on Windows, and just a bit of Android)
  • TODO: Look more closely at the actual proxy C# and JS code. I did look at it, but of course it's the biggest part of the change, so I want to look at it a bit more in depth.

@rbrundritt thank you so much for this PR! This looks fantastically cool and seem to work great! Please let me know if you have any thoughts on my comments so far. You can see just my update if you look at this commit: 69ede5a

@rbrundritt
Copy link
Contributor Author

The picture in pictures.zip I grabbed from wikimedia. I don't recall any attributions being required (main reason I grabbed a random image from there).

Like your improvements. Let me know if you have any questions on my code.

@Eilon
Copy link
Owner

Eilon commented Mar 9, 2024

Thank you again @rbrundritt . I just pushed another small change and have this further though on the Proxy code itself (especially pertaining to the Async __ajax part):

  • TODO: Consider changing the new JS method SendInvokeMessageToDotNetAsync to be another message type (we already have message type 0 (raw) and 1 (invoke method). It could then internally uses the new proxy mechanism, but perhaps not get mixed into the 'first-class' proxy feature? (This could make the actual proxy code a bit cleaner maybe?)

Regarding the totally of my comments, I'll get started working on addressing all of these.

I think most of my proposed changes don't affect anything publicly visible.

The two things that would affect publicly visible behavior:

  • Rename /proxy to something like /_proxy
  • Consider any necessary async API changes. I'm not sure if this would necessitate any changes, but it could. We really have to avoid calling .Wait() because it really does cause deadlocks.

@Eilon Eilon mentioned this pull request Mar 11, 2024
Open
@Eilon
Copy link
Owner

Eilon commented Mar 11, 2024

Alright, I tested everything on iOS (Simulator) and MacCat and I good news / bad news / medium news:

  • The good news: Most stuff works as expected in the sample apps
  • The bad news: The call to .Wait() hangs 100% of the time on iOS/MacCat 😿
  • The medium news: The hang was predicted in my earlier comment, so I wasn't terribly surprised 😁

So I think after an additional quick review I'll still merge it in, and we'll track the follow-up items in a separate issue: #51

@Eilon
Squashed commit of the following:
ec0c8b7 
commit 6bb1c5f
Author: Eilon Lipton <[email protected]>
Date:   Mon Mar 11 11:53:17 2024 -0700

    Update HybridWebViewHandler.MacCatalyst.cs

commit 3b56ebe
Author: Eilon Lipton <[email protected]>
Date:   Mon Mar 11 11:43:19 2024 -0700

    Try to make iOS proxy code async
@Eilon
Copy link
Owner

Eilon commented Mar 11, 2024

It turns out this was easier to fix than I thought! All I had to do was simply make the code async! And the call to .DidFinish() after the async call worked as I hoped it would.

Here's the async call (without .Wait()): https://github.com/Eilon/MauiHybridWebView/pull/43/files#diff-b4fc8e93eee00c8c72fdbb7f72c38f36a3b5521687754a84260813ea5e6abb05R134

And the original (unchanged) .DidFinish() seals the deal: https://github.com/Eilon/MauiHybridWebView/pull/43/files#diff-b4fc8e93eee00c8c72fdbb7f72c38f36a3b5521687754a84260813ea5e6abb05R91

So now it all works great on iOS and MacCat!

I'll leave #51 open because I want to check how to fix Android as well because it's still using .Wait() though I haven't seen it hang (yet).

@Eilon Eilon mentioned this pull request Mar 11, 2024
Open
3 tasks
@Eilon
Copy link
Owner

Eilon commented Mar 11, 2024

And I logged #52 for some of the other follow-up tasks.

@Eilon Eilon merged commit 2c777e2 into Eilon:main Mar 11, 2024
2 checks passed
@Eilon Eilon linked an issue Mar 11, 2024 that may be closed by this pull request
Feature requests: Proxy requests #42
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature requests: Proxy requests
3 participants
@rbrundritt @HelloooJoe @Eilon