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.

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

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

Closed
RendijsSmukulis opened this issue Nov 20, 2023 · 7 comments
Assignees
Labels
bug Something isn't working tracked We are tracking this work internally.

Comments

@RendijsSmukulis
Copy link

RendijsSmukulis commented Nov 20, 2023

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
Copy link

@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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link

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

@monica-ch
Copy link
Contributor

monica-ch commented Apr 8, 2024

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
Copy link

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
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tracked We are tracking this work internally.
Projects
None yet
Development

No branches or pull requests

6 participants