Closed
Bug 1359129
Opened 7 years ago
Closed 7 years ago
Crash in IPCError-browser | PDocAccessibleParent::AddChildDoc binding to nonexistant proxy!
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: jimm, Assigned: handyman)
References
Details
(Keywords: crash, Whiteboard: aes+)
Crash Data
Attachments
(1 file, 1 obsolete file)
Back again for a second showing. https://crash-stats.mozilla.com/signature/?date=%3C2017-04-24T17%3A15%3A55%2B00%3A00&date=%3E%3D2017-04-17T17%3A15%3A55%2B00%3A00&product=Firefox&version=55.0a1&signature=IPCError-browser%20%7C%20PDocAccessibleParent%3A%3AAddChildDoc%20binding%20to%20nonexistant%20proxy%21
Comment 1•7 years ago
|
||
I can reproduce the crash: Reproducible: 100% Steps To Reproduce: 1. Open https://www.minhembio.com/forum/index.php?showforum=40 and wait to complete page loading 2. Click a subject link and wait to complete page loading 3. Navigation Back 4. Repeat from Step.2 in several times Actual Results: Win10 64bit Nightly 64bit : bp-056b13e8-6533-464f-9ab9-a75650170425 Win10 64bit Nightly 32bit : bp-3a413e69-964d-45b7-ace3-cd10a0170425
status-firefox55:
--- → affected
status-firefox57:
affected → ---
Comment 2•7 years ago
|
||
[Tracking Requested - why for this release]: reproducible crash And also reproduced on 54.0b2 with e10s: bp-71cc380f-9413-4fd4-84ff-a9c070170425
status-firefox54:
--- → affected
tracking-firefox54:
--- → ?
Reporter | ||
Comment 3•7 years ago
|
||
(In reply to Alice0775 White from comment #2) > [Tracking Requested - why for this release]: reproducible crash > > And also reproduced on 54.0b2 with e10s: > bp-71cc380f-9413-4fd4-84ff-a9c070170425 Alice, do you have e10s forced on while using an IME / accessibility client? This crash shouldn't happen in beta 54 since a11y activation should disabled e10s.
Flags: needinfo?(alice0775)
Comment 4•7 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #3) > (In reply to Alice0775 White from comment #2) > > [Tracking Requested - why for this release]: reproducible crash > > > > And also reproduced on 54.0b2 with e10s: > > bp-71cc380f-9413-4fd4-84ff-a9c070170425 > > Alice, do you have e10s forced on while using an IME / accessibility client? > This crash shouldn't happen in beta 54 since a11y activation should disabled > e10s. Yes, ATOK2016 activates a11y. And It happens when forcibly enabled e10s.
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → davidp99
Assignee | ||
Comment 5•7 years ago
|
||
We sometimes briefly have more than one root DocAccessible associated with a TabChild, for example, while navigating links in a page. This patch makes sure that we use the correct accessible (the most recent one) when delaying messages that we forward to the parent process. Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb89336fc434d18c1cb4b30bf6beef34845b46c6
Assignee | ||
Updated•7 years ago
|
Attachment #8862067 -
Flags: review?(aklotz)
Comment 6•7 years ago
|
||
Comment on attachment 8862067 [details] [diff] [review] Use the most recent RootDocAccessible when delaying DocAccessibleChild messages Review of attachment 8862067 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nits addressed. ::: accessible/ipc/win/DocAccessibleChild.cpp @@ +100,5 @@ > auto tabChild = static_cast<dom::TabChild*>(Manager()); > if (!tabChild) { > return; > } > + topLevelIPCDoc = nit: please put a blank line between the end of the above if block and this line. ::: dom/ipc/TabChild.cpp @@ +401,5 @@ > , mLayerObserverEpoch(0) > #if defined(XP_WIN) && defined(ACCESSIBILITY) > , mNativeWindowHandle(0) > #endif > + , mTopLevelDocAccessibleChild(nullptr) Please put inside a #ifdef ACCESSIBILITY block ::: dom/ipc/TabChild.h @@ +674,5 @@ > already_AddRefed<nsISHistory> GetRelatedSHistory(); > > mozilla::dom::TabGroup* TabGroup(); > > + void SetTopLevelDocAccessibleChild(PDocAccessibleChild* aTopLevelChild) Can you put these two methods inside an #ifdef ACCESSIBILITY block? @@ +678,5 @@ > + void SetTopLevelDocAccessibleChild(PDocAccessibleChild* aTopLevelChild) > + { > + mTopLevelDocAccessibleChild = aTopLevelChild; > + } > + PDocAccessibleChild* GetTopLevelDocAccessibleChild() Nit: Please add a blank line between the end of the previous method definition and this one. @@ +850,5 @@ > // The handle associated with the native window that contains this tab > uintptr_t mNativeWindowHandle; > #endif // defined(XP_WIN) > > + PDocAccessibleChild* mTopLevelDocAccessibleChild; Please put inside a #ifdef ACCESSIBLITY block
Attachment #8862067 -
Flags: review?(aklotz) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Includes aklotz's comments.
Attachment #8862067 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2a2e5e6b3091 Use the most recent RootDocAccessible when delaying DocAccessibleChild messages. r=aklotz
Keywords: checkin-needed
Comment 9•7 years ago
|
||
Comment on attachment 8863489 [details] [diff] [review] Use the most recent RootDocAccessible when delaying DocAccessibleChild messages (r=aklotz) >@@ -1476,16 +1476,19 @@ DocAccessible::DoInitialUpdate() > { > if (nsCoreUtils::IsTabDocument(mDocumentNode)) { > mDocFlags |= eTabDocument; > if (IPCAccessibilityActive()) { > nsIDocShell* docShell = mDocumentNode->GetDocShell(); > if (RefPtr<dom::TabChild> tabChild = dom::TabChild::GetFrom(docShell)) { > DocAccessibleChild* ipcDoc = new DocAccessibleChild(this, tabChild); > SetIPCDoc(ipcDoc); >+ if (IsRoot()) { the only thing that goes through here is top level documents, so that shouldn't be necessary, and really these top level documents shouldn't be RootAccessibles. >+ topLevelIPCDoc = >+ static_cast<DocAccessibleChild*>(tabChild->GetTopLevelDocAccessibleChild()); seems like TabChild could store it as a DocAccessibleChild*, there may be a couple other places that need to account for this case, but I think they may be on the parent side and probably "work" fineish anyway. I think this may be correct, but another way of dealing with this that is perhaps more obviously correct is to just walk up the tree of accessible documents until we find a root and use that.
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2a2e5e6b3091
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
status-firefox53:
--- → disabled
status-firefox-esr52:
--- → disabled
You need to log in
before you can comment on or make changes to this bug.
Description
•