Closed Bug 1483911 Opened 6 years ago Closed 6 years ago

Crash in mozilla::ipc::LogicError | mozilla::a11y::PDocAccessibleChild::SendScrollingEvent

Categories

(Core :: Disability Access APIs, defect)

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- verified

People

(Reporter: calixte, Assigned: eeejay)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is report bp-e089ebda-954e-4e54-8289-1cdaf0180816. ============================================================= Top 10 frames of crashing thread: 0 mozglue.dll MOZ_CrashOOL mfbt/Assertions.cpp:33 1 xul.dll mozilla::ipc::LogicError ipc/glue/ProtocolUtils.cpp:309 2 xul.dll mozilla::a11y::PDocAccessibleChild::SendScrollingEvent ipc/ipdl/PDocAccessibleChild.cpp:494 3 xul.dll mozilla::a11y::Accessible::HandleAccEvent accessible/generic/Accessible.cpp:958 4 xul.dll mozilla::a11y::AccessibleWrap::HandleAccEvent accessible/windows/msaa/AccessibleWrap.cpp:1233 5 xul.dll nsEventShell::FireEvent accessible/base/nsEventShell.cpp:46 6 xul.dll void mozilla::a11y::DocAccessible::DispatchScrollingEvent accessible/generic/DocAccessible.cpp:2465 7 xul.dll mozilla::a11y::DocAccessible::ScrollPositionDidChange accessible/generic/DocAccessible.cpp:633 8 xul.dll void mozilla::ScrollFrameHelper::ScrollToImpl layout/generic/nsGfxScrollFrame.cpp:3014 9 xul.dll void mozilla::ScrollFrameHelper::CompleteAsyncScroll layout/generic/nsGfxScrollFrame.cpp:2234 ============================================================= There is 1 crash in nightly 63 with buildid 20180816100035. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1479591. [1] https://hg.mozilla.org/mozilla-central/rev?node=da7e0c710c7d
Flags: needinfo?(eitan)
This is my hypothesis, let's see if Jamie agrees. 1. This patch introduces a new event that is dispatched immediately when a scroll starts. 2. This crash is happening on startup when the event is dispatched but PDocAccessibleChild is not yet constructed. 3. I think this has to do with DocAccessibleChild::ConstructChildDocInParentProcess and how it sends a deferred constructor to the parent. Maybe a way to fix this is to check that ipcDoc has a manager before sending events?
Flags: needinfo?(eitan) → needinfo?(jteh)
(In reply to Eitan Isaacson [:eeejay] from comment #1) > 1. This patch introduces a new event that is dispatched immediately when a > scroll starts. Why would a scroll start so early, though? I guess anything is possible... > 2. This crash is happening on startup when the event is dispatched but > PDocAccessibleChild is not yet constructed. > 3. I think this has to do with > DocAccessibleChild::ConstructChildDocInParentProcess and how it sends a > deferred constructor to the parent. That sounds reasonable. DocAccessibleChild::Send*Event need to be overridden for Windows to push deferred events if the doc isn't yet constructed in the parent process. For example, we have overrides for SendHideEvent, SendRoleChangedEvent, etc. See bug 1314707 for more details. I notice there isn't such an override for SendScrollingEvent, which is probably why this is barfing.
Flags: needinfo?(jteh)
This crashes reliably for me at the end of loading in Google Docs. Does not happen if you load in a background tab.
(In reply to James Teh [:Jamie] from comment #2) > (In reply to Eitan Isaacson [:eeejay] from comment #1) > > 1. This patch introduces a new event that is dispatched immediately when a > > scroll starts. > > Why would a scroll start so early, though? I guess anything is possible... > The stack suggests that this is a focus-induced scroll. Probably some kind of onload.
Comment on attachment 9002621 [details] [diff] [review] Defer scrolling events until after ipc doc construction. r?Jamie Thanks. I really wish we didn't need to repeat all of this boilerplate for deferred events, but such is life.
Attachment #9002621 - Flags: review?(jteh) → review+
Assignee: nobody → eitan
Keywords: checkin-needed
Pushed by dvarga@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/34996338b92f Defer scrolling events until after ipc doc construction. r=Jamie
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
No more crashes since the patch landed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: