1940494 - nsICrashReporter::annotateCrashReport does not support typed annotations

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Gabriele Svelto [:gsvelto]]

The implementation for nsICrashReporter::annotateCrashReport() assumes that the annotation type is a string. This wasn't a problem when all annotations were strings, but now that some can use concrete types this doesn't work anymore. Passing a boolean for example will appear to work if the value is true in a release build because there's no checking and the value will be converted to a string. In a debug build it will crash, and a value of false will succeed but will set the annotation to "1" which is wrong.

We should at very least support integer and boolean annotations from JavaScript and return an error for all other types that aren't supported in annotations.

Comment 1

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1940494 - Allow using typed crash annotations from JavaScript r=afranchuk

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f955d08effb9
Allow using typed crash annotations from JavaScript r=afranchuk

Comment 3

[amarc]

Backed out for causing build bustages @ ws2def.h

• Backout link
• Push with failures
• Failure Log
• Failure line:

/builds/worker/fetches/vs/Windows Kits/10/Include/10.0.22621.0/shared/ws2def.h(240,16): error: redefinition of 'sockaddr'

Comment 4

[Gabriele Svelto [:gsvelto]]

This is highlighting an issue with includes when windows.h gets included before the other includes in LSPAnnotator.cpp. I don't think we need windows.h in nsExceptionHandler.h so I'll get rid of that.

Comment 5

[Gabriele Svelto [:gsvelto]]

Attachment: Bug 1940494 - Remove the windows.h inclusion in nsExceptionHandler.h to fix issues in non-unified builds r=afranchuk

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/910b00bf2240
Allow using typed crash annotations from JavaScript r=afranchuk
https://hg.mozilla.org/integration/autoland/rev/295e23cf17a0
Remove the windows.h inclusion in nsExceptionHandler.h to fix issues in non-unified builds r=afranchuk

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70821984b161
Backed out 2 changesets for causing bustages at ProtocolUtils.cpp. CLOSED TREE

Comment 8

[Gabriele Svelto [:gsvelto]]

Ugh, sorry. I had rolled a fix in the patch yesterday and it had picked up an unrelated change, so I reverted the unrelated change... or at least I thought I did that. I had reverted the fix instead. Landing again.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25c2c93f1f7a
Allow using typed crash annotations from JavaScript r=afranchuk

Comment 10

[Atila Butkovits]

Backed out for causing build bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/2fcf097964880f9e91dc0dd6bd0563f68b304708

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=TUYpJP_hT5e4KTwfAsQGiw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=25c2c93f1f7a1ba5f8bf63b782e80274ed28cdac

Failure log: https://treeherder.mozilla.org/logviewer?job_id=490223468&repo=autoland&lineNumber=135303

Comment 11

[Gabriele Svelto [:gsvelto]]

It seems that only half of the patch-set landed, I must have done something wrong on Lando. Either way here's a try run with both patches folded together. This should work.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9b4aef0aa14
Allow using typed crash annotations from JavaScript r=afranchuk,win-reviewers,gstoll

Comment 13

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/e9b4aef0aa14