Crash in mozilla::a11y::HyperTextAccessible::RelationByType(mozilla::a11y::RelationType)

VERIFIED FIXED in Firefox 41

Status

()

defect
--
major
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: jdiggs, Assigned: fredw)

Tracking

(Blocks 1 bug, {crash, regression})

Trunk
mozilla42
Unspecified
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm8+, firefox41+ verified, firefox42+ verified)

Details

(crash signature)

Attachments

(3 attachments)

Steps to reproduce:
1. Launch Nightly
2. Launch the attached accessible event listener in a terminal
3. Go to the location bar, type 'docs.google.com' and press return

Expected results: Nightly would not crash

Actual results: Nightly reliably crashes (see below)

At the moment, I'm afraid I don't have time to hunt down when this was introduced, but I believe fairly recently.

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff2b58ea9 in mozilla::a11y::HyperTextAccessible::RelationByType(mozilla::a11y::RelationType) ()
   from /home/jd/Desktop/firefox/libxul.so

(gdb) bt
#0  0x00007ffff2b58ea9 in mozilla::a11y::HyperTextAccessible::RelationByType(mozilla::a11y::RelationType) ()
    at /home/jd/Desktop/firefox/libxul.so
#1  0x00007ffff2b2f1d3 in UpdateAtkRelation(mozilla::a11y::RelationType, mozilla::a11y::Accessible*, AtkRelationType, _AtkRelationSet*) () at /home/jd/Desktop/firefox/libxul.so
#2  0x00007ffff2b2f713 in refRelationSetCB () at /home/jd/Desktop/firefox/libxul.so
#3  0x00007fffdf36e23e in impl_GetRelationSet () at /lib64/libatk-bridge-2.0.so.0
#4  0x00007fffdf36d07a in handle_message () at /lib64/libatk-bridge-2.0.so.0
#5  0x00007fffef300663 in _dbus_object_tree_dispatch_and_unlock () at /lib64/libdbus-1.so.3
#6  0x00007fffef2f2104 in dbus_connection_dispatch () at /lib64/libdbus-1.so.3
#7  0x00007fffdf13b085 in message_queue_dispatch () at /lib64/libatspi.so.0
#8  0x00007fffeed9fa8a in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#9  0x00007fffeed9fe20 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0
#10 0x00007fffeed9fecc in g_main_context_iteration () at /lib64/libglib-2.0.so.0
#11 0x00007ffff13189df in nsAppShell::ProcessNextNativeEvent(bool) () at /home/jd/Desktop/firefox/libxul.so
#12 0x00007ffff26d143d in nsBaseAppShell::DoProcessNextNativeEvent(bool, unsigned int) () at /home/jd/Desktop/firefox/libxul.so
#13 0x00007ffff1318045 in nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool, unsigned int) ()
    at /home/jd/Desktop/firefox/libxul.so
#14 0x00007ffff12a6863 in nsThread::ProcessNextEvent(bool, bool*) () at /home/jd/Desktop/firefox/libxul.so
#15 0x00007ffff12abbbe in NS_ProcessNextEvent(nsIThread*, bool) () at /home/jd/Desktop/firefox/libxul.so
#16 0x00007ffff12bc6cf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) () at /home/jd/Desktop/firefox/libxul.so
#17 0x00007ffff171bf63 in MessageLoop::Run() () at /home/jd/Desktop/firefox/libxul.so
#18 0x00007ffff26cb9d8 in nsBaseAppShell::Run() () at /home/jd/Desktop/firefox/libxul.so
#19 0x00007ffff2c9f27c in nsAppStartup::Run() () at /home/jd/Desktop/firefox/libxul.so
#20 0x00007ffff2cdcd8a in XREMain::XRE_mainRun() () at /home/jd/Desktop/firefox/libxul.so
#21 0x00007ffff2cdf41b in XREMain::XRE_main(int, char**, nsXREAppData const*) () at /home/jd/Desktop/firefox/libxul.so
#22 0x00007ffff2cdf6ca in XRE_main () at /home/jd/Desktop/firefox/libxul.so
#23 0x0000000000408c96 in do_main(int, char**, nsIFile*) ()
#24 0x00000000004052cb in main ()
Keywords: crash, regression
1. Launch the listener in a terminal (though I'll give you a simpler one shortly)
2. Launch Nightly
3. Having launched Nightly, Ctrl+O and open the attached

As you'll see, window.location.assign() is the trigger. (As you will see in the new listener which I'll attach next, the offending crashing object is the document frame.)

Oh, and the regression was introduced on 06-27.
Posted patch PatchSplinter Review
Assignee: nobody → fred.wang
Status: NEW → ASSIGNED
So the attached patch seems to fix the crash for these testcases.

I think I didn't understand what was asked in bug 1176123 comment 3 regarding the HasOwnContent() check... so I'm adding it back. I'm also adding a verification for parentContent != NULL, although I don't know if that can actually happen.
Attachment #8628885 - Flags: review?(mzehe)
Comment on attachment 8628885 [details] [diff] [review]
Patch

Sorry, but the original review comment leading to the removal of the check in the first place confuses me, too, and I don't know enough about the finer points of this part of code to give a qualified review. Please ask surkov or tbsaunde to see if this is the right fix.
Attachment #8628885 - Flags: review?(mzehe)
Attachment #8628885 - Flags: review?(surkov.alexander)
Comment on attachment 8628885 [details] [diff] [review]
Patch

I'm inclined to give this an r+ after all, since the crashiness is widespread and can hit randomly, too, see bug 1180581, which is probably a dupe of this one.
Attachment #8628885 - Flags: review?(surkov.alexander) → review+
Crash Signature: [@ mozilla::a11y::HyperTextAccessible::RelationByType(mozilla::a11y::RelationType)]
Summary: Reliable segfault in mozilla::a11y::HyperTextAccessible::RelationByType() in Nightly → Crash in mozilla::a11y::HyperTextAccessible::RelationByType(mozilla::a11y::RelationType)
Duplicate of this bug: 1180581
Info from bug 1180581: A page that reproduces this reliably for me is http://www.drwindows.de/windows-10-desktop/85128-updates-ausblenden.html.
Comment on attachment 8628885 [details] [diff] [review]
Patch

I can confirm that this patch fixes the crash in a local build. Frédéric, I think this is good to land.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=075887631fa1

Thanks, do you know if the other crash can be reproduced on other platforms than Windows?
Keywords: checkin-needed
Comment on attachment 8628885 [details] [diff] [review]
Patch

Review of attachment 8628885 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/generic/HyperTextAccessible.cpp
@@ +1910,5 @@
>    Relation rel = Accessible::RelationByType(aType);
>  
>    switch (aType) {
>      case RelationType::NODE_CHILD_OF:
> +      if (HasOwnContent() && mContent->IsMathMLElement()) {

it makes sense to move HasOwnContnet() after Accessible::RelationByType call
[Tracking Requested - why for this release]:
This is the #3 crash for the browser process in 41 Developer Edition, with 3.5% of all browser crashes there.
Adding a tracking flag for FF41 as this was identified as a top crash on Aurora in today's channel meeting.
tracking-e10s: --- → ?
https://hg.mozilla.org/mozilla-central/rev/d92864a3b3ae
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8628885 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: bug 1176123
[User impact if declined]: A lot of crashiness.
[Describe test coverage new/current, TreeHerder]: Manual verification with test case and other scenarios, landed on Central.
[Risks and why]:  Null check.
[String/UUID change made/needed]: None.
Attachment #8628885 - Flags: approval-mozilla-aurora?
Fix confirmed on crash-stats: no results after the 7/7 build.
Comment on attachment 8628885 [details] [diff] [review]
Patch

DMajor confirmed that this crash has gone away since the fix was checked into the nightly build. Seems safe for uplift to Aurora.
Attachment #8628885 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.