nvdaHelperRemote: Use SendMessageCallback instead of SendMessage to execute most cross-thread calls. #6380
Conversation
…tual buffer backend render threads. This is necessary for Firefox multi-process. When we use SendMessage to do this, outgoing cross-process COM calls fail with RPC_E_CANTCALLOUT_ININPUTSYNCCALL; see Mozilla bug 1297549 comment 14. We avoid this by instead using PostMessage and waiting on an event.
|
@michaelDCurran, can you please review? Thanks! |
…ndow, which is used for the in-process Mozilla text code. Fixes problems with editable text fields in Firefox multi-process.
jcsteh
changed the title
|
@michaelDCurran, I updated this to also do this for execInWindow. I've changed the title and body of the PR accordingly. |
| this->renderThread_initialize(); | ||
| }; | ||
| LOG_DEBUG(L"Calling execInWindow"); | ||
| execInWindow((HWND)UlongToHandle(rootDocHandle),func); |
There was a problem hiding this comment.
I'm a little unsure if this is completely safe as it is a call from one instance of the CRT to another, using a c++ class or specific feature. I really have no evidence that it won't work, but just something we should keep an eye on in various operating systems. For instance, we avoided ever passing c++ strings for this reason. At worse
|
As discussed, I tested this with Firefox, Chrome and IE in Windows 10,
as well as Firefox and IE in Windows XP, and did not encounter any
crashes or other instability. Hopefully, this multiple CRT thing won't
be an issue, but if it is, it should get picked up during incubation.
|
|
Would this affect anything else using IE code such as Outlook Express etc? bglists@blueyonder.co.uk
|
|
I don't anticipate that it will break anything, but yes, this new code does get used in anything that uses IE. |
… text box in Mozilla applications. In this case, the window dies before the message is sent, but the return value of PostMessage wasn't being checked. Therefore, we were waiting forever for an event that would never be set. However, it's also possible that the thread might die after the message is sent but before it is handled. Therefore, switch to using SendMessageCallback, which calls the callback with a 0 result if the thread dies. This also fixes a leak, since event handles in the previous code weren't being closed. Fixes #6422.
|
@michaelDCurran, could you please review this again? |
|
Fine with me. |
jcsteh
changed the title
This is necessary for Firefox multi-process. When we use SendMessage to execute calls in the main thread, outgoing cross-process COM calls fail with RPC_E_CANTCALLOUT_ININPUTSYNCCALL; see Mozilla bug 1297549 comment 14. We avoid this by instead using PostMessage and waiting on an event.
Even though this is only relevant to Firefox multi-process right now, this applies to any case where we might call a COM proxy in a cross-thread call.
execInWindow now uses SendMessageCallback. Virtual buffer backends now use execInWindow to initialize/terminate the render thread instead of their own message/hook.