a11y crashes due to null COM proxy on top-level PDocAccessible

RESOLVED FIXED in Firefox 56

Status

()

defect
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: simona.marcu, Assigned: aklotz)

Tracking

(Blocks 1 bug, {crash})

55 Branch
mozilla56
Unspecified
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 disabled, firefox55 disabled, firefox56 fixed)

Details

(Whiteboard: aes+, crash signature)

Attachments

(8 attachments, 1 obsolete attachment)

1.11 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review
20.57 KB, patch
jimm
: review+
benjamin
: feedback+
Details | Diff | Splinter Review
6.18 KB, patch
jimm
: review+
Details | Diff | Splinter Review
1.98 KB, patch
jimm
: review+
Details | Diff | Splinter Review
10.82 KB, patch
jimm
: review+
Details | Diff | Splinter Review
3.10 KB, patch
jimm
: review+
Details | Diff | Splinter Review
6.07 KB, patch
eeejay
: review+
Details | Diff | Splinter Review
5.81 KB, patch
jimm
: review+
Details | Diff | Splinter Review
This bug was filed from the Socorro interface and is 
report bp-f18f0286-8dcf-49a4-929f-589252170406.
=============================================================

Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0
Buid ID: 20170406030206

NonVisual Desktop Access (NVDA)
Version: 2017.1
URL: http://www.nvaccess.org

Steps to reproduce:
1. Open NVDA
2. Launch Firefox with a new profile

Expected results:
NVDA should read the text displayed in the Nightly window.

Actual results:
Nightly crashes over and over. These are the crash reports I'm getting:
https://crash-stats.mozilla.com/report/index/a53d3a04-10aa-4e3c-8f30-8c6fc2170406
https://crash-stats.mozilla.com/report/index/6716ab5c-5434-4be6-9a21-532222170406
Please note that I'm only getting these crashes on the 32-bit builds. The 64-bit builds are not crashing.
Aaro, is this due to some of the work you've been doing recently with TLBs etc.? Those were 32-bit centric, weren't they?
Blocks: 1258839
Flags: needinfo?(aklotz)
Whiteboard: aes?
(Assignee)

Comment 3

2 years ago
Doesn't look like it, I'm afraid. FWIW I think I saw this happen on a one-off occurrence on a 64-bit build as well.
Whiteboard: aes? → aes+
(Assignee)

Updated

2 years ago
Flags: needinfo?(aklotz)
Stack overflow caused by mutual recursion over:
GetIAccessibleFor
GetCOMInterface
GetProxiedAccessibleInSubtree
Duplicate of this bug: 1341185

Updated

2 years ago
Assignee: nobody → aklotz
(Assignee)

Comment 6

2 years ago
I think that this was regressed by bug 1332444. Unfortunately the problem that bug was trying to solve is not described at all. I've ni? Trevor in that bug with the hope that he might remember what the problem was.
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1360373
(Assignee)

Comment 8

2 years ago
I have reopened bug 1332444 and backed out its patches. I'll leave this open until we can verify that this is fixed.
(Assignee)

Updated

2 years ago
Crash Signature: [@ GetProxiedAccessibleInSubtree] [@ mozilla::a11y::AccessibleWrap::GetIAccessibleFor ] → [@ GetProxiedAccessibleInSubtree] [@ mozilla::a11y::AccessibleWrap::GetIAccessibleFor ] [@ mozilla::a11y::ProxyAccessible::GetCOMInterface ]
(Assignee)

Comment 9

2 years ago
OK, it turns out that bug 1332444 was okay. Why are we hitting stack overflows in this bug, then? Based on what I'm seeing, the invariant that proxies to top-level docs should always have valid COM proxies is broken.
(Assignee)

Comment 10

2 years ago
Looking at the crashes with this signature, the majority of them include a CoGetInterfaceAndReleaseStream failure code of TYPE_E_LIBNOTREGISTERED (0x8002801D).

We are failing to unmarshal IAccessible because of a missing typelib and are left with null.

Two thoughts:

1. We should be catching this and returning IPC_FAIL when we see it in TabParent::RecvPDocAccessibleConstructor. That won't eliminate the crashes but at least the failure will be more clear.
2. We need to monitor crash-stats over the next couple of days and see whether the updater patch from bug 1358276 had any effect. I don't see any TYPE_E_LIBNOTREGISTERED annotations from today's Nightly, but we'll need a few days' worth of data to know for sure.
(Assignee)

Updated

2 years ago
Keywords: leave-open
Attachment #8863078 - Flags: review?(tbsaunde+mozbugs) → review+
(Assignee)

Comment 12

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/39726aed41e676e5d3dc4638ab69f27a1dac0562
Bug 1354077: Return IPC_FAIL from TabParent::RecvPDocAccessibleConstructor when we receive a top-level document with a null COM proxy; r=tbsaunde
(Assignee)

Comment 13

2 years ago
(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now) from comment #10)
> 2. We need to monitor crash-stats over the next couple of days and see
> whether the updater patch from bug 1358276 had any effect. I don't see any
> TYPE_E_LIBNOTREGISTERED annotations from today's Nightly, but we'll need a
> few days' worth of data to know for sure.

Crash-stats is showing no crashes with that annotation since the April 27 Nightly. So far so good, but I'd like to wait a few more days because weekend.
(Assignee)

Updated

2 years ago
Crash Signature: [@ GetProxiedAccessibleInSubtree] [@ mozilla::a11y::AccessibleWrap::GetIAccessibleFor ] [@ mozilla::a11y::ProxyAccessible::GetCOMInterface ] → [@ GetProxiedAccessibleInSubtree] [@ mozilla::a11y::AccessibleWrap::GetIAccessibleFor ] [@ mozilla::a11y::ProxyAccessible::GetCOMInterface ] [@ IPCError-browser | PBrowserParent::RecvPDocAccessibleConstructor Constructing a top-level PDocAccessible wit…
(Assignee)

Updated

2 years ago
Summary: Crash in GetProxiedAccessibleInSubtree when NVDA is on → a11y crashes due to null COM proxy on top-level PDocAccessible
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1361516
(Assignee)

Comment 16

2 years ago
Looks like we still have some of these :-(
(Assignee)

Comment 17

2 years ago
Something is corrupt on these systems and I cannot fix it without knowing what exactly is going on. When we fail with TYPE_E_LIBNOTREGISTERED, I'd like to annotate the crash report with some additional data about the current registry configuration for the IAccessible interface. This would consist of some GUIDs, a file path to a DLL or typelib, information about whether that file actually exists on disk, and version information from that file, if present.
Hey Aaron, softvision needs a fix for this to start their testing. Is this still an issue and if so, can we move it up on the priority list?
Flags: needinfo?(aklotz)
(Assignee)

Comment 19

2 years ago
Simona, are you able to reproduce these yourself?
Flags: needinfo?(aklotz) → needinfo?(simona.marcu)
(Assignee)

Comment 20

2 years ago
Adding bsmedberg for feedback re: privacy review

This patch adds crash report annotations when a ProxyStream fails to marshal or unmarshal. These annotations consist of the COM registration info from the registry in JSON format.

An example from running this against my local laptop (newlines and comments added by me):

{
 "HKLM": {
  "InterfaceName": "IAccessible",
  "ProxyStub": {
   "CLSID": "{00020424-0000-0000-C000-000000000046}",
   "ClassName": "PSOAInterface",
   # This is the path as specified in the registry verbatim
   "Path": "C:\\Windows\\SysWOW64\\oleaut32.dll",
   # This is the path as resolved by the Windows loader
   "LoadedPath": "C:\\WINDOWS\\System32\\OLEAUT32.dll",
   "ThreadingModel": "Both"
  },
  "TypeLib": {
   "ID": "{C523F390-9C83-11D3-9094-00104BD0D535}",
   "Version": "3.0",
   "Description": "Acrobat Access 3.0 Type Library",
   "Flags": "0",
   # The following "0" represents the language-neutral locale.
   # There may be other locales registered so we enumerate all of them.
   # If present, their names would be non-zero integers.
   "0": {
    "Win32": {
     "Path": "C:\\Program Files (x86)\\Adobe\\Acrobat Reader DC\\Reader\\plug_ins\\Accessibility.api",
     "Exists": true
    }
    # Win64 builds may also include a "Win64" section with the same format
   }
  }
 },
 # We repeat the process for HKCU as they can override HKLM,
 # though normally this is likely to be empty
 "HKCU": {
 }
}
Attachment #8865991 - Flags: review?(jmathies)
Attachment #8865991 - Flags: feedback?(benjamin)
(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now) from comment #19)
> Simona, are you able to reproduce these yourself?

No, I can no longer reproduce the crash on Windows 10 x64 using the latest Nightly 55.0a1 with NVDA version 2017.1.
Flags: needinfo?(simona.marcu)
Comment on attachment 8865991 [details] [diff] [review]
Annotate crash report with COM interface registration info when marshaling fails

data-r=me

Please treat this annotation as a private annotation for the purposes of crash-stats, because it contains path data that might reveal usernames. It should only be visible to users who have logged in and have minidump access.
Attachment #8865991 - Flags: feedback?(benjamin) → feedback+
Comment on attachment 8865991 [details] [diff] [review]
Annotate crash report with COM interface registration info when marshaling fails

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

::: ipc/mscom/InterfaceRegistrationAnnotator.cpp
@@ +73,5 @@
> +                            RRF_RT_REG_SZ | RRF_RT_REG_EXPAND_SZ, nullptr,
> +                            nullptr, &numBytes);
> +  if (result) {
> +    return false;
> +  }

I think these should be explicit checks for result != ERROR_SUCCESS.

@@ +123,5 @@
> +    return false;
> +  }
> +
> +  WCHAR finalPath[MAX_PATH + 1] = {};
> +  DWORD result = GetModuleFileName(mod, finalPath, ArrayLength(finalPath));

