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

wxWebView - wxWebViewBackendEdge - Runscript - get stuck #19075

Closed
wxtrac opened this issue Feb 9, 2021 · 12 comments
Closed

wxWebView - wxWebViewBackendEdge - Runscript - get stuck #19075

wxtrac opened this issue Feb 9, 2021 · 12 comments

Comments

@wxtrac
Copy link
Collaborator

wxtrac commented Feb 9, 2021

Issue migrated from trac ticket # 19075

component: WebView | priority: normal | keywords: wxWebView,wxWebViewBackendEdge

2021-02-09 20:53:48: hiemann (Rico Hiemann) created the issue


Hi,

I used the wxWebView sample from master branch and wrote a little test html, see below.
I noticed different behaviour between wxWebViewBackendEdge vs
wxWebViewBackendIE especially with RunScript.

Using the backend: wxWebViewBackendIE
All links change the images as expected:
InternGrey - grey image - OK
InternRed - red image - OK
ExternGrey - grey image - OK
ExternRed - red image - OK

Than switching to the backend: wxWebViewBackendEdge
(Compilation and installation worked - backend message from log:
wxWebViewEdge Version: Microsoft Edge WebView2 88.0.705)

InternGrey - grey image - OK
InternRed - red image - OK
ExternGrey - change image on first click but function get stuck
ExternRed - change image on first click but function get stuck

The internal C++ part:

void WebFrame::OnTitleChanged(wxWebViewEvent& evt)
{
    wxString Msg = evt.GetString();
    if (Msg == "testexternred")
    {
        RunScript("setImage('red.jpg');");
    }
    else if (Msg == "testexterngrey")
    {
        RunScript("setImage('grey.jpg');");
    }
}

wxWebViewBackendEdge get stuck in line (infinite loop, no error or any response):

if (m_browser->RunScript(javascript, &result))

Thanks in advance
Rico

<html>
  <head>
    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
  </head>
  <body>
<script>
function setImage(content) 
{
	document.getElementById('myimage').setAttribute('src', content);
}
function mytitle(id)
{
	document.title = id;	
}	
</script>  
    <nav id="head">
	  <a href="#" id="testinterngrey" onclick="setImage('grey.jpg')">InternGrey</a>
	  <a href="#" id="testinternred" onclick="setImage('red.jpg')">InternRed</a>
	  <a href="#" id="testexterngrey" onclick="mytitle(this.id)">ExternGrey</a>
	  <a href="#" id="testexternred" onclick="mytitle(this.id)">ExternRed</a>
	</nav>
    <main>
		<img id="myimage" class="center" src="grey.jpg">
     </main>
  </body>
</html>
@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 9, 2021

2021-02-09 20:54:26: hiemann (Rico Hiemann) uploaded file simple.html (0.7 KiB)

Simple.html test script

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 9, 2021

2021-02-09 20:54:52: hiemann (Rico Hiemann) uploaded file red.jpg (0.8 KiB)

red test image
red.jpg

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 9, 2021

2021-02-09 20:55:06: hiemann (Rico Hiemann) uploaded file grey.jpg (0.4 KiB)

grey test image
grey.jpg

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 9, 2021

2021-02-09 22:02:15: @TcT2k changed status from new to confirmed

2021-02-09 22:02:15: @TcT2k commented

Can confirm the issue:
some how the callback is newer called. Maybe because it's called from within a HandleWindowEvent() and uses wxYield() to wait for the callback?

    HRESULT executionResult = m_impl->m_webView->ExecuteScript(javascript.wc_str(), Callback<ICoreWebView2ExecuteScriptCompletedHandler>(
        [&scriptExecuted, &executionResult, output](HRESULT error, PCWSTR result) -> HRESULT
    {
        ...

        scriptExecuted = true;

        return S_OK;
    }).Get());

    // Wait for script exection
    while (!scriptExecuted)
        wxYield();

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 9, 2021

2021-02-09 22:23:04: @vadz commented


How are callbacks dispatched in Edge SDK? Are they executed in the same thread or in a different one?

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 9, 2021

2021-02-09 22:30:56: @TcT2k commented


Replying to [comment:2 vadz]:

How are callbacks dispatched in Edge SDK? Are they executed in the same thread or in a different one?

Same thread as far as I'm aware.

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 9, 2021

2021-02-09 22:45:00: @vadz commented


Then it probably indeed relies on message dispatching working (because I don't know how else could it work). Can we find out which message it uses? If you have a simple example where this does work, it would be enough to run it under debugger, break in the callback and see where is it called from in the stack (yes, it's a pretty brute force method but those have the advantage of usually working...).

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 10, 2021

2021-02-10 23:23:42: @TcT2k commented


Replying to [comment:4 vadz]:

Then it probably indeed relies on message dispatching working (because I don't know how else could it work). Can we find out which message it uses? If you have a simple example where this does work, it would be enough to run it under debugger, break in the callback and see where is it called from in the stack (yes, it's a pretty brute force method but those have the advantage of usually working...).

Seems like it's using WM_USER+1 (0x401).

In this case I think the problem is practically described here:
https://docs.microsoft.com/en-us/microsoft-edge/webview2/concepts/threading-model#reentrancy

If you have an event handler running and begin a message loop, no other event handlers or completion callbacks are able to begin running in a reentrant manner.

So from a callback like in this case title change wxWebViewEdge::RunScriptSync() could not be run because wxWebViewEdgeImpl::OnDocumentTitleChanged() calling HandleWindowEvent() is not yet finished. Maybe the correct solution would be to post the events from the callbacks? but that would only be possible if it's an event that can't be vetoed if I'm not mistaken. Maybe using CallAfter() from within the edge callbacks should be used and that could then call HandleWindowEvent() and free the edge event handler?

@wxtrac
Copy link
Collaborator Author

wxtrac commented Feb 20, 2021

2021-02-20 14:15:49: @vadz commented


There is a very simple workaround for this particular bug as the code could just use CallAfter() in WebFrame::OnTitleChanged(), but generally speaking I don't see how can we solve this inside wx. The best we could do would probably be to detect such situation and trigger an assertion failure with a message saying that the code should use CallAfter().

The only alternative is to indeed do what you propose and use CallAfter() for JS event dispatching, but this might result in unexpected events order and mismatch between the expected and actual DOM state (i.e. by the time you get the event in C++ code the DOM may have already changed) which would certainly be rare, but very unpleasant when/if it does happen, as it would be difficult to reproduce and impossible to debug due to timing issues. So I'd be a bit afraid to do this unconditionally...

@wxtrac
Copy link
Collaborator Author

wxtrac commented Apr 4, 2021

2021-04-04 17:24:00: ancwrd1 (Dmitry Pankratov) commented


I have the same issue calling "RunScript" from the wxEVT_WEBVIEW_LOADED event handler, it hangs in the tight loop.

I tried the approach recommended here with "CallAfter", however it only works after a small delay: if I call "RunScript" immediately inside the CallAfter closure it returns FALSE, if it is called after a tiny sleep then it is ok.

I think it could be some race condition inside Edge backend.

TcT2k added a commit to TcT2k/wxWidgets that referenced this issue Sep 28, 2022
Using HandleWindowEvent() from the WebView2 event callbacks
is a common error source when user code does something blocking
like RunScript() or ShowModal().
Queue the events to prevent this where possible, in
wxEVT_WEBVIEW_NAVIGATING at least warn the developer instead
of hanging silently.

Closes: wxWidgets#22744, wxWidgets#19075
TcT2k added a commit to TcT2k/wxWidgets that referenced this issue Sep 30, 2022
Using HandleWindowEvent() from the WebView2 event callbacks
is a common error source when user code does something blocking
like RunScript() or ShowModal().
Queue the events to prevent this where possible, in
wxEVT_WEBVIEW_NAVIGATING at least warn the developer instead
of hanging silently.

Closes: wxWidgets#22744, wxWidgets#19075
TcT2k added a commit to TcT2k/wxWidgets that referenced this issue Oct 1, 2022
Using HandleWindowEvent() from the WebView2 event callbacks
is a common error source when user code does something blocking
like RunScript() or ShowModal().
Queue the events to prevent this where possible, in
wxEVT_WEBVIEW_NAVIGATING at least warn the developer instead
of hanging silently.

Closes: wxWidgets#22744, wxWidgets#19075
vadz pushed a commit that referenced this issue Oct 4, 2022
Using HandleWindowEvent() from the WebView2 event callbacks
is a common error source when user code does something blocking
like RunScript() or ShowModal().
Queue the events to prevent this where possible, in
wxEVT_WEBVIEW_NAVIGATING at least warn the developer instead
of hanging silently.

Closes #22834, #22744, #19075
@PBfordev
Copy link
Contributor

PBfordev commented Oct 6, 2022

This Issue should probably be either closed or labeled for backport.

@vadz
Copy link
Contributor

vadz commented Dec 6, 2022

I don't know this code well enough to be confident in backporting this correctly, so I won't do it, even though I think it would indeed be useful to have this in 3.2, so if anybody is interested in doing this, please create a PR for 3.2 doing this.

@vadz vadz closed this as completed Dec 6, 2022
TcT2k added a commit to TcT2k/wxWidgets that referenced this issue Dec 8, 2022
Using HandleWindowEvent() from the WebView2 event callbacks
is a common error source when user code does something blocking
like RunScript() or ShowModal().
Queue the events to prevent this where possible, in
wxEVT_WEBVIEW_NAVIGATING at least warn the developer instead
of hanging silently.

Closes wxWidgets#22834, wxWidgets#22744, wxWidgets#19075

(cherry picked from commit 0ace989)
vadz pushed a commit that referenced this issue Dec 10, 2022
Using HandleWindowEvent() from the WebView2 event callbacks
is a common error source when user code does something blocking
like RunScript() or ShowModal().
Queue the events to prevent this where possible, in
wxEVT_WEBVIEW_NAVIGATING at least warn the developer instead
of hanging silently.

See #22834, #22744, #19075

Closes #23025.

(cherry picked from commit 0ace989)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants