Duplicate entries for first Root Window with ATSPI2

RESOLVED FIXED in mozilla2.0b4

Status

()

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)

Tracking

({regression})

Trunk
mozilla2.0b4
x86
Linux
regression
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
With ATSPI2, we can see 2 duplicate entries under Firefox root accessible in accercicer.

In debugger, if we do
print atk_object_get_n_accessible_children(atk_get_root())
we get 2.

So the bug is in our side.

It is a regression of Bug 560239.

It may crash Firefox if we open second browser window and close the first one.
(with accercier running)

So ask blocking 2.0.

Although it is only reproducible with ATSPI2, but I think perhaps it can also reproducible with ATSPI if GTK+ library or Gecko library slightly changes.
Maybe even on Windows.
(Assignee)

Comment 1

8 years ago
Created attachment 461509 [details]
stack

The stacks for adding duplicate entries.

It looks like we're creating accessible for the same document twice.

Comment 2

8 years ago
Ginn, can you work on this?
(Assignee)

Comment 3

8 years ago
oops, I should use "where" instead "where 20" in dbx, the 2 creating are actually on the same stack.

So, actually the problem is when we do CreateDocOrRootAccessible() for the first time, we get into nsApplicationAccessibleWrap::Init(), and it will re-enter CreateDocOrRootAccessible().

Macro, yes, I can work on this.
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
(Assignee)

Comment 4

8 years ago
Created attachment 461512 [details]
the stack shows we re-enter CreateDocOrRootAccessible()
Attachment #461509 - Attachment is obsolete: true
(Assignee)

Comment 5

8 years ago
Created attachment 461514 [details] [diff] [review]
patch

This will break the re-enter.
Attachment #461514 - Flags: review?(surkov.alexander)
Comment on attachment 461512 [details]
the stack shows we re-enter CreateDocOrRootAccessible()

This stack is worrisome.

>  [8] atk_object_get_n_accessible_children(0x8b108f0, 0x0), at 0xf926b89d 
>  [9] append_children(0x8b108f0, 0x8b29850), at 0xf920fe7c 
>  [10] add_pending_items(0x8b27908, 0x0), at 0xf9210073 
>  [11] add_subtree(0x8b27908, 0x8b108f0), at 0xf920ff80 

Is this in atspi2? It isn't clear to me why it goes ahead and tries to add children at this point.

>  [12] spi_cache_init(0x8b27908, 0x8dea630, 0x8045b60, 0xfe80c436), at 0xf920f8dd 
>  [13] g_type_create_instance(0x8de5f20, 0xfef5f000, 0x8045be0, 0xfe7f3b91), at 0xfe80c98c 
>  [14] g_object_constructor(0x8de5f20, 0x0, 0x0, 0xfe7f29b9), at 0xfe7f3baa 
>  [15] g_object_newv(0x8de5f20, 0x0, 0x0, 0xfe7f292a), at 0xfe7f2ef5 
>  [16] g_object_new(0x8de5f20, 0x0), at 0xfe7f296c 
>  [17] adaptor_init(0x0, 0x0), at 0xf9212084 
>  [18] gnome_accessibility_module_init(0x8045fb8, 0xfdf0163c, 0xfd219f6c, 0xfe1eba5c, 0x8045e44, 0x0), at 0xf92123a0 
>  [19] nsApplicationAccessibleWrap::Init(this = 0x8b7d140), line 586 in "nsApplicationAccessibleWrap.cpp"
Mike please see comment 6. You expertise welcome here.
blocking2.0: ? → final+
(Assignee)

Comment 8

8 years ago
David, AFAIK, at-spi2 keeps a cache of a11y tree in their side, so it tries to obtain the children information of root accessible at startup (i.e. gnome_accessibility_module_init).
(In reply to comment #8)
> David, AFAIK, at-spi2 keeps a cache of a11y tree in their side, so it tries to
> obtain the children information of root accessible at startup (i.e.
> gnome_accessibility_module_init).

Thanks, yes that seems likely.

at-spi2 0.3.6 changed some initialization... I wonder if it helps this bug. Either way we should take a fix here I think.
Alexander do you have time to review Ginn's patch?
Comment on attachment 461514 [details] [diff] [review]
patch

Sorry for delay. This really looks like a hack. But since we need to get this asap and we don't have anything better then I'm fine with it. Please add a comment for this line.
Attachment #461514 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/mozilla-central/rev/06b1b2e4ed45

Actually, I don't think it is a hack, we should do EnsureChildren() before looking for a child in cache.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
You need to log in before you can comment on or make changes to this bug.