[Problem/Bug]: Race condition registering for WebResourceRequested events when handling NewWindowRequested

RendijsSmukulis commented 11 months ago

What happened?

We found a timing problem similar to script injection, but this time with web resource request handling, and it can be reproduced both with the WebResourceRequested event and the Chromium DevTools Protocol Fetch.requestPaused event.

The essence of the issue is the following:

Assigning a handler to these events before setting CoreWebView2NewWindowRequestedEventArgs.NewWindow has no effect (handlers are not called).
Assigning a handler to these events after setting CoreWebView2NewWindowRequestedEventArgs.NewWindow results in a race condition, because navigation is already started in the background when setting NewWindow, so nothing guarantees that we already have a handler in place when the first web resource requests need to be processed. @gkerenyi has created a small repro app, please find it here with additional information in the readme: https://github.com/gkerenyi/CdpVsNewWindow

Either it should be possible to set up both types of event handlers before setting NewWindow, or navigation should not start until the deferral for NewWindowRequested is completed.

Since the WebResourceRequested event is lacking in functionality compared to CDP Fetch, we especially need the Fetch method to be guaranteed to be ready to process all web requests when starting navigation in a new window.

Importance

Blocking. My app's basic functions are not working due to this issue.

Runtime Channel

Stable release (WebView2 Runtime), Prerelease (Edge Canary/Dev/Beta)

Runtime Version

No response

SDK Version

No response

Framework

WPF

Operating System

Windows 10, Windows 11

OS Version

No response

Repro steps

See the repro app, please find it here with additional information in the readme: https://github.com/gkerenyi/CdpVsNewWindow

Repros in Edge Browser

Not Applicable

Regression

No, this never worked

Last working version (if regression)

No response

AB#47964385

pdolanjski commented 8 months ago

@nishitha-burman, as discussed, this issue is causing widespread problems on websites and we'd very much appreciate some help on resolving it. Here are a few examples:

In the seller portal on Etsy.com, hitting Print on the website attempts to load the orders in a new tab which errors out because of the race condition mentioned above.
Attempting to view statements on bankofamerica.com ("Last Statement" and then "View PDF") results in the same situation as the Etsy issue.
Attempting to log in using a Google account on tradingview.com results in a JS error: CSRF check failed (same root cause as above).
Clicking Pins saved on pinterest.com doesn't properly load (same root cause).
On https://oceanofpdf.com/authors/himanshu-agrawal/pdf-epub-kubernetes-fundamentals-a-step-by-step-development-and-interview-guide-download/?id=002005380764, clicking on the red PDF icon down the page should result in a PDF download being triggered, but it doesn't load properly due to this issue.

You can see the examples by trying the above use cases in our browser: duckduckgo.com/windows. @yildirimcagri-msft , let us know if it would be helpful to step through the issue live.

monica-ch commented 8 months ago

@RendijsSmukulis @pdolanjski To intercept requests from iframes, use AddWebResourceRequestedFilter with CoreWebView2WebResourceRequestSourceKinds param which should be available in release package soon. And make sure to use the filter once the new window is set.

I am currently investigating on race condition issue and will update the thread once I have a fix ready.

RendijsSmukulis commented 8 months ago

@monica-ch thank you for investigating this!

While the repro uses both AddWebResourceRequestedFilter and Chrome DevTools Protocol's Fetch API, our actual application only relies on the CDP's Fetch API.

Let us know if we can help with testing the fix in Canary or similar.

pdolanjski commented 7 months ago

Hi @monica-ch , have you had any luck with the investigation?

monica-ch commented 6 months ago

Hi everyone, we fixed the issue to allow apps to run cdp messages by calling CallDevToolsProtocolMethod before new window is set in Edge Canary version 125.0.2505.0 and above.

webView.CoreWebView2.NewWindowRequested += (sender, args) =>
{
      // Apply cdp messages to new window before navigation begins
      // if the messages need to apply for new window navigation.
      await newWebview.CoreWebView2.CallDevToolsProtocolMethodAsync(methodName);
      args.NewWindow = newWebView.CoreWebView2;
      args.Handled = true;
};

Please validate and let us know if you have any questions.

pdolanjski commented 6 months ago

Please validate and let us know if you have any questions.

Thanks @monica-ch. We're working on validating against a number of examples, so we'll get back to you soon.

gkerenyi commented 6 months ago

We have tested the changes in our app and can confirm it is working as expected. @monica-ch thank you for investigating and fixing this!

MicrosoftEdge / WebView2Feedback