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)
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)
(deleted),
patch
|
Jamie
:
review+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•6 years ago
|
||
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)
Comment 2•6 years ago
|
||
(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.
Assignee | ||
Comment 4•6 years ago
|
||
Attachment #9002621 -
Flags: review?(jteh)
Assignee | ||
Comment 5•6 years ago
|
||
(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 6•6 years ago
|
||
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 | ||
Updated•6 years ago
|
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
Comment 8•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•