Crash [@ nsAccessible::VisibilityState] when closing a tab, depending on when accessibility was enabled

RESOLVED FIXED in mozilla12

Status

()

Core
Disability Access APIs
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: davidb)

Tracking

(Blocks: 1 bug, {crash, regression, testcase})

Trunk
mozilla12
crash, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 587927 [details]
testcase (requires extension for timing)

This testcase requires https://www.squarefree.com/extensions/domFuzzLite3.xpi, but I think it's possible to trigger the bug with a less insane timing sequence.
(Reporter)

Comment 1

6 years ago
Created attachment 587928 [details]
stack trace

Comment 2

6 years ago
It first appeared in 12.0a1/20120111.
The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c713003d3226&tochange=e79ef0ffcb09

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsAccessible%3A%3AVisibilityState()
https://crash-stats.mozilla.com/report/list?signature=nsAccessible%3A%3AVisibilityState
Crash Signature: [@ nsAccessible::VisibilityState] → [@ nsAccessible::VisibilityState] [@ nsAccessible::VisibilityState()]
Keywords: testcase
OS: Linux → All
Hardware: x86_64 → All

Comment 3

6 years ago
I haven't seen this crash myself on Windows using NVDA 2011.3.
(Assignee)

Updated

6 years ago
Assignee: nobody → bolterbugz
(Assignee)

Comment 4

6 years ago
I'm trying to recreate on windows. Jesse do I just keep pressing the button that opens another tab and closes it? (I have the fuzzer extension)

Comment 5

6 years ago
(In reply to David Bolter [:davidb] from comment #4)
> I'm trying to recreate on windows.
Windows crashes are restricted to 64-bit builds:
https://crash-stats.mozilla.com/report/list?signature=nsAccessible%3A%3AVisibilityState%28%29
(Assignee)

Comment 6

6 years ago
OK thanks.

amd64 gives me deja vu, but I don't remember the other crash sig.
(Reporter)

Comment 7

6 years ago
(In reply to David Bolter [:davidb] from comment #4)
> I'm trying to recreate on windows. Jesse do I just keep pressing the button
> that opens another tab and closes it? (I have the fuzzer extension)

Just once. If you don't hit the bug the first time, you'll have to restart Firefox before trying again, because "enabling accessibility" is permanent for the process.

Comment 8

6 years ago
It also happens now on 32-bit builds.
(Assignee)

Comment 9

6 years ago
Robert, given the stacks linked in comment 2, do you think this is a bad frame situation (how)?
No, I don't think it is.

VisibilityState() has this code:

  nsAccessible* accessible = this;
  do {
    // We don't want background tab page content to be aggressively invisible.
    // Otherwise this foils screen reader virtual buffer caches.
    roles::Role role = accessible->Role();
    if (role == roles::PROPERTYPAGE || role == roles::PANE)
      break;

    nsIFrame* frame = accessible->GetFrame();
    if (!frame)
      return vstates;

    const nsIView* view = frame->GetView();
    if (view && view->GetVisibility() == nsViewVisibility_kHide)
      return vstates;
    
  } while (accessible = accessible->Parent());

  nsIFrame* frame = GetFrame();
  ...
  const nsSize frameSize = frame->GetSize();

So if 'this' has role PROPERTYPAGE or PANE, we exit the loop immediately, and then if the accessible has no frame, we crash. That's probably your bug.
(Assignee)

Comment 11

6 years ago
Doh!
(Assignee)

Comment 12

6 years ago
Created attachment 590300 [details] [diff] [review]
frame check
Attachment #590300 - Flags: review?(roc)
(Assignee)

Comment 13

6 years ago
Comment on attachment 590300 [details] [diff] [review]
frame check

Robert, we have an IsDefunct check in nsAccessible::State (earlier in the call stack) and at some point after that the frame for the propertypage/pane goes bad it seems.

Do you know where we might be causing frame mutation?

(If it has to do with view's that might go away with Timonthy's work)
(In reply to David Bolter [:davidb] from comment #13)
> Robert, we have an IsDefunct check in nsAccessible::State (earlier in the
> call stack) and at some point after that the frame for the propertypage/pane
> goes bad it seems.

IsDefunct only checks for null mContent. It's totally possible to have an mContent but no frame.
Attachment #590300 - Flags: review?(roc) → review+

Comment 15

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
> (In reply to David Bolter [:davidb] from comment #13)
> > Robert, we have an IsDefunct check in nsAccessible::State (earlier in the
> > call stack) and at some point after that the frame for the propertypage/pane
> > goes bad it seems.
> 
> IsDefunct only checks for null mContent. It's totally possible to have an
> mContent but no frame.

definitely, the crash happens when a11y events are fired, in case of hide event we are almost guaranteed there's no frame for event target.
(In reply to alexander :surkov from comment #15)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
> > (In reply to David Bolter [:davidb] from comment #13)
> > > Robert, we have an IsDefunct check in nsAccessible::State (earlier in the
> > > call stack) and at some point after that the frame for the propertypage/pane
> > > goes bad it seems.
> > 
> > IsDefunct only checks for null mContent. It's totally possible to have an
> > mContent but no frame.

but, mContent not being null implies we haven't shut the accessible down.  Which is actually the case until after the event is fired which I seem to have forgotten and is a little concerning.

> definitely, the crash happens when a11y events are fired, in case of hide
> event we are almost guaranteed there's no frame for event target.
I wonder why the atk object doesn't already exist if this is a hide event.but if its a hide event why where we trying to create an  atk object?

Comment 17

6 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #16)

> > definitely, the crash happens when a11y events are fired, in case of hide
> > event we are almost guaranteed there's no frame for event target.
> I wonder why the atk object doesn't already exist if this is a hide
> event.but if its a hide event why where we trying to create an  atk object?

good question, that shouldn't happen. since we have a test case then I guess David as assignee could look what happens here. or someone else.
(Assignee)

Comment 18

6 years ago
STR never worked for me.
(Assignee)

Comment 19

6 years ago
(In reply to alexander :surkov from comment #15)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
> > (In reply to David Bolter [:davidb] from comment #13)
> > > Robert, we have an IsDefunct check in nsAccessible::State (earlier in the
> > > call stack) and at some point after that the frame for the propertypage/pane
> > > goes bad it seems.
> > 
> > IsDefunct only checks for null mContent. It's totally possible to have an
> > mContent but no frame.
> 
> definitely, the crash happens when a11y events are fired, in case of hide
> event we are almost guaranteed there's no frame for event target.

Indeed.

I think this patch should land, and I plan to land it shortly. Investigation can still continue by someone who can recreate (as time permits).
(Assignee)

Comment 20

6 years ago
(In reply to alexander :surkov from comment #17)
> (In reply to Trevor Saunders (:tbsaunde) from comment #16)
> 
> > > definitely, the crash happens when a11y events are fired, in case of hide
> > > event we are almost guaranteed there's no frame for event target.
> > I wonder why the atk object doesn't already exist if this is a hide
> > event.but if its a hide event why where we trying to create an  atk object?
> 
> good question, that shouldn't happen. since we have a test case then I guess
> David as assignee could look what happens here. or someone else.

I don't understand the question. I think I need more details.
(Assignee)

Comment 21

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/778cf04d7c55
https://hg.mozilla.org/mozilla-central/rev/778cf04d7c55
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.