1234121 - Cache active item from child process in parent and remove sync call for FocusedChild

Status: RESOLVED FIXED

(Core :: Disability Access APIs, task)

Comment 1

[Lorien Hu (:lsocks)]

Attachment: focusedchildcache

I need to test this on a platform where focusedChild is actually requested, but do you think this is in the right direction?

Comment 2

[Lorien Hu (:lsocks)]

Oops, ignore that printf.

Comment 3

[Trevor Saunders (:tbsaunde)]

(In reply to Lorien Hu (:lsocks) from comment #1)
> Created attachment 8700473 [details] [diff] [review]
> focusedchildcache
> 
> I need to test this on a platform where focusedChild is actually requested,
> but do you think this is in the right direction?

Well, I'd start by caching what FocusManager::FocusedAccessible() returns.  The testsuite should cover that part so you can test it that way.

Comment 4

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8700473 [details] [diff] [review]
focusedchildcache

>+FocusManager::FocusedRemoteChild() const
>+{
>+  if (mActiveRemoteItem) {
>+    return mActiveRemoteItem;
>+  }
>+
>+  return nullptr;

that's a overly complicated way of saying return mActiveRemoteItem;  However in light of my previous comment you might not want to do this part at all.

>+++ b/accessible/generic/Accessible.cpp
>@@ -859,16 +859,19 @@ Accessible::HandleAccEvent(AccEvent* aEv
>+        case nsIAccessibleEvent::EVENT_FOCUS:
>+          ipcDoc->SendFocusEvent(id);

I don't see the need for this, you can just add some code specific to focus events to DocAccessibleParent::RecvEvent()

> ProxyAccessible::FocusedChild()
> {
>-  uint64_t childID = 0;
>-  bool ok = false;
>-  Unused << mDoc->SendFocusedChild(mID, &childID, &ok);
>-  return ok ? mDoc->GetAccessible(childID) : nullptr;
>+  ProxyAccessible* focus = FocusMgr()->FocusedRemoteChild();
>+  if (focus && (focus == this || focus->Parent() == this)) {
>+    printf("Returned focused child %llu\n", focus->mID);
>+    return focus;
>+  }

This doesn't handle the case the proxy is a document with its special handling of this method.

Comment 5

[Lorien Hu (:lsocks)]

Attachment: remove sync calls for focusedchild

Comment 6

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8703223 [details] [diff] [review]
remove sync calls for focusedchild

sorry about the lag, I ended up not working over the holidays.

> FocusManager::FocusedAccessible() const
> {
>   if (mActiveItem)
>     return mActiveItem;

can you get rid of this check?

>-  }
>+  return mFocusedAcc;

seems simple enough to make inline

>+FocusManager::FocusedRemoteChild() const
>+{
>+  return mFocusedOrActiveProxy;

same

>+FocusManager::RemoteFocusChanged(ProxyAccessible* aProxy)
>+{
>+    // Either active item or DOM focused proxy, we don't care
>+    // which it is for the child process
>+    mFocusedOrActiveProxy = aProxy;

same

>+    return;

not needed

>@@ -251,16 +259,19 @@ FocusManager::ProcessDOMFocus(nsINode* a
>     if (!focusedNode)
>       return;
> 
>     Accessible* DOMFocus =
>       document->GetAccessibleEvenIfNotInMapOrContainer(focusedNode);
>     if (target != DOMFocus)
>       return;
> 
>+    // Cache DOM focused accessible
>+    mFocusedAcc = target;

it seems better to do this in ProcessFocusEvent() so the timing is similar to what happens for remote focus events

>   RefPtr<Accessible> mActiveItem;
>+  RefPtr<Accessible> mFocusedAcc;
>+  ProxyAccessible* mFocusedOrActiveProxy;

I'd just name it mFocusedProxy we also need to investigate what exactly happens when focus shifts from the child process to the parent, and back.

>+  const ProxyAccessible* FocusedChild()
>+    { return FocusMgr()->FocusedRemoteChild(); }

unused

> ProxyAccessible::FocusedChild()
> {
>-  uint64_t childID = 0;
>-  bool ok = false;
>-  Unused << mDoc->SendFocusedChild(mID, &childID, &ok);
>-  return ok ? mDoc->GetAccessible(childID) : nullptr;
>+  ProxyAccessible* focus = FocusMgr()->FocusedRemoteChild();
>+  if (focus && (focus == this || focus->Parent() == this)) {
>+    return focus;
>+  }
>+
>+  return nullptr;

you need to consider the case this is a document

Comment 7

[Lorien Hu (:lsocks)]

Attachment: updated focus patch

Comment 8

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8706186 [details] [diff] [review]
updated focus patch

>@@ -289,16 +288,18 @@ FocusManager::ProcessFocusEvent(AccEvent
>     if (target != DOMFocus)
>       return;
> 
>     Accessible* activeItem = target->CurrentItem();
>     if (activeItem) {
>       mActiveItem = activeItem;
>       target = activeItem;
>     }
>+
>+    mFocusedAcc = target;

why is this guarded by a check target != mActiveItem?

>+  inline Accessible* FocusedAccessible() const

inline keyword is redundant

>+    { return mFocusedAcc; }

I think that fits on the previous line

>+
>+  /**
>+   * Return remote focused accessible.
>+   */
>+  inline ProxyAccessible* FocusedRemoteChild() const
>+    { return mFocusedProxy; }

same on both counts

>   /**
>+   * Called when focused item in child process is changed.
>+   */
>+    // Either active item or DOM focused proxy, we don't care
>+    // which it is for the child process

this comment is weirdly formatted.  I'd probably just skip the second part the active item idea feels like its internal to focus tracking not this, and I wonder if we could actually remove mActiveItem now

>+  inline void RemoteFocusChanged(ProxyAccessible* aProxy)
>+    { mFocusedProxy = aProxy; }

remove inline and one line.

> private:
>   RefPtr<Accessible> mActiveItem;
>+  RefPtr<Accessible> mFocusedAcc;
>+  ProxyAccessible* mFocusedProxy;
>   RefPtr<Accessible> mActiveARIAMenubar;

it kind of seems like mActiveMenuBar and mActiveItem logically belong together, but it hardly matters.

>+   switch(aEventType) {
>+        case nsIAccessibleEvent::EVENT_FOCUS:
>+            FocusMgr()->RemoteFocusChanged(proxy);
>+            break;
>+    }

probably best to have an explicit default

>+  ProxyAccessible* focus = FocusMgr()->FocusedRemoteChild();
>+  if (IsDoc() || (focus && (focus == this || focus->Parent() == this))) {

nit, it'd be easier to read that if it was multiple conditions

Comment 9

[Lorien Hu (:lsocks)]

Attachment: updated focus patch 2

Comment 10

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8723184 [details] [diff] [review]
updated focus patch 2

Sorry, but thinking about this again I realized you need to add a check to ProxyAccessible::Shutdown() to make sure we don't shutdown a proxy with the focus manager still pointing at it.

> 
>   // If active item is changed then fire accessible focus event on it, otherwise
>   // if there's no an active item then fire focus event to accessible having
>   // DOM focus.
>-  Accessible* target = FocusedAccessible();
>-  if (target)
>+

extra blank line

>     Accessible* activeItem = target->CurrentItem();
>     if (activeItem) {
>       mActiveItem = activeItem;
>       target = activeItem;
>     }
>   }
> 
>+  mFocusedAcc = target;

seems like it would be good to make mFocusedProxy null here?

>+  ProxyAccessible* FocusedRemoteChild() const { return mFocusedProxy; }

FocusedRemoteAccessible() makes more sense since its not a child of anything?

Comment 11

[Lorien Hu (:lsocks)]

(In reply to Trevor Saunders (:tbsaunde) from comment #10)
> Comment on attachment 8723184 [details] [diff] [review]
> updated focus patch 2
> 
> Sorry, but thinking about this again I realized you need to add a check to
> ProxyAccessible::Shutdown() to make sure we don't shutdown a proxy with the
> focus manager still pointing at it.

Wouldn't it be better to just set the focus manager to point to null? It doesn't seem to quite make sense for focus to stay on something if we've received a hide event for it, and especially not if its parent is getting shutdown.

Comment 12

[Lorien Hu (:lsocks)]

Attachment: focuspatch2 check if hide event would hide focused child and clear

Comment 13

[Lorien Hu (:lsocks)]

Scratch that last patch.

Comment 14

[Trevor Saunders (:tbsaunde)]

(In reply to Lorien Hu (:lsocks) from comment #11)
> (In reply to Trevor Saunders (:tbsaunde) from comment #10)
> > Comment on attachment 8723184 [details] [diff] [review]
> > updated focus patch 2
> > 
> > Sorry, but thinking about this again I realized you need to add a check to
> > ProxyAccessible::Shutdown() to make sure we don't shutdown a proxy with the
> > focus manager still pointing at it.
> 
> Wouldn't it be better to just set the focus manager to point to null? It
> doesn't seem to quite make sense for focus to stay on something if we've
> received a hide event for it, and especially not if its parent is getting
> shutdown.

I guess I wasn't clear, that's what I was saying to do.

Comment 15

[Lorien Hu (:lsocks)]

Attachment: p1

Comment 16

[Lorien Hu (:lsocks)]

Attachment: p2

Comment 17

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8724843 [details] [diff] [review]
p1

> FocusManager::IsFocused(const Accessible* aAccessible) const
> {
>+  return aAccessible == mFocusedAcc;

might as well be inline now right?

Comment 18

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8724844 [details] [diff] [review]
p2

The split of these patches seems wierd, since this one is fixing a bug in the first.  I'd think the natural way to do things is first change the focus manager stuff and then have another patch for the proxy stuff, but I guess landing these as one patch is fine.

>+void
>+FocusManager::ResetFocusedAccessible()
>+{
>+  mFocusedAcc = nullptr;
>+  mFocusedProxy = nullptr;

probably wouldn't hurt to make it inline

>   mContent = nullptr;
>   mDoc = nullptr;
>   if (SelectionMgr() && SelectionMgr()->AccessibleWithCaret(nullptr) == this)
>     SelectionMgr()->ResetCaretOffset();
>+
>+  if (this == FocusedChild()) {
>+    FocusMgr()->ResetFocusedAccessible();

you should use IsFocused(), and I'm not sure this is needed, but I guess it doesn't hurt.

>     if (mChildren.Length() != 1)
>       MOZ_CRASH("outer doc doesn't own adoc!");
> 
>     mChildren[0]->AsDoc()->Unbind();
>   }
> 
>+  if (this == FocusedChild()) {
>+    FocusMgr()->ResetFocusedAccessible();

I guess you don't have IsFocused() here, but you could just do FocusManager()->FocusedRemoteAccessible() == this

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a9ecb630db6
https://hg.mozilla.org/integration/mozilla-inbound/rev/071d44d633ea

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for Marionette-e10s crashes on Linux in test_accessibility.py TestAccessibility.test_click_raises_no_exceptions:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e17e914479ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c5141a1d8d9

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=071d44d633eaf4b6981ec86fa87e8c8c538be762
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=25342200&repo=mozilla-inbound

04:57:28     INFO -  TEST-START | test_accessibility.py TestAccessibility.test_click_raises_element_not_accessible
04:57:31     INFO -  TEST-PASS | test_accessibility.py TestAccessibility.test_click_raises_element_not_accessible | took 3128ms
04:57:31     INFO -  TEST-START | test_accessibility.py TestAccessibility.test_click_raises_no_exceptions
04:57:32     INFO -  mozcrash Downloading symbols from: https://queue.taskcluster.net/v1/task/dqy2jcQATwilwBhZ3bSzKw/artifacts/public/build/firefox-48.0a1.en-US.linux-x86_64.crashreporter-symbols.zip
04:57:38     INFO -  mozcrash Copy/paste: /builds/slave/test/build/linux64-minidump_stackwalk /tmp/tmpdnWK3U.mozrunner/minidumps/03acc87b-56b7-14ce-0580e070-423db052.dmp /tmp/tmpAjIJEl
04:57:51     INFO -  mozcrash Saved minidump as /builds/slave/test/build/blobber_upload_dir/03acc87b-56b7-14ce-0580e070-423db052.dmp
04:57:51     INFO -  mozcrash Saved app info as /builds/slave/test/build/blobber_upload_dir/03acc87b-56b7-14ce-0580e070-423db052.extra
04:57:51    ERROR -  PROCESS-CRASH | runner.py | application crashed [@ mozilla::a11y::DocAccessibleParent::RecvEvent]
04:57:51     INFO -  Crash dump filename: /tmp/tmpdnWK3U.mozrunner/minidumps/03acc87b-56b7-14ce-0580e070-423db052.dmp
04:57:51     INFO -  Operating system: Linux
04:57:51     INFO -                    0.0.0 Linux 3.2.0-76-generic #111-Ubuntu SMP Tue Jan 13 22:16:09 UTC 2015 x86_64
04:57:51     INFO -  CPU: amd64
04:57:51     INFO -       family 6 model 62 stepping 4
04:57:51     INFO -       1 CPU
04:57:51     INFO -  Crash reason:  SIGSEGV
04:57:51     INFO -  Crash address: 0x10
04:57:51     INFO -  Process uptime: not available
04:57:51     INFO -  Thread 0 (crashed)
04:57:51     INFO -   0  libxul.so!mozilla::a11y::DocAccessibleParent::RecvEvent [FocusManager.h:071d44d633ea : 105 + 0x0]
04:57:51     INFO -      rax = 0x0000000000000000   rdx = 0x0000000000000000
04:57:51     INFO -      rcx = 0x0000000000000078   rbx = 0x00007f5a9c298690
04:57:51     INFO -      rsi = 0x0000000000000000   rdi = 0x00007f5a9c298660
04:57:51     INFO -      rbp = 0x00007fff435b9ad0   rsp = 0x00007fff435b9a90
04:57:51     INFO -       r8 = 0x00007f5a90df882c    r9 = 0x0000000000000701
04:57:51     INFO -      r10 = 0x00007f5a90df8040   r11 = 0x0000000000000246
04:57:51     INFO -      r12 = 0x00007fff435b9b20   r13 = 0x00007f5a9c298660
04:57:51     INFO -      r14 = 0x00007fff435b9b38   r15 = 0x00007fff435b9b20
04:57:51     INFO -      rip = 0x00007f5ac603c663
04:57:51     INFO -      Found by: given as instruction pointer in context
04:57:51     INFO -   1  libxul.so!mozilla::a11y::PDocAccessibleParent::OnMessageReceived [PDocAccessibleParent.cpp:071d44d633ea : 5482 + 0x6]
04:57:51     INFO -      rbx = 0x0000000000000000   rbp = 0x00007fff435b9b80
04:57:51     INFO -      rsp = 0x00007fff435b9ae0   r12 = 0x00007fff435b9b30
04:57:51     INFO -      r13 = 0x00007fff435b9b28   r14 = 0x00007fff435b9b38
04:57:51     INFO -      r15 = 0x00007fff435b9b20   rip = 0x00007f5ac4cba05a
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   2  libxul.so!mozilla::dom::PContentParent::OnMessageReceived [PContentParent.cpp:071d44d633ea : 3808 + 0xc]
04:57:51     INFO -      rbx = 0x00007fff435b9f00   rbp = 0x00007fff435b9dd0
04:57:51     INFO -      rsp = 0x00007fff435b9b90   r12 = 0x00007fff435b9f00
04:57:51     INFO -      r13 = 0x0000000000000001   r14 = 0x00007f5a9c2a4000
04:57:51     INFO -      r15 = 0x0000000000000006   rip = 0x00007f5ac4c97c69
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   3  libxul.so!mozilla::ipc::MessageChannel::DispatchAsyncMessage [MessageChannel.cpp:071d44d633ea : 1646 + 0x6]
04:57:51     INFO -      rbx = 0x00007f5a9c2a4068   rbp = 0x00007fff435b9e10
04:57:51     INFO -      rsp = 0x00007fff435b9de0   r12 = 0x00007fff435b9f00
04:57:51     INFO -      r13 = 0x0000000000000001   r14 = 0x00007fff435b9e00
04:57:51     INFO -      r15 = 0x0000000000000000   rip = 0x00007f5ac4adfffb
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   4  libxul.so!mozilla::ipc::MessageChannel::DispatchMessage [MessageChannel.cpp:071d44d633ea : 1584 + 0xb]
04:57:51     INFO -      rbx = 0x00007fff435b9f00   rbp = 0x00007fff435b9ee0
04:57:51     INFO -      rsp = 0x00007fff435b9e20   r12 = 0x00007f5a9c2a4068
04:57:51     INFO -      r13 = 0x00007fff435b9e30   r14 = 0x00007fff435b9e40
04:57:51     INFO -      r15 = 0x00007fff435b9e38   rip = 0x00007f5ac4ae75e8
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   5  libxul.so!mozilla::ipc::MessageChannel::OnMaybeDequeueOne [MessageChannel.cpp:071d44d633ea : 1551 + 0xb]
04:57:51     INFO -      rbx = 0x00007fff435b9f00   rbp = 0x00007fff435b9f50
04:57:51     INFO -      rsp = 0x00007fff435b9ef0   r12 = 0x00007f5a9c2a4068
04:57:51     INFO -      r13 = 0x00007fff435ba001   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x00007fff435ba09f   rip = 0x00007f5ac4ae93ef
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   6  libxul.so!MessageLoop::RunTask [message_loop.cc:071d44d633ea : 349 + 0x9]
04:57:51     INFO -      rbx = 0x00007f5a9178bee0   rbp = 0x00007fff435b9f70
04:57:51     INFO -      rsp = 0x00007fff435b9f60   r12 = 0x00007f5ac1406bc0
04:57:51     INFO -      r13 = 0x00007fff435ba001   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x00007fff435ba09f   rip = 0x00007f5ac4ac7ae5
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   7  libxul.so!MessageLoop::DeferOrRunPendingTask [message_loop.cc:071d44d633ea : 357 + 0x5]
04:57:51     INFO -      rbx = 0x00007f5ac1406b01   rbp = 0x00007fff435b9f90
04:57:51     INFO -      rsp = 0x00007fff435b9f80   r12 = 0x00007fff435b9fa8
04:57:51     INFO -      r13 = 0x00007fff435ba001   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x00007fff435ba09f   rip = 0x00007f5ac4acca1c
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   8  libxul.so!MessageLoop::DoWork [message_loop.cc:071d44d633ea : 444 + 0x5]
04:57:51     INFO -      rbx = 0x00007f5ac1406bc0   rbp = 0x00007fff435b9fe0
04:57:51     INFO -      rsp = 0x00007fff435b9fa0   r12 = 0x00007fff435b9fa8
04:57:51     INFO -      r13 = 0x00007fff435ba001   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x00007fff435ba09f   rip = 0x00007f5ac4accb33
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -   9  libxul.so!mozilla::ipc::DoWorkRunnable::Run [MessagePump.cpp:071d44d633ea : 227 + 0x9]
04:57:51     INFO -      rbx = 0x00007f5ac1406bc0   rbp = 0x00007fff435ba000
04:57:51     INFO -      rsp = 0x00007fff435b9ff0   r12 = 0x00007fff435ba001
04:57:51     INFO -      r13 = 0x00007fff435ba038   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x00007fff435ba09f   rip = 0x00007f5ac4adbb62
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  10  libxul.so!nsThread::ProcessNextEvent [nsThread.cpp:071d44d633ea : 994 + 0x6]
04:57:51     INFO -      rbx = 0x00007f5acf347b70   rbp = 0x00007fff435ba080
04:57:51     INFO -      rsp = 0x00007fff435ba010   r12 = 0x00007fff435ba028
04:57:51     INFO -      r13 = 0x00007fff435ba038   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x00007fff435ba09f   rip = 0x00007f5ac481318d
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  11  libxul.so!NS_ProcessNextEvent [nsThreadUtils.cpp:071d44d633ea : 297 + 0xd]
04:57:51     INFO -      rbx = 0x0000000000000000   rbp = 0x00007fff435ba0b0
04:57:51     INFO -      rsp = 0x00007fff435ba090   r12 = 0x00007f5ac1406bc0
04:57:51     INFO -      r13 = 0x00007f5acf347b70   r14 = 0x00007fff435ba401
04:57:51     INFO -      r15 = 0x00007f5acf3f3c20   rip = 0x00007f5ac482cc9a
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  12  libxul.so!mozilla::ipc::MessagePump::Run [MessagePump.cpp:071d44d633ea : 98 + 0xa]
04:57:51     INFO -      rbx = 0x00007f5acf3f3c00   rbp = 0x00007fff435ba110
04:57:51     INFO -      rsp = 0x00007fff435ba0c0   r12 = 0x00007f5ac1406bc0
04:57:51     INFO -      r13 = 0x00007f5acf347b70   r14 = 0x00007fff435ba401
04:57:51     INFO -      r15 = 0x00007f5acf3f3c20   rip = 0x00007f5ac4adc4a6
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  13  libxul.so!MessageLoop::Run [message_loop.cc:071d44d633ea : 223 + 0x8]
04:57:51     INFO -      rbx = 0x00007f5ac1406bc0   rbp = 0x00007fff435ba150
04:57:51     INFO -      rsp = 0x00007fff435ba120   r12 = 0x00007f5acf347b70
04:57:51     INFO -      r13 = 0x00007fff435ba340   r14 = 0x00007fff435ba4a1
04:57:51     INFO -      r15 = 0x00007fff435ba210   rip = 0x00007f5ac4ac7b50
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  14  libxul.so!nsBaseAppShell::Run [nsBaseAppShell.cpp:071d44d633ea : 156 + 0xd]
04:57:51     INFO -      rbx = 0x00007f5ab69ad9e0   rbp = 0x00007fff435ba170
04:57:51     INFO -      rsp = 0x00007fff435ba160   r12 = 0x00007f5acf347b70
04:57:51     INFO -      r13 = 0x00007fff435ba340   r14 = 0x00007fff435ba4a1
04:57:51     INFO -      r15 = 0x00007fff435ba210   rip = 0x00007f5ac5b72829
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  15  libxul.so!nsAppStartup::Run [nsAppStartup.cpp:071d44d633ea : 281 + 0x6]
04:57:51     INFO -      rbx = 0x00007f5ab6933060   rbp = 0x00007fff435ba190
04:57:51     INFO -      rsp = 0x00007fff435ba180   r12 = 0x00007fff435ba220
04:57:51     INFO -      r13 = 0x00007fff435ba340   r14 = 0x00007fff435ba4a1
04:57:51     INFO -      r15 = 0x00007fff435ba210   rip = 0x00007f5ac6165c01
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  16  libxul.so!XREMain::XRE_mainRun [nsAppRunner.cpp:071d44d633ea : 4340 + 0x6]
04:57:51     INFO -      rbx = 0x00007fff435ba250   rbp = 0x00007fff435ba2e0
04:57:51     INFO -      rsp = 0x00007fff435ba1a0   r12 = 0x00007fff435ba220
04:57:51     INFO -      r13 = 0x00007fff435ba340   r14 = 0x00007fff435ba4a1
04:57:51     INFO -      r15 = 0x00007fff435ba210   rip = 0x00007f5ac619c2ec
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  17  libxul.so!XREMain::XRE_main [nsAppRunner.cpp:071d44d633ea : 4437 + 0x5]
04:57:51     INFO -      rbx = 0x00007fff435ba340   rbp = 0x00007fff435ba320
04:57:51     INFO -      rsp = 0x00007fff435ba2f0   r12 = 0x0000000000000000
04:57:51     INFO -      r13 = 0x0000000000000000   r14 = 0x0000000000000005
04:57:51     INFO -      r15 = 0x00007f5acf35b480   rip = 0x00007f5ac619c5b0
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  18  libxul.so!XRE_main [nsAppRunner.cpp:071d44d633ea : 4543 + 0x11]
04:57:51     INFO -      rbx = 0x00007fff435ba340   rbp = 0x00007fff435ba4d0
04:57:51     INFO -      rsp = 0x00007fff435ba330   r12 = 0x0000000000000005
04:57:51     INFO -      r13 = 0x00007fff435bb7c8   r14 = 0x00007fff435ba540
04:57:51     INFO -      r15 = 0x00007f5acf35b480   rip = 0x00007f5ac619c81d
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  19  firefox!do_main [nsBrowserApp.cpp:071d44d633ea : 220 + 0x6]
04:57:51     INFO -      rbx = 0x00007fff435bb7c8   rbp = 0x00007fff435bb570
04:57:51     INFO -      rsp = 0x00007fff435ba4e0   r12 = 0x0000000000000005
04:57:51     INFO -      r13 = 0x00007fff435ba540   r14 = 0x00007f5acf35b6c0
04:57:51     INFO -      r15 = 0x00007f5acf35b480   rip = 0x0000000000405cda
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  20  firefox!main [nsBrowserApp.cpp:071d44d633ea : 360 + 0x15]
04:57:51     INFO -      rbx = 0x00007fff435bb7c8   rbp = 0x00007fff435bb6e0
04:57:51     INFO -      rsp = 0x00007fff435bb580   r12 = 0x0000000000000005
04:57:51     INFO -      r13 = 0x00007fff435bb7f8   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x00000032851ef232   rip = 0x00000000004053a5
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  21  libc-2.15.so + 0x2176d
04:57:51     INFO -      rbx = 0x0000000000000000   rbp = 0x0000000000000000
04:57:51     INFO -      rsp = 0x00007fff435bb6f0   r12 = 0x0000000000405490
04:57:51     INFO -      r13 = 0x00007fff435bb7c0   r14 = 0x0000000000000000
04:57:51     INFO -      r15 = 0x0000000000000000   rip = 0x00007f5acf5a476d
04:57:51     INFO -      Found by: call frame info
04:57:51     INFO -  22  firefox!init [replace_malloc.c:071d44d633ea : 133 + 0x2]
04:57:51     INFO -      rsp = 0x00007fff435bb710   rip = 0x00000000004052f7
04:57:51     INFO -      Found by: stack scanning

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I believe this also caused this Windows debug assertion in m(oth):
https://treeherder.mozilla.org/logviewer.html#?job_id=25350312&repo=mozilla-inbound

Comment 22

[Lorien Hu (:lsocks)]

Marionette test failures are due to accessibility being enabled only in the child and not the parent, but DocAccessibleParents are still being created in the parent. Still looking into the Windows assertion failures.

Comment 23

[James Teh [:Jamie]]

Bug 1738051 implements DocAccessibleParent::GetFocusedAcc which can be easily used to implement RemoteAccessibleBase::FocusedChild.

Comment 24

[James Teh [:Jamie]]

Here's what needs to be done here:

1. Unify FocusedChild into base Accessible.
2. In DocAccessible::FocusedChild, if FocusManager returns null, get the focused BrowserParent -> DocAccessibleParent and use GetFocusedAcc on that.
3. Implement DocAccessibleParent::FocusedChild to get the RootAccessible and call FocusedChild on that.
4. Implement RemoteAccessibleBase::FocusedChild to use Document()->GetFocusedAcc(). Ensure it is this or a direct child, otherwise return null.
5. Unify/remove platform specific/XPCOM code for handling remote focus. For Windows, MsaaAccessible::get_accFocus can be tweaked, while MsaaRootAccessible::get_accFocus can be removed.

Comment 25

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1234121: Add GetFrom method to DocAccessibleParent r?Jamie

Depends on D153798

Comment 26

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1234121: Unify FocusedChild() in Accessible base class r?Jamie

Depends on D153997

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bbdce3552ac6
Add GetFrom method to DocAccessibleParent r=Jamie
https://hg.mozilla.org/integration/autoland/rev/48933d8cec46
Unify FocusedChild() in Accessible base class r=Jamie

Comment 28

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/bbdce3552ac6
https://hg.mozilla.org/mozilla-central/rev/48933d8cec46