1314707 - Eliminate SendCOMProxy from PDocAccessible protocol

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

We'll use PDocAccessibleConstructor and do a dance of async messages instead.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch

This patch accomplishes two things:

1) Replaces SendCOMProxy. Instead we use PDocAccessibleConstructor to send the content proxy to chrome. Chrome then sends an async ParentCOMProxy message to content.

2) We defer execution of DocAccessible::NotifyOfLoad until the parent com proxy has been delivered to content. This prevents a race between the load event and receipt of the parent com proxy.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=58e83cad9707

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r2)

This revision takes enqueues any calls to DocAccessibleChild::Send* and plays them back when the parent COM proxy is received.

This is a bit ugly since Send* functions are no longer generated as virtual by IPDL. For the most part I just overrode them anyway since general the static type of the object being called is DocAccessibleChild. The one exception to that is when sending show events, so I made a virtual function for that case.

Of course, if the overriding of non-virtual functions is too nasty for your liking I can use different function names...

Comment 4

[Trevor Saunders (:tbsaunde)]

(In reply to Aaron Klotz [:aklotz] from comment #3)
> Created attachment 8808350 [details] [diff] [review]
> Patch (r2)
> 
> This revision takes enqueues any calls to DocAccessibleChild::Send* and
> plays them back when the parent COM proxy is received.
> 
> This is a bit ugly since Send* functions are no longer generated as virtual
> by IPDL. For the most part I just overrode them anyway since general the
> static type of the object being called is DocAccessibleChild. The one
> exception to that is when sending show events, so I made a virtual function
> for that case.
> 
> Of course, if the overriding of non-virtual functions is too nasty for your
> liking I can use different function names...

on first thought that would seem safer, you can just drop the Send part, and then have all this logic live in DocAccessibleChildBase, but then checking if you have the parent proxy would be a little more complicated.  Also I'm pretty sure we never pass around PDocAccessibleChild* and I'm not sure why we would want to, so the safety thing probably isn't a big deal.  So I guess meh this is fine.

Comment 5

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8808350 [details] [diff] [review]
Patch (r2)

r=me if you have a good explanation for the last question.

># HG changeset patch
># User Aaron Klotz <[e-mail]>
># Date 1478550463 25200
>#      Mon Nov 07 13:27:43 2016 -0700
># Node ID a652622a33ddee7f85615e1073ab16ee117c7c53
># Parent  2fd096140370a618ad4e41a0f0fbe3d157798896
>Bug 1314707: Replace PDocAccessible::SendCOMProxy with new parameter to PDocAccessibleConstructor; r=tbsaunde

probably worth explaining the event business in the commit message.

>+++ b/accessible/generic/DocAccessible-inl.h
>@@ -3,16 +3,17 @@
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>  * You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> #ifndef mozilla_a11y_DocAccessible_inl_h_
> #define mozilla_a11y_DocAccessible_inl_h_
> 
> #include "DocAccessible.h"
>+#include "mozilla/a11y/DocAccessibleChild.h"

you can revert that now right?

>+  /**
>+   * @return true if the show event was sent immediately,
>+   *         false if the show event was deferred
>+   */
>+  virtual bool MaybeSendShowEvent(ShowEventData& aData, bool aFromUser)
>+  { return SendShowEvent(aData, aFromUser); }

who cares about the return value? please drop it if nothing does.

>+DocAccessibleParent::SetCOMInterface(const RefPtr<IAccessible>& aCOMProxy)

This overloading seems weird, can you name one of these better?

>+
>+// So that we don't include a bunch of other Windows junk.
>+#if !defined(COM_NO_WINDOWS_H)
>+#define COM_NO_WINDOWS_H
>+#endif // !defined(COM_NO_WINDOWS_H)

yuk, windows headers are terrible.

>+bool
>+DocAccessibleChild::RecvParentCOMProxy(const IAccessibleHolder& aParentCOMProxy)
> {
>-  IAccessibleHolder parentProxy;
>-  PDocAccessibleChild::SendCOMProxy(aProxy, &parentProxy);
>-  mParentProxy.reset(parentProxy.Release());
>+  MOZ_ASSERT(!aParentCOMProxy.IsNull());

assert mParentProxy is null too?

This reminds me I think I've seen cases where the parent will get replaced, so we may need to handle that, but it should be easier with this patch so that's good.

>+  bool RecvParentCOMProxy(const IAccessibleHolder& aParentCOMProxy) override;

fwiw personally I prefer explicit virtual, but I guess some people don't, so whatever.

>+  bool SendEvent(const uint64_t& aID, const uint32_t& type);
>+  bool SendHideEvent(const uint64_t& aRootID, const bool& aFromUser);
>+  bool SendStateChangeEvent(const uint64_t& aID, const uint64_t& aState,
>+                            const bool& aEnabled);
>+  bool SendCaretMoveEvent(const uint64_t& aID, const int32_t& aOffset);
>+  bool SendTextChangeEvent(const uint64_t& aID, const nsString& aStr,
>+                           const int32_t& aStart, const uint32_t& aLen,
>+                           const bool& aIsInsert, const bool& aFromUser);
>+  bool SendSelectionEvent(const uint64_t& aID, const uint64_t& aWidgetID,

can we make all these return void?  I guess not because we Unused << the result.

> private:
>+  struct DeferredEvent

it might be good to comment this setup some maybe some of which belongs here.

>+  struct SerializedShow : public DeferredEvent

this and the other structs can be final right?

>+      , mFromUser(aFromUser)
>+    {
>+      // Since IDPL doesn't generate a move constructor for ShowEventData,

IPDL ;)

>--- a/ipc/mscom/COMPtrHolder.h
>+++ b/ipc/mscom/COMPtrHolder.h
>@@ -33,17 +33,17 @@ public:
>   {
>   }
> 
>   Interface* Get() const
>   {
>     return mPtr.get();
>   }
> 
>-  MOZ_MUST_USE Interface* Release()
>+  MOZ_MUST_USE Interface* Release() const
>   {
>     return mPtr.release();

is this necessary? why? that doesn't seem const at all.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Trevor Saunders (:tbsaunde) from comment #5)
> Comment on attachment 8808350 [details] [diff] [review]
> Patch (r2)
> 
> r=me if you have a good explanation for the last question.
> 
> >--- a/ipc/mscom/COMPtrHolder.h
> >+++ b/ipc/mscom/COMPtrHolder.h
> >@@ -33,17 +33,17 @@ public:
> >   {
> >   }
> > 
> >   Interface* Get() const
> >   {
> >     return mPtr.get();
> >   }
> > 
> >-  MOZ_MUST_USE Interface* Release()
> >+  MOZ_MUST_USE Interface* Release() const
> >   {
> >     return mPtr.release();
> 
> is this necessary? why? that doesn't seem const at all.

This is another IPDL-ism due to lack of move semantics. In DocAccessibleChild::RecvParentCOMProxy, we want to move the proxy pointer out of aParentCOMProxy into mParentProxy, but aParentCOMProxy is a const reference.

Comment 7

[Trevor Saunders (:tbsaunde)]

(In reply to Aaron Klotz [:aklotz] from comment #6)
> (In reply to Trevor Saunders (:tbsaunde) from comment #5)
> > Comment on attachment 8808350 [details] [diff] [review]
> > Patch (r2)
> > 
> > r=me if you have a good explanation for the last question.
> > 
> > >--- a/ipc/mscom/COMPtrHolder.h
> > >+++ b/ipc/mscom/COMPtrHolder.h
> > >@@ -33,17 +33,17 @@ public:
> > >   {
> > >   }
> > > 
> > >   Interface* Get() const
> > >   {
> > >     return mPtr.get();
> > >   }
> > > 
> > >-  MOZ_MUST_USE Interface* Release()
> > >+  MOZ_MUST_USE Interface* Release() const
> > >   {
> > >     return mPtr.release();
> > 
> > is this necessary? why? that doesn't seem const at all.
> 
> This is another IPDL-ism due to lack of move semantics. In
> DocAccessibleChild::RecvParentCOMProxy, we want to move the proxy pointer
> out of aParentCOMProxy into mParentProxy, but aParentCOMProxy is a const
> reference.

ah, localizing the evilness with const_cast there seems somewhat better if you can, but I guess this is sort of necessary otherwise.

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Trevor Saunders (:tbsaunde) from comment #7)
> 
> ah, localizing the evilness with const_cast there seems somewhat better if
> you can, but I guess this is sort of necessary otherwise.

Sure, I can do that instead.

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r3)

Revised patch with all comments addressed.

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43835f5fa2b217ba7e2a59fb9f52910a2e06a28a
Bug 1314707: Replace PDocAccessible::SendCOMProxy with new parameter to PDocAccessibleConstructor and async RecvParentCOMProxy call in child. Sending of a11y events from child to parent is now deferred until DocAccessibleChild::RecvParentCOMProxy is called; r=tbsaunde

Comment 11

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/841f82fde3b4 for https://treeherder.mozilla.org/logviewer.html#?job_id=38935406&repo=mozilla-inbound

Comment 12

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

I was playing around with the last patch and I get errors/crash similar to this (with different AT's):

IPDL protocol error: Handler returned error code!

###!!! [Parent][DispatchAsyncMessage] Error: PBrowser::Msg_PDocAccessibleConstructor Processing error: message was deserialized, but the handler returned false (indicating failure)

IPDL protocol error: Handler returned error code!

###!!! [Parent][DispatchAsyncMessage] Error: PBrowser::Msg_PDocAccessibleConstructor Processing error: message was deserialized, but the handler returned false (indicating failure)


###!!! [Parent][RunMessage] Error: Channel error: cannot send/recv


###!!! [Parent][RunMessage] Error: Channel error: cannot send/recv


###!!! [Parent][RunMessage] Error: Channel error: cannot send/recv

Assertion failure: wrapper, at c:/gecko-dev/accessible/windows/msaa/Platform.cpp:84
#01: mozilla::a11y::DocAccessibleParent::Destroy (c:\gecko-dev\accessible\ipc\docaccessibleparent.cpp:429)
#02: mozilla::a11y::DocAccessibleParent::ActorDestroy (c:\gecko-dev\obj-i686-pc-mingw32\dist\include\mozilla\a11y\docaccessibleparent.h:94)
#03: mozilla::dom::PBrowserParent::DestroySubtree (c:\gecko-dev\obj-i686-pc-mingw32\ipc\ipdl\pbrowserparent.cpp:5182)
#04: mozilla::dom::PContentParent::DestroySubtree (c:\gecko-dev\obj-i686-pc-mingw32\ipc\ipdl\pcontentparent.cpp:8909)
#05: mozilla::dom::PContentParent::OnChannelError (c:\gecko-dev\obj-i686-pc-mingw32\ipc\ipdl\pcontentparent.cpp:8878)
#06: mozilla::detail::RunnableMethodImpl<void (__thiscall mozilla::ipc::MessageChannel::*)(void),0,1>::Run (c:\gecko-dev\obj-i686-pc-mingw32\dist\include\nsthreadutils.h:812)
#07: nsThread::ProcessNextEvent (c:\gecko-dev\xpcom\threads\nsthread.cpp:1216)
#08: NS_ProcessNextEvent (c:\gecko-dev\xpcom\glue\nsthreadutils.cpp:361)
#09: mozilla::ipc::MessagePump::Run (c:\gecko-dev\ipc\glue\messagepump.cpp:96)
#10: MessageLoop::RunInternal (c:\gecko-dev\ipc\chromium\src\base\message_loop.cc:232)
#11: MessageLoop::RunHandler (c:\gecko-dev\ipc\chromium\src\base\message_loop.cc:226)
#12: MessageLoop::Run (c:\gecko-dev\ipc\chromium\src\base\message_loop.cc:206)
#13: nsBaseAppShell::Run (c:\gecko-dev\widget\nsbaseappshell.cpp:158)
#14: nsAppShell::Run (c:\gecko-dev\widget\windows\nsappshell.cpp:264)
#15: nsAppStartup::Run (c:\gecko-dev\toolkit\components\startup\nsappstartup.cpp:284)
#16: XREMain::XRE_mainRun (c:\gecko-dev\toolkit\xre\nsapprunner.cpp:4467)
#17: XREMain::XRE_main (c:\gecko-dev\toolkit\xre\nsapprunner.cpp:4600)
#18: XRE_main (c:\gecko-dev\toolkit\xre\nsapprunner.cpp:4691)
#19: do_main (c:\gecko-dev\browser\app\nsbrowserapp.cpp:282)
#20: NS_internal_main (c:\gecko-dev\browser\app\nsbrowserapp.cpp:415)
#21: wmain (c:\gecko-dev\toolkit\xre\nswindowswmain.cpp:118)
#22: __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:255)
#23: BaseThreadInitThunk[C:\WINDOWS\System32\KERNEL32.DLL +0x162c4]
#24: RtlSubscribeWnfStateChangeNotification[C:\WINDOWS\SYSTEM32\ntdll.dll +0x60719]
#25: RtlSubscribeWnfStateChangeNotification[C:\WINDOWS\SYSTEM32\ntdll.dll +0x606e4]

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

I cannot reproduce those failures locally. :-( It's probably the fact that these tests are run on a VM that we can see this.

From what I can see looking at the stacks on those failures, the real problem is that something is causing xul.dll!mozilla::dom::ContentParent::KillHard(char const *), which then injects breakpoints into everything to trigger crash reports.

...

I have examined the browser minidump and extracted the reason message from the KillHard call:

PBrowser::Msg_PDocAccessibleConstructor Processing error: message was deserialized, but the handler returned false (indicating  failure)

So it looks like this patch can't land until we fix the PDocAccessibleConstructor problems. :-(
OTOH, we now know how to consistently reproduce this.

Adding bug 1270916 as a dependency.

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r4)

Updated to reflect new return type of IPDL Recv* functions.

Comment 15

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a1448396b1a

Comment 16

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Defer non-top-level PDocAccessibleConstructor calls

The first patch is still failing even though the PDocAccessibleConstructor patch landed. I realized that the reason why is that even though the top-level DocAccessibleChild has not yet transmitted any deferred show events, we're still trying to SendPDocAccessibleConstructor calls that depend on them.

This additional patch adds deferral of those SendPDocAccessibleConstructor calls as well. This appears to work, but if you can think of a better way of doing this...

Comment 17

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=54c7e203d71d

Comment 18

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r5)

OK, this patch allows us to enqueue all the things in each top level DocAccessibleChild.

Comment 19

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26eb2d4bbed1

Comment 20

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r6)

Revision 5 was crashing during a top-level document shutdown because DocAccessibleChild::GetTopLevelIPCDoc was failing to account for the fact that DocAccessible::mDocumentNode is cleared *before* DocAccessibleChild::Shutdown is called.

Comment 21

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b3ca5998a0c

Comment 22

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r7)

More clownshoes.

Comment 23

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d1a33e015be

Comment 24

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

Trevor what do you think of this latest patch?

Comment 25

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8812406 [details] [diff] [review]
Patch (r7)

Sorry about the lag here.

>+
>+#if defined(XP_WIN)
>+      MOZ_ASSERT(parentIPCDoc);
>+      parentIPCDoc->ConstructChildDocInParentProcess(ipcDoc, id,
>+                                                     AccessibleWrap::GetChildIDFor(childDoc));
>+#else
>       nsCOMPtr<nsITabChild> tabChild =
>         do_GetInterface(mDocument->DocumentNode()->GetDocShell());
>       if (tabChild) {
>         MOZ_ASSERT(parentIPCDoc);
>         static_cast<TabChild*>(tabChild.get())->
>-          SendPDocAccessibleConstructor(ipcDoc, parentIPCDoc, id,
>-#if defined(XP_WIN)
>-                                        AccessibleWrap::GetChildIDFor(childDoc)
>-#else
>-                                        0
>+          SendPDocAccessibleConstructor(ipcDoc, parentIPCDoc, id, 0, 0);
>+      }
> #endif

might be nice to hide more of this junk in the Construct method, but whatever.

>-DocAccessibleChild::SendCOMProxy(const IAccessibleHolder& aProxy)
>+DocAccessibleChild::Shutdown()
> {
>-  IAccessibleHolder parentProxy;
>-  PDocAccessibleChild::SendCOMProxy(aProxy, &parentProxy);
>-  mParentProxy.reset(parentProxy.Release());
>+  DocAccessibleChild* rootIPCDoc = GetTopLevelIPCDoc();
>+  if (!rootIPCDoc || rootIPCDoc->IsConstructedInParentProcess()) {
>+    DocAccessibleChildBase::Shutdown();

So, I think there's still a race here, what happens if the parent process hasn't yet send us a com proxy for the top level doc but we go to shutdown a sub document?  I think this will end up sending a shutdown message for an actor we haven't yet told the main process about.  I'm not completely sure if this is a problem with other messages, or just this one.

>+DocAccessibleChild*
>+DocAccessibleChild::GetTopLevelIPCDoc()
>+{
>+  if (!mDoc) {
>+    // This is possible during shutdown
>+    return nullptr;
>+  }
>+
>+  DocAccessible* topLevelDocAcc = nullptr;
>+
>+  if (mDoc->IsRoot()) {
>+    topLevelDocAcc = mDoc;
>+  } else {
>+    topLevelDocAcc = mDoc->RootAccessible();
>+    if (!topLevelDocAcc) {
>+      // This is possible during shutdown
>+      return nullptr;
>+    }
>+  }
>+
>+  return topLevelDocAcc->IPCDoc();
>+}

so, I think a different way to this that may always return something is to get the DocAccessibleChild actor's mannager which is a TabChild, andthen look through the DocAccessibleChilds it manages for one that is a top level doc, I believe there should only be one.  That's because I believe actor's should get destroyed bottum up, and there should only be one top level doc in a tab.  I'm not completely sure that'll work though.

That might fix the race above?

>+  bool SendEvent(const uint64_t& aID, const uint32_t& type);
>+  bool SendHideEvent(const uint64_t& aRootID, const bool& aFromUser);
>+  bool SendStateChangeEvent(const uint64_t& aID, const uint64_t& aState,

since bool is now inconsistant with the ipdl generated stuff and the ipc result name seems long and pointless might make sense to just return void, but whatever.

>+  {
>+    void Dispatch()
>+    {
>+      Dispatch(mTarget);
>+    }
>+
>+    virtual ~DeferredEvent() {}
>+
>+  protected:
>+    explicit DeferredEvent(DocAccessibleChild* aTarget)
>+      : mTarget(aTarget)
>+    {}
>+
>+    virtual void Dispatch(DocAccessibleChild* aIPCDoc) = 0;
>+
>+  private:
>+    DocAccessibleChild* mTarget;

seems like it might be simpler to just put this member in the subclasses, and get rid of the DispatchEvent(DocAccessible*) overload, but doesn't really matter.

It'd be nice to reduce the boilerplate a bit, but I guess it can wait.

We need to deal with that race, but I think that's much less important than the current issues.

Comment 26

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 27

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=76da6b47117d

Comment 28

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 29

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 30

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r8)

(In reply to Trevor Saunders (:tbsaunde) from comment #25)
> Comment on attachment 8812406 [details] [diff] [review]
> Patch (r7)
> 
> >-DocAccessibleChild::SendCOMProxy(const IAccessibleHolder& aProxy)
> >+DocAccessibleChild::Shutdown()
> > {
> >-  IAccessibleHolder parentProxy;
> >-  PDocAccessibleChild::SendCOMProxy(aProxy, &parentProxy);
> >-  mParentProxy.reset(parentProxy.Release());
> >+  DocAccessibleChild* rootIPCDoc = GetTopLevelIPCDoc();
> >+  if (!rootIPCDoc || rootIPCDoc->IsConstructedInParentProcess()) {
> >+    DocAccessibleChildBase::Shutdown();
> 
> So, I think there's still a race here, what happens if the parent process
> hasn't yet send us a com proxy for the top level doc but we go to shutdown a
> sub document?  I think this will end up sending a shutdown message for an
> actor we haven't yet told the main process about.  I'm not completely sure
> if this is a problem with other messages, or just this one.
> 
> >+DocAccessibleChild*
> >+DocAccessibleChild::GetTopLevelIPCDoc()
> >+{
> >+  if (!mDoc) {
> >+    // This is possible during shutdown
> >+    return nullptr;
> >+  }
> >+
> >+  DocAccessible* topLevelDocAcc = nullptr;
> >+
> >+  if (mDoc->IsRoot()) {
> >+    topLevelDocAcc = mDoc;
> >+  } else {
> >+    topLevelDocAcc = mDoc->RootAccessible();
> >+    if (!topLevelDocAcc) {
> >+      // This is possible during shutdown
> >+      return nullptr;
> >+    }
> >+  }
> >+
> >+  return topLevelDocAcc->IPCDoc();
> >+}
> 
> so, I think a different way to this that may always return something is to
> get the DocAccessibleChild actor's mannager which is a TabChild, andthen
> look through the DocAccessibleChilds it manages for one that is a top level
> doc, I believe there should only be one.  That's because I believe actor's
> should get destroyed bottum up, and there should only be one top level doc
> in a tab.  I'm not completely sure that'll work though.
> 
> That might fix the race above?
> 

I have moved top-level doc resolution to a function called DocAccessibleChild::PushDeferredEvent that obtains it from the tabChild.

Sub-document shutdowns will now call PushDeferredEvent to enqueue its shutdown to its top-level doc, thus placing its shutdown message some point after its own constructor.

Comment 31

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 32

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f56b053ab5cffd9e050e5a8aca69840a9d6a611
Bug 1314707: Replace PDocAccessible::SendCOMProxy with new parameter to PDocAccessibleConstructor and async RecvParentCOMProxy call in child. Sending of a11y events from child to parent is now deferred until DocAccessibleChild::RecvParentCOMProxy is called; r=tbsaunde

Comment 33

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2f56b053ab5c

Comment 34

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8819060 [details] [diff] [review]
Patch (r8)

Approval Request Comment
[Feature/Bug causing the regression]: a11y+e10s
[User impact if declined]: Crashes
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: Depends on the uplift for bug 1321935
[Is the change risky?]: No
[Why is the change risky/not risky?]: This was originally landed on 52 but was backed out because of the dependent bug 1321935. Now that the dependent bug is fixed and everything looks good on Nightly, this change can return back to 52.
[String changes made/needed]: None

Comment 35

[Julien Cristau [:jcristau] (back April 22)]

I guess I'll wait for bug 1323996 before getting this in aurora.

Comment 36

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8819060 [details] [diff] [review]
Patch (r8)

e10s+a11y fix for aurora52

Comment 37

[Carsten Book [:Tomcat]]

needs rebasing for aurora

Comment 38

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4a224e3df2d7787b2716438de00e55ec71e48f42

Comment 39

[(No longer employed by Mozilla) Aaron Klotz]

One more due to bustage from the IPCResult changeover in version 53.
https://hg.mozilla.org/releases/mozilla-aurora/rev/79763f9eac51d555813ae9c9b87ece57e2859b31