[Bug] WebView Navigating Race Condition

[Axemasta]

Description

This issue report is both a bug AND an enhancement. I'm essentially saying that the current functionality should be changed because it is very inflexible. The issue is caused by the way the WebViewRenderer and WebView are interacting with the base platforms, The functionality I am describing in this report is easy to implement in a native app. I would call this a design bug as opposed to a code bug.

The Xamarin.Forms.WebView has the event Navigating. This event passes WebNavigatingEventArgs which contain the property Cancel. When the Navigating event is fired, you can override the navigation event and prevent it from occuring by setting e.Cancel = false.

This is useful functionality if you want to prevent the user from browsing to certain sites. The issue arises when you want to evaluate whether the user can navigate to a url using a background thread. If you subscribe to the WebView.Navigating method your code may look like:

private void OnNavigating(object sender, WebNavigatingEventArgs e)
{
    var allowed = CanUserNavigateToSite(e.Url)

    e.Cancel = !allowed;
}

This code executes no problem, but If I say want to something more complicated like talk to an API to see if the url's domain name is allowed, the code will not work.

private async void OnNavigating(object sender, WebNavigatingEventArgs e)
{
    var allowed = await CanUserNavigateToSiteAsync(e.Url)

    e.Cancel = !allowed;
}

This is caused because at the renderer level, the Navigating event is sent to the forms object to raise, then the value of the args is used to determine whether navigation should be cancelled. If your code can complete fast enough then you can prevent the navigation, if you do some lengthy task (say using async await) then the native code will have already executed before your task completed allowing navigation to the site, even if you didn't want it.

Steps to Reproduce

I've provided a fully coded example

1. Go to my example reproduction repository: https://github.com/Axemasta/WebViewNavigatingIssue
2. Read the README, I have explained the issue, my code and proposed a solution (open to feedback).
3. Pull the code and run the app, on each tab try using google to search for & browse to Facebook. When you are executing the code on the same thread the site access is blocked. When you are executing the code on a background thread the browser navigates before evaluating the site access request thus demonstrating the race condition that exists in the Xamarin.Forms code.

Expected Behavior

Xamarin.Forms.WebView can be prevented from navigating using asynchronous code (WebView.CanNavigate = await _myService.EvaluateCanNavigate("https://google.co.uk")).

Actual Behavior

Xamarin.Forms.WebView navigates before evaluating asynchronous code, which returns after the navigation has occurred.

Basic Information

• Version with issue: 4.8.0.1560 (but all issues appear to be affected)
• Last known good version: ?
• IDE: Visual Studio For Mac 2019 / Visual Studio Professional 2019
• Platform Target Frameworks:
  • iOS: 14.1
  • Android: 9.0
• Android Support Library Version: AndroidX
• Nuget Packages: Out of the box only
• Affected Devices: All

Screenshots

Sample app browsing using both sync & async
Both facebook and twitter should be banned from navigation

Console output when using sync method

Console output when using async method

Reproduction Link

WebViewNavigatingIssue

Workaround

Requires code changes at both the Xamarin.Forms.WebView level and at the native renderer level.

[Axemasta]

I've forked the code and will start my implementation of the code changes, you can follow my progress here.

I understand the changes might need some work etc or may not even be approved, I will be manually updating forms and including the DLL in my project because I absolutely require this functionality and may aswell put this work back into forms when its up to standard!

[Axemasta]

I have found a fairly elegant solution that doesn't add the spaghetti bolietplate that I propse in my sample repo:

Add a new event to WebView: NavigatingAsync which contains the following event args:

public class WebNavigatingAsyncEventArgs : WebNavigationEventArgs
{
	public WebNavigatingAsyncEventArgs(WebNavigationEvent navigationEvent, WebViewSource source, string url) : base(navigationEvent, source, url)
	{
	}

	public Func<Task<bool>> CancelTask { get; set; }
}

Now if the WebView invokes this new event, a view can handle it and pass back the code it wants executing:

public class CoolWebPage : ContentPage
{
    private List<string> _bannedSites { get; } = new List<string>()
    {
        "facebook.com",
        "reddit.com",
        "twitter.com"
    };

    private WebView MyWebView { get; } = new WebView
    {
        Source = "https://google.co.uk"
    };

    public CoolWebPage()
    {
        Content = MyWebView;

        MyWebView.NavigatingAsync += OnNavigatingAsync;
    }

    private void OnNavigatingAsync(object sender, WebNavigatingAsyncEventArgs e)
    {
        e.CancelTask = () => MyLongRunningTask(e.Url);
    }
    
    private async Task<bool> MyLongRunningTask(string url)
    {
        Debug.WriteLine("Starting my long running task");

        await Task.Delay(2000);

        bool urlFriendly = !_bannedSites.Any(b => url.Contains(b));

        Debug.WriteLine($"Finished my long running task - url is friendly? {urlFriendly}");

        return urlFriendly;
    }
}

MyLongRunningTask could be replaced by any code from anywhere, including a view model (providing it is made public).

Now I've only tested so far on iOS but here is what I'm doing in the WKWebViewRenderer:

//DecidePolicy
var asyncArgs = new WebNavigatingAsyncEventArgs(navEvent, new UrlWebViewSource { Url = lastUrl }, lastUrl);
WebView.SendNavigatingAsync(asyncArgs);
_renderer.UpdateCanGoBackForward();

bool canNavigate = true;

try
{
    canNavigate = await asyncArgs?.CancelTask.Invoke();
}
catch
{
    //handle
}

decisionHandler(BoolToActionPolicy(canNavigate));

This way the event handler pattern can be respected whilst affording the developer more flexibility over the controls functionality.

[jfversluis]

See PR