Will this code get compiled in with thunderbird? If so I wonder if they still support ascii builds, in which case we should use the specific 'W' function name.

@@ +180,5 @@
> +    return;
> +  }
> +
> +  nsCOMPtr<nsIFile> typelib;
> +  nsresult rv = NS_NewLocalFile(nsDependentString(buf.get(), bufCharLen), false,

Where does this go looking typically? Wondering about issues with read restrictions and sandboxing.

@@ +250,5 @@
> +  LONG result = RegOpenKeyEx(aHive, typelibSubKey.get(), 0, KEY_READ,
> +                             &rawTypelibKey);
> +  if (result) {
> +    return;
> +  }

another ERROR_SUCCESS return value

@@ +258,5 @@
> +  WCHAR keyName[kMaxLcidCharLen];
> +
> +  for (DWORD index = 0; !result; ++index) {
> +    DWORD keyNameLength = ArrayLength(keyName);
> +    result = RegEnumKeyEx(typelibKey, index, keyName, &keyNameLength, nullptr,

another ERROR_SUCCESS
Attachment #8865991 - Flags: review?(jmathies) → review+
(Assignee)

Comment 24

2 years ago
(In reply to Jim Mathies [:jimm] from comment #23)
> Comment on attachment 8865991 [details] [diff] [review]
> Annotate crash report with COM interface registration info when marshaling
> fails
> 
> Review of attachment 8865991 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: ipc/mscom/InterfaceRegistrationAnnotator.cpp
> @@ +73,5 @@
> > +                            RRF_RT_REG_SZ | RRF_RT_REG_EXPAND_SZ, nullptr,
> > +                            nullptr, &numBytes);
> > +  if (result) {
> > +    return false;
> > +  }
> 
> I think these should be explicit checks for result != ERROR_SUCCESS.

Sure. Fixed.

> 
> @@ +123,5 @@
> > +    return false;
> > +  }
> > +
> > +  WCHAR finalPath[MAX_PATH + 1] = {};
> > +  DWORD result = GetModuleFileName(mod, finalPath, ArrayLength(finalPath));
> 
> Will this code get compiled in with thunderbird? If so I wonder if they
> still support ascii builds, in which case we should use the specific 'W'
> function name.

Huh. TIL Thunderbird supports ANSI builds. I changed it to use the W suffix just in case.

> 
> @@ +180,5 @@
> > +    return;
> > +  }
> > +
> > +  nsCOMPtr<nsIFile> typelib;
> > +  nsresult rv = NS_NewLocalFile(nsDependentString(buf.get(), bufCharLen), false,
> 
> Where does this go looking typically? Wondering about issues with read
> restrictions and sandboxing.

We normally expect this to be in system32. If there was any tampering then it might be another path, most likely somewhere in Program Files. But if the sandbox blocks it, that's a good thing, as it is reflecting what COM is also experiencing and helps to explain the failures.

> 
> @@ +250,5 @@
> > +  LONG result = RegOpenKeyEx(aHive, typelibSubKey.get(), 0, KEY_READ,
> > +                             &rawTypelibKey);
> > +  if (result) {
> > +    return;
> > +  }
> 
> another ERROR_SUCCESS return value

Fixed

> 
> @@ +258,5 @@
> > +  WCHAR keyName[kMaxLcidCharLen];
> > +
> > +  for (DWORD index = 0; !result; ++index) {
> > +    DWORD keyNameLength = ArrayLength(keyName);
> > +    result = RegEnumKeyEx(typelibKey, index, keyName, &keyNameLength, nullptr,
> 
> another ERROR_SUCCESS

Fixed
(Assignee)

Comment 25

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2268a93ddfc82e0fc7dc941b8eba87faef878a80
Bug 1354077: Annotate crash reports with COM interface configuration information when marshaling fails; r=jimm
Comment hidden (offtopic)
(Assignee)

Updated

2 years ago
Blocks: 1365645
(In reply to Jim Mathies [:jimm] from comment #27)
> Hey Grover, could you please confirm this addresses the crashing in bug
> 1341185? Should be in tomorrow's nightly.

nm, diagnostic landing.
Flags: needinfo?(gwimberly)

Comment 29

2 years ago
No worries. Please needinfo me when it's ready to check this.
Flags: needinfo?(jmathies)
Flags: needinfo?(aklotz)

Updated

2 years ago
Flags: needinfo?(jmathies)
(Assignee)

Comment 30

2 years ago
I've looked at some early returns on the new annotations. So far this is what I've found:

1. Lots of failures due to IAccessible registration being *completely missing*. Further annoying me is the fact that oleacc.dll's DllRegisterServer is a no-op -- we can't even ask oleacc.dll to restore these missing keys! We will have to do it manually, sadly.

2. It looks like Office 2003 did something similar to Adobe Acrobat and trampled over the default typelib. I see one report where the typelib is set to "C:\Program Files (x86)\Common Files\Microsoft Shared\OFFICE11\MSO.DLL". The annotations claim that the typelib itself was able to be loaded successfully, but that was probably in the browser process. My suspicion here is that this path was blocked by the sandbox.

3. I found a bug in the annotations where CoGetInterfaceAndReleaseStream failures are missing interface annotations. I am posting a follow-up patch to mitigate that so that we no longer have a blind spot there.
Flags: needinfo?(aklotz)
(Assignee)

Comment 31

2 years ago
We were missing some annotations when CoGetInterfaceAndReleaseStream failed because I was deferring that annotation to a later time which would never be called.

I have added the aIID parameter to one of the ProxyStream constructors to make it possible to annotate at the time of failure.

I have also modified the annotation code to use separate keys for parent process vs child process annotations. I want to do this to ensure that a content failure due to sandboxing is not clobbered by a parent failure. In case of a sandboxing failure, I expect the child annotations to differ from the parent annotations.
Attachment #8869584 - Flags: review?(jmathies)

Comment 32

2 years ago
(In reply to Carsten Book [:Tomcat] from comment #26)
> https://hg.mozilla.org/mozilla-central/rev/2268a93ddfc8

I have build errors after that check-in.

> 13:04.08 Unified_cpp_ipc_mscom0.cpp
> 13:04.08 d:/Building_Mozilla/source/hg/moz-central/ipc/mscom/InterfaceRegistrationAnnotator.cpp(189): error C2065: "nsPrintfCString": nichtdeklarierter Bezeichner
> 13:04.08 d:/Building_Mozilla/source/hg/moz-central/ipc/mscom/InterfaceRegistrationAnnotator.cpp(189): error C2146: Syntaxfehler: Fehlendes ";" vor Bezeichner "loadResult"
> 13:04.08 d:/Building_Mozilla/source/hg/moz-central/ipc/mscom/InterfaceRegistrationAnnotator.cpp(189): error C3861: "loadResult": Bezeichner wurde nicht gefunden.
> 13:04.08 d:/Building_Mozilla/source/hg/moz-central/ipc/mscom/InterfaceRegistrationAnnotator.cpp(190): error C2065: "loadResult": nichtdeklarierter Bezeichner
> 13:04.08 d:/Building_Mozilla/source/hg/moz-central/ipc/mscom/InterfaceRegistrationAnnotator.cpp(190): error C2228: Links von ".get" m�ssen sich in einer Klasse/Struktur/Union befinden

https://dxr.mozilla.org/mozilla-central/rev/8d60d0f825110cfb646ac31dc16dc011708bcf34/ipc/mscom/InterfaceRegistrationAnnotator.cpp#189
> nsPrintfCString loadResult("0x%08X", hr);

Updated

2 years ago
Attachment #8869584 - Flags: review?(jmathies) → review+
(Assignee)

Comment 33

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9be478667895a3cfb3a00fb2a3bde9437cc20fcc
Bug 1354077: Fix missing interface registration annotations for CoGetInterfaceAndReleaseStream failures; r=jimm
Comment on attachment 8871410 [details] [diff] [review]
Fix problems with failed RegGetValue type check

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

works for me on win7

::: ipc/mscom/InterfaceRegistrationAnnotator.cpp
@@ +75,4 @@
>    DWORD numBytes = 0;
>    LONG result = RegGetValue(aBaseKey, flatSubKey.get(), valueName,
> +                            RRF_RT_ANY, &type, nullptr, &numBytes);
> +  if (result != ERROR_SUCCESS || (type != REG_SZ && type != REG_EXPAND_SZ) {

missing an ending parenthesis here
Attachment #8871410 - Flags: review?(jmathies) → review+
(Assignee)

Comment 37

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/73c786971ec0525d496003ee2b2b996e4d0a8769
Bug 1354077: Fix some problems with RegGetValue call failing due to failed type checking; r=jimm
(Assignee)

Comment 39

2 years ago
I have attempted implementing my "select an appropriate manifest at runtime" proposal and it looks like it is working on both pre and post Creators Update installations. I think we might be able to move back to this approach which completely isolates us from registry corruption and is guaranteed to be sandboxing-safe.
(Assignee)

Updated

2 years ago
Depends on: 1368150
(Assignee)

Updated

2 years ago
Depends on: 1371376
(Assignee)

Comment 41

2 years ago
32-bit creators update needs the 64-bit manifest, so we must always embed the 64-bit manifest even for 32-bit builds.
Attachment #8875874 - Flags: review?(jmathies)
(Assignee)

Comment 42

2 years ago
In part 2 I embed manifests into xul.dll's resources. The 32-bit manifest is embedded with resource ID 32, while the 64-bit manifest is embedded with resource ID 64.

In Win64 builds, we use the 64-bit manifest unconditionally. OTOH, on 32-bit builds, we must use the 32-bit manifest unless we are running Windows 10 Creators Update or newer, which requires the 64-bit manifest instead.

We then load the activation context using the appropriate resource ID.

Since activation contexts affect the thread on which they are executing, we need to activate them on three distinct threads:

* The parent process main thread, via a11y::PlatformInit;
* The content process main thread, via a11y::PlatformChild::PlatformChild;
* The content process MTA thread via a11y::PlatformChild::PlatformChild using mscom::EnsureMTA.

Some further explanation on EnsureMTA:

EnsureMTA executes the provided lambda on the MTA thread. mActCtxMTA is a special type of unique pointer that will destroy its object on the MTA thread (this requires bug 1371376 to land first).

(I realize that a11y::PlatformChild::PlatformChild is a bit of a mess right now. I plan to clean that up eventually.)
Attachment #8875883 - Flags: review?(eitan)

Updated

2 years ago
Attachment #8875867 - Flags: review?(jmathies) → review+

Updated

2 years ago
Attachment #8875874 - Flags: review?(jmathies) → review+
Comment on attachment 8875883 [details] [diff] [review]
Part 3 - Activate appropriate activation contexts on both main thread and content MTA thread

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

This looks good, thanks for the lengthy explanation. Wouldn't understand it otherwise.

::: accessible/ipc/win/PlatformChild.cpp
@@ +55,5 @@
> +#else
> +  if (IsWin10CreatorsUpdateOrLater()) {
> +    actCtxResourceId = 64;
> +  } else {
> +    actCtxResourceId = 32;

These numbers aren't documented outside of anything besides xulrunner.rc, maybe a comment here about the resource IDs.
Attachment #8875883 - Flags: review?(eitan) → review+
(Assignee)

Comment 44

2 years ago
(In reply to Eitan Isaacson [:eeejay] from comment #43)
> Comment on attachment 8875883 [details] [diff] [review]
> Part 3 - Activate appropriate activation contexts on both main thread and
> content MTA thread
> 
> Review of attachment 8875883 [details] [diff] [review]:
> -----------------------------------------------------------------
> These numbers aren't documented outside of anything besides xulrunner.rc,
> maybe a comment here about the resource IDs.

Will do.
Keywords: leave-open
Hardware: x86 → Unspecified
(Assignee)

Comment 45

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0fc3a1a9122564f748c5b870b176e744fc4635b
Bug 1354077: Refactor mscom::ActivationContext to separate context from activation; r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b5602bd352afe00fe8591c5788d55bf9ba055e2
Bug 1354077: Add manifests for IAccessible variants: 32-bit and 64-bit; r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/1380fe0f701b50bdd975fed1a2388e156abde454
Bug 1354077: Modify a11y platform initialization to select and enable appropriate IAccessible manifest; r=eeejay
(Assignee)

Updated

2 years ago
Blocks: 1372357
(Assignee)

Comment 49

2 years ago
I think I have a lead on why we're sometimes seeing problems:

The thunk that Windows uses to invoke our window procedure pushes a blank activation context overtop of our assembly activation context for the duration of a winproc call. Any accessibles that are marshaled during that time will not be using our overrides that are specified in the manifest.

There might be other places where Windows does this.

I am not yet sure what the best path forward is. If we initialize the activation context earlier (say, right after loading xul.dll, instead of some time after the first CreateWindow), these problems might go away.

Otherwise we're stuck having to explicitly push and pop actctx all over the place, which is fragile.

I am going to play with this some more. I have also written a debugger extension which dumps the current thread's activation context stack, which is proving useful.

Note:

user32!UserCallWinProcCheckWow is the function responsible for modifying the actctx stack via these internal functions:
ntdll!RtlActivateActivationContextUnsafeFast
ntdll!RtlDeactivateActivationContextUnsafeFast

Setting breakpoints on those latter two functions allows you to intercept this actctx activity.
Flags: needinfo?(aklotz)
(Assignee)

Comment 51

2 years ago
(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now, please send interceptor reviews to dmajor or handyman) from comment #49)
> There might be other places where Windows does this.

Come to think of it, I believe that APCs do this as well; QueueUserAPC saves the current thread's activation context and pushes it on the target thread before executing the APC.
(Assignee)

Comment 52

2 years ago
Okay, I can confirm that pushing the activation contexts before creating any windows (this includes the COM STA message pump Window) results in the correct activation context being uses in window procedures. More investigation forthcoming...
(Assignee)

Comment 53

2 years ago
(This patch applies atop the first 3 parts)

If we activate the a11y manifest before our first initialization of COM, it looks like all WndProcs are invoked with the correct activation context.

Try push looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b7412f598156e6f28f448f3960f89cf7098b486
Attachment #8881821 - Flags: review?(jmathies)
(Assignee)

Comment 54

2 years ago
Removed a vestigial printf.
Attachment #8881821 - Attachment is obsolete: true
Attachment #8881821 - Flags: review?(jmathies)
Attachment #8881823 - Flags: review?(jmathies)

Updated

2 years ago
Attachment #8881823 - Flags: review?(jmathies) → review+
(Assignee)

Comment 55

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e16ecb250cfeb97c488d8c8d6fcaf8fc1ae5c513
Bug 1354077: Refactor mscom::ActivationContext to separate context from activation; r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ffd99eb9b0a607d3f3cb21bceede4e7a8e60e36
Bug 1354077: Add manifests for IAccessible variants: 32-bit and 64-bit; r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b9799502f1e8526ee8a9f0bb8d7656bded85da3
Bug 1354077: Modify a11y platform initialization to select and enable appropriate IAccessible manifest; r=eeejay

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d989e8e00f6aa689ccd4c85d04db388f6e34f5e
Bug 1354077: Push a11y activation context during mscom::MainThreadRuntime initialization; r=jimm
(Assignee)

Updated

2 years ago
Blocks: 1377195
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
See Also: → 1378141
AIUI this only happens with e10s forced on, so marking status as disabled for 55.
This is still visible in the the Windows nightlies of 20170706030206,
as topcrash #2.
(Assignee)

Comment 60

2 years ago
(In reply to Jim Mathies [:jimm] from comment #59)
> Looks like we may have regressed this, Aaron, any ideas what's going on?
> 
> https://crash-stats.mozilla.com/signature/?product=Firefox&version=56.
> 0a1&signature=IPCError-
> browser%20%7C%20PBrowserParent%3A%3ARecvPDocAccessibleConstructor%20Construct
> ing%20a%20top-level%20PDocAccessible%20with%20null%20COM&date=%3E%3D2017-06-
> 01T14%3A06%3A00.000Z&date=%3C2017-07-08T14%3A06%3A08.
> 000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_colum
> ns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-
> date&page=1#graphs

bug 1378141.
Flags: needinfo?(aklotz)
(Assignee)

Updated

2 years ago
Blocks: 1380158
See Also: → 1380214
You need to log in before you can comment on or make changes to this bug.