Last Comment Bug 671808 - start windows emulation for multiprocess Firefox
: start windows emulation for multiprocess Firefox
Status: RESOLVED FIXED
[e10s]
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All Windows 7
: -- normal (vote)
: mozilla8
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks: 646596
  Show dependency treegraph
 
Reported: 2011-07-15 00:50 PDT by alexander :surkov
Modified: 2011-07-23 06:40 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (779 bytes, patch)
2011-07-15 00:50 PDT, alexander :surkov
dbolter: review+
Details | Diff | Splinter Review
patch v2 (886 bytes, patch)
2011-07-23 04:51 PDT, alexander :surkov
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description alexander :surkov 2011-07-15 00:50:14 PDT
Having this patch on top of others NVDA works in terms it sees tab document which can be traversed by arrows.
Comment 1 alexander :surkov 2011-07-15 00:50:56 PDT
Created attachment 546114 [details] [diff] [review]
patch
Comment 2 alexander :surkov 2011-07-20 07:27:22 PDT
Jamie, perhaps you should know. This bug (on top of bug 671510 and bug 671550) makes NVDA 2011.1 working (focus and vb traversal), 2011.2b3 announces focus events but fails to arrow the vb (it appears vb thinks he's on previous tab that is in chrome process because it announces something from that page).
Comment 3 David Bolter [:davidb] 2011-07-20 11:51:46 PDT
Comment on attachment 546114 [details] [diff] [review]
patch

r=me. Looks right in principle.
Comment 4 James Teh [:Jamie] 2011-07-20 19:23:46 PDT
(In reply to comment #2)
> Jamie, perhaps you should know. This bug (on top of bug 671510 and bug
> 671550) makes NVDA 2011.1 working (focus and vb traversal), 2011.2b3
> announces focus events but fails to arrow the vb (it appears vb thinks he's
> on previous tab that is in chrome process because it announces something
> from that page).
That is very odd. Any chance of a try build with those three patches?
Comment 5 alexander :surkov 2011-07-20 20:43:26 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > Jamie, perhaps you should know. This bug (on top of bug 671510 and bug
> > 671550) makes NVDA 2011.1 working (focus and vb traversal), 2011.2b3
> > announces focus events but fails to arrow the vb (it appears vb thinks he's
> > on previous tab that is in chrome process because it announces something
> > from that page).
> That is very odd. Any chance of a try build with those three patches?

Yes, I'll to do it
Comment 7 Marco Zehe (:MarcoZ) on PTO until August 15 2011-07-21 05:56:37 PDT
For me, NVDA 2011.1 also does not get a virtual buffer for the second tab in this try-server build. If I tab into the document, the name of the first document (the one from tab 1) is reported to me, but a virtual buffer never becomes active. No, with this try-server build, NVDA is definitely not working for me in multi-process environments yet.
Comment 8 alexander :surkov 2011-07-21 07:11:55 PDT
Just checked the build with NVDA 2011.1.1 and 2011.2b3, I see same result I observed in comment 2.
Comment 9 James Teh [:Jamie] 2011-07-21 16:53:42 PDT
Confirmed (although it's difficult to test because of tabbing problems :)).

The problem is that the first tab document is contained within the chrome HWND instead of its own content HWND. The chrome HWND is naturally the parent of all content process HWNDs. When NVDA checks whether a given object belongs to a buffer, the check succeeds if the object's HWND is a descendant of the document's HWND. In this case, the second tab document's HWND is a descendant of the first document's (chrome) HWND. We do check accChild and the check fails if accChild fails, but only if the HWNDs are equal (not descendants). Btw, the reason for this descendant window stuff is that Gecko <= 1.9.1 rendered frame documents in different HWNDs.

I guess we could add another version specific check. However, I'm curious: why does the first tab document use the chrome HWND? I initially thought all documents would be rendered in their own content processes, so they would all have different HWNDs.
Comment 10 alexander :surkov 2011-07-21 17:55:39 PDT
inbound land: http://hg.mozilla.org/integration/mozilla-inbound/rev/eac851d9d505
Comment 11 alexander :surkov 2011-07-21 18:21:57 PDT
(In reply to comment #9)

> The problem is that the first tab document is contained within the chrome
> HWND instead of its own content HWND.

Got it.

> I guess we could add another version specific check. However, I'm curious:
> why does the first tab document use the chrome HWND? I initially thought all
> documents would be rendered in their own content processes, so they would
> all have different HWNDs.

I'm not sure why it happens, just first tab lives in chrome process.
Comment 12 James Teh [:Jamie] 2011-07-21 19:26:17 PDT
(In reply to comment #11)
> > why does the first tab document use the chrome HWND?
> I'm not sure why it happens, just first tab lives in chrome process.
So the question is: is this intentional or a bug? It seems inconsistent to me and means that a crash in the first tab doc will crash the chrome. If it's a bug, this issue with NVDA will go away once that bug is fixed, so no specific a11y fix is required. If it's intentional, there are two possible solutions:
1. Gecko exposes a fake HWND for the first tab doc anyway, which makes it consistent with all other tab docs; or
2. We add a version specific check in NVDA to disable the descendant window check, since that is only valid for Gecko <= 1.9.1.
Consistency is always nice (which suggests option 1), but I also don't want to get caught in the "legacy" trap. Are there any other cases where a document will be hosted by the chrome process? If so, we should go for option 2.
Comment 13 alexander :surkov 2011-07-21 21:36:49 PDT
All Firefox specific pages (like addons page) open in tabs will live in chrome process. So if we want to be consistent then we should expose HWND for them too. Jamie, should we go with number 1?
Comment 14 James Teh [:Jamie] 2011-07-21 22:31:56 PDT
If creating fake HWNDs for all of these chrome-hosted documents is a huge amount of code, we can do option 2. However, if not (I'm guessing the code is already there), it probably makes sense to do option 1 for the sake of consistency if nothing else.
Comment 15 alexander :surkov 2011-07-22 05:22:08 PDT
As far I can tell it doesn't help. I'll file try server build.
Comment 16 alexander :surkov 2011-07-22 05:31:38 PDT
(In reply to comment #15)
> As far I can tell it doesn't help. I'll file try server build.

try server build will be available here http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.alexander@gmail.com-d6faae7e4d84
Comment 17 David Bolter [:davidb] 2011-07-22 06:48:58 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > > why does the first tab document use the chrome HWND?
> > I'm not sure why it happens, just first tab lives in chrome process.
> So the question is: is this intentional or a bug?

I'm pretty sure it is intentional, and comes from our mobile design. To get details I asked on dev.platform here: http://bit.ly/pB0ZtG

I agree with comment 13 though.
Comment 18 alexander :surkov 2011-07-22 06:53:51 PDT
(In reply to comment #17)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > > why does the first tab document use the chrome HWND?
> > > I'm not sure why it happens, just first tab lives in chrome process.
> > So the question is: is this intentional or a bug?
> 
> I'm pretty sure it is intentional, and comes from our mobile design. To get
> details I asked on dev.platform here: http://bit.ly/pB0ZtG

it's intentional for mobile (due to pref) but it shouldn't make much sense for Firefox.
Comment 20 alexander :surkov 2011-07-22 22:41:19 PDT
let's reopen it until we get sure we finished
Comment 21 James Teh [:Jamie] 2011-07-22 22:58:09 PDT
(In reply to comment #16)
> > As far I can tell it doesn't help. I'll file try server build.
> try server build will be available here
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.
> alexander@gmail.com-d6faae7e4d84
This seems to work for me. NVDA no longer incorrectly associates every tab with the first document buffer. Alex, what were you seeing? Can you double check? Note that focus events are sometimes fired on a broken object or not fired at all when they should be, but I'm definitely not seeing the issue from comment #2.

(In reply to comment #17)
> > > > why does the first tab document use the chrome HWND?
> > So the question is: is this intentional or a bug?
> I'm pretty sure it is intentional, and comes from our mobile design. To get
> details I asked on dev.platform here: http://bit.ly/pB0ZtG
David, in your second post, you noted that this was just because of about:home. However, I'm seeing this even for documents I open myself using the location bar as long as it's the only document open at the time. So the original question still stands.
Comment 22 alexander :surkov 2011-07-22 23:09:32 PDT
(In reply to comment #21)

> (In reply to comment #17)
> > > > > why does the first tab document use the chrome HWND?
> > > So the question is: is this intentional or a bug?
> > I'm pretty sure it is intentional, and comes from our mobile design. To get
> > details I asked on dev.platform here: http://bit.ly/pB0ZtG
> David, in your second post, you noted that this was just because of
> about:home. However, I'm seeing this even for documents I open myself using
> the location bar as long as it's the only document open at the time. So the
> original question still stands.

It's not intentional but I dont' see a point to care about this because there are tab documents like addons page that live in chrome process by design.
Comment 23 James Teh [:Jamie] 2011-07-22 23:14:27 PDT
(In reply to comment #22)
> > > > > > why does the first tab document use the chrome HWND?
> > > > So the question is: is this intentional or a bug?
> It's not intentional but I dont' see a point to care about this because
> there are tab documents like addons page that live in chrome process by
> design.
True, but the Addons page is an application, not a document. I realise this might be similar internally, but NVDA (and probably other ATs) does not do anything special to handle applications, only documents. However, if about:support, about:plugins, about:license, about:mozilla, etc. will be rendered in the chrome process, then I agree that we needn't worry about it and should just move on because the fix is still required. :)
Comment 24 alexander :surkov 2011-07-22 23:42:33 PDT
(In reply to comment #23)
> (In reply to comment #22)
> > > > > > > why does the first tab document use the chrome HWND?
> > > > > So the question is: is this intentional or a bug?
> > It's not intentional but I dont' see a point to care about this because
> > there are tab documents like addons page that live in chrome process by
> > design.
> True, but the Addons page is an application, not a document. I realise this
> might be similar internally, but NVDA (and probably other ATs) does not do
> anything special to handle applications, only documents. However, if
> about:support, about:plugins, about:license, about:mozilla, etc. will be
> rendered in the chrome process, then I agree that we needn't worry about it
> and should just move on because the fix is still required. :)

I'm not 100% sure but these pages should live in chrome process (at least no reason to keep them in content process) and some of them must live in chrome process (like about:addons, about:config, about:cache since they are supposed to work with private information). Note, there's a pref that makes all webpages rendered in chrome process too. So ideally we should end up with solution that make AT working not depending where the specific document (or application) located in.
Comment 25 James Teh [:Jamie] 2011-07-22 23:57:57 PDT
Fair enough; I agree. In that case, assuming Alex's comment #15 is a false alarm, I guess this can be marked as fixed again.
Comment 26 alexander :surkov 2011-07-23 04:49:21 PDT
(In reply to comment #25)
> Fair enough; I agree. In that case, assuming Alex's comment #15 is a false
> alarm, I guess this can be marked as fixed again.

tried again on the fresh build, can't reproduce it with 2011.2b3.
Comment 27 alexander :surkov 2011-07-23 04:51:08 PDT
Created attachment 547919 [details] [diff] [review]
patch v2
Comment 28 alexander :surkov 2011-07-23 05:23:28 PDT
landed - http://hg.mozilla.org/mozilla-central/rev/bf7c3e1c6174

Note You need to log in before you can comment on or make changes to this bug.