1269369 - [e10s] DOMNodeId accessible proxy is not implemented on Windows.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Yura Zenevich [:yzen] (please use "needinfo")]

It looks like document loaded does not fire when a document is loaded in test context with e10s. It will however fire if remote browser is explicitly reloaded.
The tests that fail are from bug 1186029 and a sample logs with and without e10s are here:
https://pastebin.mozilla.org/8869231
https://pastebin.mozilla.org/8869230

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

This is an issue when testing my a11y+e10s builds.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

David, is there somebody on the a11y team who could look into this?

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Trevor any hunches?
Yura it looks like your logs expired... can you get fresh ones?

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

Here's a sample: https://pastebin.mozilla.org/8913239

Essentially the test just times out because it only starts when doc loaded event (a11y one) comes from the content process. This however never happens, and from what I remember, when talking to Trevor, it's because it fires too early and is never processed in chrome process.

Comment 5

[Yura Zenevich [:yzen] (please use "needinfo")]

Did more investigation. It looks like document in content process is loaded (DOM event) much earlier than the moment a11y is started in the content process. Aaron, perhaps you'd know (off the top of your head) of a reason this only happens on windows: e.g. document in content process loaded before a11y is initialized?

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

I haven't looked much into that, I'm afraid. Most of my stuff is lazily initialized upon the first instantiation of a DocAccessibleChild.

Comment 7

[Trevor Saunders (:tbsaunde)]

(In reply to Yura Zenevich [:yzen] from comment #5)
> Did more investigation. It looks like document in content process is loaded
> (DOM event) much earlier than the moment a11y is started in the content
> process. Aaron, perhaps you'd know (off the top of your head) of a reason
> this only happens on windows: e.g. document in content process loaded before
> a11y is initialized?

Shouldn't ApplicationAccessible::Init() make that work anyway?

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Trevor Saunders (:tbsaunde) from comment #7)
> (In reply to Yura Zenevich [:yzen] from comment #5)
> > Did more investigation. It looks like document in content process is loaded
> > (DOM event) much earlier than the moment a11y is started in the content
> > process. Aaron, perhaps you'd know (off the top of your head) of a reason
> > this only happens on windows: e.g. document in content process loaded before
> > a11y is initialized?
> 
> Shouldn't ApplicationAccessible::Init() make that work anyway?

This doesn't work with e10s in content process, does it? We can't actually enumerate windows when ApplicationAccessible is initialized in content.

Comment 9

[Trevor Saunders (:tbsaunde)]

(In reply to Yura Zenevich [:yzen] from comment #8)
> (In reply to Trevor Saunders (:tbsaunde) from comment #7)
> > (In reply to Yura Zenevich [:yzen] from comment #5)
> > > Did more investigation. It looks like document in content process is loaded
> > > (DOM event) much earlier than the moment a11y is started in the content
> > > process. Aaron, perhaps you'd know (off the top of your head) of a reason
> > > this only happens on windows: e.g. document in content process loaded before
> > > a11y is initialized?
> > 
> > Shouldn't ApplicationAccessible::Init() make that work anyway?
> 
> This doesn't work with e10s in content process, does it? We can't actually
> enumerate windows when ApplicationAccessible is initialized in content.

duh yeah.

That said I think we do need something similar eventually to handle a11y being enabled after session startup.

I'm not really sure why this happens though.  It might be interesting to debug how / when we start the child process on linux / mac and how it happens on windows and compare those.

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

I'll take over the investigation of this.

Comment 11

[Yura Zenevich [:yzen] (please use "needinfo")]

Quick note on this, DOM node ID property is not implemented for proxy accessibles which is part of the problem, even if the events were firing, the test machinery would not recognize that the doc load event is for the expected document.

Comment 12

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 1269369 wip

In case this is helpful (slightly out of date as there's a more useful method nsGlobalWindow method - isTopLevelWindow or something like that). Olli suggested, since in content process windowEnumerator will not give us much to use nsGlobalWindow for these purposes.

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: rebased wip

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

I did tweak the wip to use IsTopLevelWindow.

Comment 15

[Yura Zenevich [:yzen] (please use "needinfo")]

just ni'ing Trevor regardin DOMNodeId stuff for a proxy on Windows.

Comment 16

[Yura Zenevich [:yzen] (please use "needinfo")]

When tests are enabled we should uplift to 52, as per RyanVM

Comment 17

[Trevor Saunders (:tbsaunde)]

(In reply to Yura Zenevich [:yzen] from comment #15)
> just ni'ing Trevor regardin DOMNodeId stuff for a proxy on Windows.

ok, it should be pretty easy to do that on top of the patches in bug 1301148, can you handle that?  And the rest of the com interface stuff ideally?

Comment 18

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: DOMNodeID implementation

This depends on IGeckoCustom stuff landing first.

Comment 19

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8814163 [details] [diff] [review]
DOMNodeID implementation

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

I guess it won't hurt to get a review on this one (still depends on IGeckoCustom  stuff landing.

Comment 20

[(No longer employed by Mozilla) Aaron Klotz]

What's the status of this? I have a sneaking suspicion that bug 1325244 is being caused by the fact that we don't have top-level DocAccessibles immediately after a11y is started.

Comment 21

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Aaron Klotz [:aklotz] from comment #20)
> What's the status of this? I have a sneaking suspicion that bug 1325244 is
> being caused by the fact that we don't have top-level DocAccessibles
> immediately after a11y is started.

OK, I am no longer sure that this is the case. But I'd still like to know where we stand with this bug :-)

Comment 22

[Yura Zenevich [:yzen] (please use "needinfo")]

This is blocked by bug 1301148 ATM, but then I should be able to enable the tests (with the patch attached to this bug) and see where we stand in terms of the e10s browser tests.

Comment 23

[Yura Zenevich [:yzen] (please use "needinfo")]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=93e5f20181baf89b02f17180f3c3c83fd6e6a0c8

Comment 24

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 1269369 rebased

Carrying over the r+

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1f076f55d8c
add support for GetID XPCOM method on Windows with e10s. r=tbsaunde

Comment 26

[(No longer employed by Mozilla) Aaron Klotz]

Yura, I think this bug has been overloaded a bit.

From your perspective, you are mainly concerned about automated tests. OTOH, from the user's perspective, we still have a problem: If there are already documents loaded in an e10s browser before a11y is instantiated, those content accessibles are not visible to the AT after a11y instantiation until the previously existing documents have been reloaded. I think that your obsoleted patch in attachment https://bugzilla.mozilla.org/attachment.cgi?id=8798887 helped with this.

What should we do here? Should that work be forked off to a separate bug?

Comment 27

[Yura Zenevich [:yzen] (please use "needinfo")]

This should be a separate bug, I believe. In fact, it should not be too difficult to make up a test case once the browser tests can run on Windows (once bug 1329697 is taken care of). I'll leave ni? to make sure I open the bug for the remaining issue.

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c1f076f55d8c