1845523 - ExtensionProcessCrashObserver should use integer (number) instead of string type for childID

Status: RESOLVED FIXED

(WebExtensions :: General, task, P3)

Description

[Rob Wu [:robwu]]

The childID parameter in ExtensionProcessCrashObserver is currently a string, but the more common convention is for the value to be a number.

We should use parseInt(data, 10) instead of just data at: https://searchfox.org/mozilla-central/rev/b6b8ff043e944a5e32ea63208d3ba7cb6b16191d/toolkit/components/extensions/Extension.sys.mjs#697

and for test coverage, add Assert.ok(Number.isInteger(currentProcessChildID), "correct type"); after https://searchfox.org/mozilla-central/rev/b6b8ff043e944a5e32ea63208d3ba7cb6b16191d/toolkit/components/extensions/test/browser/browser_ext_process_crash_handling.js#107

Comment 1

[Rob Wu [:robwu]]

This is a good first bug for people who want to familiarize themselves with setting up a development environment and submitting a patch. To get started, see https://wiki.mozilla.org/WebExtensions/Contribution_Onramp

Since this bug is essentially two lines of change, I recommend the following steps once you have set up the development environment:

1. Run the test first to verify that it passes as expected:
   ./mach test toolkit/components/extensions/test/browser/browser_ext_process_crash_handling.js
2. Add the test as explained in the first comment.
3. Run the test again to verify that it not fails.
4. Fix the bug as suggested in the first comment.
5. Run the test again to verify that the test now passes again.
6. Submit the patch for review.

Comment 2

[Alex Stout]

Attachment: WIP: Bug 1845523 - ExtensionProcessCrashObserver.observe now uses a number instead of string for childID, and test added to assert correct type used

Comment 3

[Alex Stout]

Attachment: Bug 1845523 - ExtensionProcessCrashObserver.observe now uses a number instead of string for childID, and test added to assert correct type used. r?robwu!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e18f258d294d
https://hg.mozilla.org/integration/autoland/rev/4e64a1b16592
ExtensionProcessCrashObserver.observe now uses a number instead of string for childID, and test added to assert correct type used. r=robwu

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4e64a1b16592