Closed Bug 1585851 Opened 5 years ago Closed 5 years ago

Assertion failure: aNewIndex <= mChildren.Length() (Wrong new index was given), at src/accessible/generic/Accessible.cpp:2219

Categories

(Core :: Disability Access APIs, defect, P1)

70 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 --- wontfix
firefox71 --- fixed
firefox72 --- fixed

People

(Reporter: tsmith, Assigned: Jamie)

References

(Blocks 1 open bug, Regression)

Details

(4 keywords)

Crash Data

Attachments

(2 files)

Attached file testcase.html (deleted) —

Reduced with m-c:
BuildID=20191002163554
SourceStamp=314a0fee08fdb0cbeb9c9023f634ef95375dab9b

Assertion failure: aNewIndex <= mChildren.Length() (Wrong new index was given), at src/accessible/generic/Accessible.cpp:2219

==106154==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7fb54c8ab94a bp 0x7ffeab0c9b10 sp 0x7ffeab0c9ab0 T0)
==106154==The signal is caused by a WRITE memory access.
==106154==Hint: address points to the zero page.
    #0 0x7fb54c8ab949 in mozilla::a11y::Accessible::RelocateChild(unsigned int, mozilla::a11y::Accessible*) src/accessible/generic/Accessible.cpp:2215:3
    #1 0x7fb54c8c96fe in mozilla::a11y::DocAccessible::MoveChild(mozilla::a11y::Accessible*, mozilla::a11y::Accessible*, int) src/accessible/generic/DocAccessible.cpp:2337:16
    #2 0x7fb54c8c8e50 in mozilla::a11y::DocAccessible::ProcessContentInserted(mozilla::a11y::Accessible*, nsTArray<nsCOMPtr<nsIContent> > const*) src/accessible/generic/DocAccessible.cpp:1946:9
    #3 0x7fb54c8313f3 in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:743:16
    #4 0x7fb5499524bd in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1928:12
    #5 0x7fb54996447f in TickDriver src/layout/base/nsRefreshDriver.cpp:373:13
    #6 0x7fb54996447f in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:350
    #7 0x7fb549963e40 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:367:5
    #8 0x7fb549967683 in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:807:5
    #9 0x7fb549967683 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:727
    #10 0x7fb5499669b9 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) src/layout/base/nsRefreshDriver.cpp:622:9
    #11 0x7fb54a25b6db in mozilla::layout::VsyncChild::RecvNotify(mozilla::VsyncEvent const&) src/layout/ipc/VsyncChild.cpp:65:16
    #12 0x7fb541caba75 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:187:54
    #13 0x7fb54175c5cb in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:5876:32
    #14 0x7fb540fc2736 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2185:25
    #15 0x7fb540fbd38d in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2109:9
    #16 0x7fb540fbf9b7 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1954:3
    #17 0x7fb540fc0847 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1985:13
    #18 0x7fb53fd87e79 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1225:14
    #19 0x7fb53fd8eae8 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:486:10
    #20 0x7fb540fcbb1f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:88:21
    #21 0x7fb540ec49e2 in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #22 0x7fb540ec49e2 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #23 0x7fb540ec49e2 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #24 0x7fb5493be619 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #25 0x7fb54d30dabf in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:934:20
    #26 0x7fb540ec49e2 in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #27 0x7fb540ec49e2 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #28 0x7fb540ec49e2 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #29 0x7fb54d30d366 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:769:34
    #30 0x55aed89f5cea in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #31 0x55aed89f5cea in main src/browser/app/nsBrowserApp.cpp:272
Flags: in-testsuite?

Jamie, could this be another fall-out from bug 686400?

Flags: needinfo?(jteh)

I'd say so, yes. I was hoping this might be fixed by bug 1581589 or bug 1582619, but alas not. I get the RelocateChild crash covered by bug 1578185 when I run this test case in a nightly build, so that bug will probably be fixed by this one.

Blocks: 1578185
Flags: needinfo?(jteh)
Regressed by: 1576690
Priority: -- → P1

Morgan, do you want to take a stab at debugging this? This goes deep into our tree building code, so I imagine we'll need to collaborate to fully understand this one, but figuring out what code paths we're failing on here would be an enormously good start.

Assignee: nobody → mreschenberg
Version: unspecified → 70 Branch

Hi! We were looking at our list of regression bugs carried over into 71 and this came up. Do we think we'll be able to get a fix in 71?

Flags: needinfo?(mreschenberg)

Please let me know if a Pernosco session would be helpful and I will create one.

I haven't been able to make much progress on this, so it won't likely land in 71. I would love a Pernosco session, thank you @tsmith :)

Flags: needinfo?(mreschenberg) → needinfo?(twsmith)

A Pernosco session is available here: https://pernos.co/debug/Pwl_z-Koo__dj1JTicv-JQ/index.html
It will automatically expire in 7 days.

Flags: needinfo?(twsmith)

Taking this due to p1 71 crash and Morgan having her plate full.

Assignee: mreschenberg → jteh

Sometimes, depending on how children were changed, children might be in the insertion list out of order; e.g. [child2, child1].
It's also possible that an earlier child (child1 in the above example) is being moved out of another container.
When processing the earlier insertion (child2), we'll determine we need to move it within its parent and will fetch its new previous sibling so we can move it into the right place.
However, in this case, the new previous sibling (child1) will be in the wrong container.
We can't move in that case; the new previous sibling's index in parent will obviously be wrong, since it's relative to the wrong container.
Therefore, we just skip the move.
Since the previous sibling (child1) is later in the insertion list, the ordering will be corrected when we process that insertion.

Pushed by mzehe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/61e813c94bc9 When processing a11y insertions, don't try to move an accessible if its new previous sibling hasn't been moved into its new container yet. r=MarcoZ
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

Please nominate this for Beta approval when you get a chance.

Flags: needinfo?(jteh)
Flags: in-testsuite?
Flags: in-testsuite+
Crash Signature: [@ mozilla::a11y::Accessible::RelocateChild ]
Flags: needinfo?(jteh)

Comment on attachment 9105115 [details]
Bug 1585851: When processing a11y insertions, don't try to move an accessible if its new previous sibling hasn't been moved into its new container yet.

Beta/Release Uplift Approval Request

  • User impact if declined: Crashes.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): A11y correctness fix which is only relevant in obscure circumstances, but avoids an obvious crash when these circumstances occur.
  • String changes made/needed: None.
Attachment #9105115 - Flags: approval-mozilla-beta?

Comment on attachment 9105115 [details]
Bug 1585851: When processing a11y insertions, don't try to move an accessible if its new previous sibling hasn't been moved into its new container yet.

Fix for a frequent a11y crash on beta, patch baked a week on nightly and proved effective and as tests, uplift approved for 71 beta 8, thanks.

Attachment #9105115 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: