If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Intermittent PROCESS-CRASH | Main app process exited normally | application crashed [@ mozilla::a11y::DocAccessibleChild::RecvParentCOMProxy(mozilla::mscom::COMPtrHolder<IAccessible,&_GUID const IID_IAccessible> const &,unsigned __int64 const &,mozilla:

RESOLVED FIXED in Firefox 54

Status

()

Core
Disability Access APIs
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: Treeherder Bug Filer, Assigned: tbsaunde)

Tracking

({intermittent-failure})

unspecified
mozilla54
intermittent-failure
Points:
---

Firefox Tracking Flags

(firefox52 unaffected, firefox53 unaffected, firefox54 fixed)

Details

(Whiteboard: [stockwell fixed])

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

8 months ago
treeherder
Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=76521962&repo=mozilla-inbound

https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-win64-debug/1486769701/mozilla-inbound_win8_64-debug_test-mochitest-e10s-browser-chrome-3-bm109-tests1-windows-build136.txt.gz

Had you left me any summary room with a decent-sized crashing frame, I would have mentioned "after Assertion failure: !mParentProxy && !aParentCOMProxy.IsNull()".
(Assignee)

Comment 1

8 months ago
so this is the same basic issue as bug 1337983 accept what happens when we remove that assert.  So it should get fixed soon as part of dealing with the  real issue there.
Summary: Intermittent Main app process exited normally | application crashed [@ mozilla::a11y::DocAccessibleChild::RecvParentCOMProxy(mozilla::mscom::COMPtrHolder<IAccessible,&_GUID const IID_IAccessible> const &,unsigned __int64 const &,mozilla::mscom::COMPtrHo → Intermittent PROCESS-CRASH | Main app process exited normally | application crashed [@ mozilla::a11y::DocAccessibleChild::RecvParentCOMProxy(mozilla::mscom::COMPtrHolder<IAccessible,&_GUID const IID_IAccessible> const &,unsigned __int64 const &,mozilla:
(Assignee)

Comment 2

7 months ago
Created attachment 8837779 [details] [diff] [review]
split ParentCOMProxy message up

sending the emulated window information is basically unrelated to sending the
parent COM proxy to the child process, and in the future it will be useful to
send these at different times.
Attachment #8837779 - Flags: review?(yzenevich)
(Assignee)

Comment 3

7 months ago
Created attachment 8837780 [details] [diff] [review]
split parts of SetCOMProxy() out to MybeInitWindowEmulation()
Attachment #8837780 - Flags: review?(yzenevich)
(Assignee)

Comment 4

7 months ago
Created attachment 8837781 [details] [diff] [review]
split up SetCOMProxy()
Attachment #8837781 - Flags: review?(yzenevich)
(Assignee)

Comment 5

7 months ago
Created attachment 8837782 [details] [diff] [review]
make RemoteChildDoc() return a DocAccessibleParent*
Attachment #8837782 - Flags: review?(yzenevich)
(Assignee)

Comment 6

7 months ago
Created attachment 8837783 [details] [diff] [review]
send a tabs parent proxy when it may have changed
Attachment #8837783 - Flags: review?(yzenevich)

Updated

7 months ago
Attachment #8837779 - Flags: review?(yzenevich) → review+

Comment 7

7 months ago
Comment on attachment 8837780 [details] [diff] [review]
split parts of SetCOMProxy() out to MybeInitWindowEmulation()

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

::: dom/ipc/TabParent.cpp
@@ +961,5 @@
>  
>      doc->SetTopLevel();
>      a11y::DocManager::RemoteDocAdded(doc);
>  #ifdef XP_WIN
> +    doc->MaybeInitWindowEmulation();

As far as I remember, setting emulation should happen after the COM proxy is set. The reason is that we sometimes calculate bounds for the proxy (https://dxr.mozilla.org/mozilla-central/source/accessible/ipc/DocAccessibleParent.cpp#556) and for that we need the COM proxy set (https://dxr.mozilla.org/mozilla-central/source/accessible/ipc/win/ProxyAccessible.cpp#210)

Comment 8

7 months ago
Trevor, see comment 7
Flags: needinfo?(tbsaunde+mozbugs)
(Assignee)

Comment 9

7 months ago
(In reply to Yura Zenevich [:yzen] from comment #7)
> Comment on attachment 8837780 [details] [diff] [review]
> split parts of SetCOMProxy() out to MybeInitWindowEmulation()
> 
> Review of attachment 8837780 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/ipc/TabParent.cpp
> @@ +961,5 @@
> >  
> >      doc->SetTopLevel();
> >      a11y::DocManager::RemoteDocAdded(doc);
> >  #ifdef XP_WIN
> > +    doc->MaybeInitWindowEmulation();
> 
> As far as I remember, setting emulation should happen after the COM proxy is
> set. The reason is that we sometimes calculate bounds for the proxy
> (https://dxr.mozilla.org/mozilla-central/source/accessible/ipc/
> DocAccessibleParent.cpp#556) and for that we need the COM proxy set
> (https://dxr.mozilla.org/mozilla-central/source/accessible/ipc/win/
> ProxyAccessible.cpp#210)

erg yeah, that's my bad.  Shouldn't be hard to fix but I need to go rearrange this series of patches.
Flags: needinfo?(tbsaunde+mozbugs)

Comment 10

7 months ago
Pushed by tsaunders@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/05d7ba5c9105
split ParentCOMProxy message up r=yzen
(Assignee)

Comment 11

7 months ago
Created attachment 8838200 [details] [diff] [review]
split parts of SetCOMProxy() out to MaybeInitWindowEmulation()
Attachment #8838200 - Flags: review?(yzenevich)
(Assignee)

Updated

7 months ago
Attachment #8837780 - Attachment is obsolete: true
Attachment #8837780 - Flags: review?(yzenevich)
(Assignee)

Comment 12

7 months ago
Created attachment 8838205 [details] [diff] [review]
split up SetCOMProxy()
Attachment #8838205 - Flags: review?(yzenevich)
(Assignee)

Updated

7 months ago
Attachment #8837781 - Attachment is obsolete: true
Attachment #8837781 - Flags: review?(yzenevich)

Comment 13

7 months ago
24 failures in 151 pushes (0.159 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 11
* mozilla-inbound: 10
* mozilla-central: 2
* graphics: 1

Platform breakdown:
* windows8-64: 20
* windows7-32-vm: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1338755&startday=2017-02-16&endday=2017-02-16&tree=all
Comment on attachment 8838200 [details] [diff] [review]
split parts of SetCOMProxy() out to MaybeInitWindowEmulation()

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

looks good, 1 nit and 1 question

::: accessible/ipc/DocAccessibleParent.cpp
@@ +559,5 @@
> +    nsIntRect rootRect = rootDocument->Bounds();
> +    rect.x = rootRect.x - rect.x;
> +    rect.y -= rootRect.y;
> +
> +    auto tab = static_cast<dom::TabParent*>(Manager());

would we need to bail if tab is destroyed?

@@ +579,5 @@
> +                                               IID_IAccessible,
> +                                               (void**)&rawHWNDAcc))) {
> +      hWndAccHolder.Set(IAccessibleHolder::COMPtrType(rawHWNDAcc));
> +    }
> +    }

nit: extra white space before }
Attachment #8838200 - Flags: review?(yzenevich) → review+
Comment on attachment 8838205 [details] [diff] [review]
split up SetCOMProxy()

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

looks good
Attachment #8838205 - Flags: review?(yzenevich) → review+

Updated

7 months ago
Attachment #8837782 - Flags: review?(yzenevich) → review+

Updated

7 months ago
Attachment #8837783 - Flags: review?(yzenevich) → review+

Comment 16

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/05d7ba5c9105
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
(Assignee)

Comment 17

7 months ago
(In reply to Yura Zenevich [:yzen] from comment #14)
> Comment on attachment 8838200 [details] [diff] [review]
> split parts of SetCOMProxy() out to MaybeInitWindowEmulation()
> 
> Review of attachment 8838200 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> looks good, 1 nit and 1 question
> 

> @@ +559,5 @@
> > +    nsIntRect rootRect = rootDocument->Bounds();
> > +    rect.x = rootRect.x - rect.x;
> > +    rect.y -= rootRect.y;
> > +
> > +    auto tab = static_cast<dom::TabParent*>(Manager());
> 
> would we need to bail if tab is destroyed?

no, the code that does that is non redundant see what happens in RecvPDocAccessibleConstructor()

Comment 18

7 months ago
Pushed by tsaunders@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/162b85cf472d
split parts of SetCOMProxy() out to MaybeInitWindowEmulation() r=yzen
https://hg.mozilla.org/integration/mozilla-inbound/rev/874b53206690
split up SetCOMProxy() r=yzen
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fcdfa7a42b7
make RemoteChildDoc() return a DocAccessibleParent* r=yzen
https://hg.mozilla.org/integration/mozilla-inbound/rev/e66b5929cc7d
send a tab's parent proxy when it may have changed r=yzen
Assignee: nobody → tbsaunde+mozbugs
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected

Comment 19

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/162b85cf472d
https://hg.mozilla.org/mozilla-central/rev/874b53206690
https://hg.mozilla.org/mozilla-central/rev/7fcdfa7a42b7
https://hg.mozilla.org/mozilla-central/rev/e66b5929cc7d

Comment 20

7 months ago
24 failures in 180 pushes (0.133 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 11
* mozilla-inbound: 10
* mozilla-central: 2
* graphics: 1

Platform breakdown:
* windows7-32-vm: 15
* windows8-64: 9

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1338755&startday=2017-02-17&endday=2017-02-17&tree=all

Comment 21

7 months ago
74 failures in 833 pushes (0.089 failures/push) were associated with this bug in the last 7 days. 

This is the #28 most frequent failure this week. 

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **

Repository breakdown:
* mozilla-inbound: 34
* autoland: 29
* mozilla-central: 7
* try: 2
* graphics: 2

Platform breakdown:
* windows8-64: 43
* windows7-32-vm: 31

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1338755&startday=2017-02-13&endday=2017-02-19&tree=all
Whiteboard: [stockwell fixed]
You need to log in before you can comment on or make changes to this bug.