Skip to content

Avoid crash in Chrome, issue 41487612 #16893

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.

Sign up for GitHub

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

Merged
seanbudd merged 6 commits into from
Jul 29, 2024
Merged

Avoid crash in Chrome, issue 41487612 #16893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aad11f1
Gecko_ia2 vbufBackend: ensure we release the document root accessible…
michaelDCurran Jul 22, 2024
1709917
Gecko vbufBackend: add assertion
michaelDCurran Jul 23, 2024
9a41e9f
Clarify comment
michaelDCurran Jul 23, 2024
0a151bc
Update what's new
michaelDCurran Jul 23, 2024
bd7b688
lint fixes
seanbudd Jul 28, 2024
13caf00
Update what's new
michaelDCurran Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,12 @@ void GeckoVBufBackend_t::renderThread_initialize() {
void GeckoVBufBackend_t::renderThread_terminate() {
unregisterWinEventHook(renderThread_winEventProcHook);
VBufBackend_t::renderThread_terminate();
// The backend holds a reference to the root accessible of the document.
// This must be specifically released here, in the UI thread where it was created.
// See https://issues.chromium.org/issues/41487612
if (this->rootDocAcc) {
this->rootDocAcc.Release();
}
}

void GeckoVBufBackend_t::render(VBufStorage_buffer_t* buffer, int docHandle, int ID, VBufStorage_controlFieldNode_t* oldNode) {
Expand All @@ -1502,6 +1508,18 @@ GeckoVBufBackend_t::GeckoVBufBackend_t(int docHandle, int ID): VBufBackend_t(doc
}

GeckoVBufBackend_t::~GeckoVBufBackend_t() {
// The backend holds a reference to the root accessible of the document.
// This must be specifically released in the UI thread where it was created.
// See https://issues.chromium.org/issues/41487612
// In most cases this will be released in renderThread_terminate.
// However in the unlikely case terminate can't run,
// we must detach and leak the COM pointer here.
// Otherwise it would be automatically deleted along with the backend which would cause a crash,
// as the COM object would be released from within an RPC worker thread.
nhAssert(!rootDocAcc);
if (this->rootDocAcc) {
this->rootDocAcc.Detach();
}
}

VBufBackend_t* GeckoVBufBackend_t_createInstance(int docHandle, int ID) {
Expand Down
1 change: 1 addition & 0 deletions user_docs/en/changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ A warning message will inform you if you try writing to a non-empty directory. (
* NVDA will correctly announce radio and checkbox menu items when first entering sub-menus in Google Chrome and Mozilla Firefox. (#14550)
* NVDA's browse mode find functionality is now more accurate when the page contains emojis. (#16317, @LeonarddeR)
* In Mozilla Firefox, NVDA now correctly reports the current character, word and line when the cursor is at the insertion point at the end of a line. (#3156, @jcsteh)
* No longer cause Google Chrome to crash when closing a document or exiting Chrome. (#16893)

@CyrilleB79 CyrilleB79 Jul 31, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelDCurran or @seanbudd, I am reviewing the translation of this item.

Translating this sentence sounds a bit strange because grammatically, the subject is missing in this sentence.

Said otherwise, I wonder, what (or who) no longer causes Chrome to crash? Is it NVDA?

Or maybe I haven't understood the sentence correctly...

* NVDA will announce correctly the autocomplete suggestions in Eclipse and other Eclipse-based environments on Windows 11. (#16416, @thgcode)
* Improved reliability of automatic text readout, particularly in terminal applications. (#15850, #16027, @Danstiv)
* NVDA will correctly announce selection changes when editing a cell's text in Microsoft Excel. (#15843)
Expand Down