1384560 - Annotate crash reports with accessibility client information

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Jim Mathies [:jimm]]

We ship oop clients to telemetry here[1], and in-proc clients here [2]. We should add this information to crash report annotations (non public) so we don't have to keep asking users what it is they run.

[1] http://searchfox.org/mozilla-central/source/accessible/windows/msaa/LazyInstantiator.cpp#351
[2] http://searchfox.org/mozilla-central/source/accessible/windows/msaa/Compatibility.cpp#134

Comment 1

[Jim Mathies [:jimm]]

Attachment: wip

Comment 2

[Jim Mathies [:jimm]]

Attachment: accum client strings - not for landing

Comment 3

[Jim Mathies [:jimm]]

Attachment: patch

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 8890845 [details] [diff] [review]
patch

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

I just noticed I'm including the exception handler header twice in both files, will address in a follow up.

Comment 5

[Jim Mathies [:jimm]]

Attachment: patch v.2

Went ahead and updated.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8890851 [details] [diff] [review]
patch v.2

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

You are going to need to fix the following preprocessor defines around LazyInstantiator::AccumulateTelemetry to make the conditional defined(MOZ_TELEMETRY_REPORTING) || defined(MOZ_CRASHREPORTER)

http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.cpp#278
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.cpp#247
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.h#89

Then you'll want to make the call to Telemetry::ScalarSet conditional on defined(MOZ_TELEMETRY_REPORTING) only.

r- because I'd like to take another look before landing.

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

Also
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.h#154

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

and
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.h#14

Comment 9

[Jim Mathies [:jimm]]

Attachment: patch v.3

updated per comments.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fb848f123a8
Annotate crash reports with accessibility client information. r=aklotz

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/8fb848f123a8

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 8892923 [details] [diff] [review]
patch v.3

Approval Request Comment
[Feature/Bug causing the regression]:
useful debugging tool - changes add annotations about a11y client to crash reports.
[User impact if declined]:
none
[Is this code covered by automated tests?]:
no
[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]:
none
[Is the change risky?]:
no
[Why is the change risky/not risky?]:
new crash annotations
[String changes made/needed]:
none

Comment 13

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Comment on attachment 8892923 [details] [diff] [review]
patch v.3

Looks useful for crash-stats info, let's uplift this for beta 1.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/411f7b44aee0

Comment 15

[Andrei Vaida [:avaida]]

(In reply to Jim Mathies [:jimm] from comment #12)
> [Is this code covered by automated tests?]:
> no
> [Has the fix been verified in Nightly?]:
> yes
> [Needs manual test from QE? If yes, steps to reproduce]: 
> no

Setting qe-verify- based on Jim's assessment on manual testing needs.

Comment 16

[Aki Sasaki (not active)]

https://hg.mozilla.org/projects/date/rev/8fb848f123a8f1b22a61bcbb57a1583f40e64b36
Bug 1384560 - Annotate crash reports with accessibility client information. r=aklotz