1635494 - Kill processes from about:processes

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, enhancement, P3)

Description

[David Teller [:Yoric] - still alive but not very active]

We need a button to kill a process.

We may wish to allow both a graceful kill and a not-so-graceful one.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

I wonder if we should also have a "Pause process" for testing purposes.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

So, after discussing this on chat, the consensus is that we shouldn't try to be gentle with the content process when we kill it.

We should, however, make sure that we don't send a crash report.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

After chatting with gsvelto, it seems that a SIGKILL or a TerminateProcess should manage to kill the content process without sending a crash report.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - WIP

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - Kill processes from about:processes

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - nsIProcessToolsService;r?lina

A small xpcom module implemented in Rust, designed to provide low-level tools to deal with processes
from JS. For the moment, the only notable feature is kill(), designed to be used from about:processes

Comment 7

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - Test that nsIProcessToolsService::kill doesn't cause crash reports;r?gsvelto

Services.processtools.kill() is designed to kill without causing crash reports.
Let's make sure it does not cause crash reports.

Depends on D82552

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - Services.processtools;r?florian

Just a utility shortcut to make using nsIProcessServiceTools more readable from JS.

Depends on D82553

Comment 9

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - Kill processes from about:processes;r?florian

This adds a new button to kill processes.

Depends on D82554

Comment 10

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - Make 'kill browser' button stand out;r?florian

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19c4d55874d9
nsIProcessToolsService;r=lina
https://hg.mozilla.org/integration/autoland/rev/36e670cd1653
Test that nsIProcessToolsService::kill doesn't cause crash reports;r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/862a5dcbe7a5
Kill processes from about:processes;r=florian,flod
https://hg.mozilla.org/integration/autoland/rev/67a16c6149f7
Make 'kill browser' button stand out;r=florian,fluent-reviewers

Comment 12

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on test_process_kill.js

backout: https://hg.mozilla.org/integration/autoland/rev/5e83cc94405fdc2f94f3daea15b224b546505005

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=OYwSQenyTQmhSebt03ZQpg.0&revision=67a16c6149f716d706a99528e3a9df5b50ca468d

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310680027&repo=autoland&lineNumber=7170

[task 2020-07-22T10:58:13.026Z] 10:58:13 INFO - TEST-START | toolkit/components/processtools/tests/xpcshell/test_process_kill.js
[task 2020-07-22T10:58:13.713Z] 10:58:13 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/processtools/tests/xpcshell/test_process_kill.js | xpcshell return code: 0
[task 2020-07-22T10:58:13.714Z] 10:58:13 INFO - TEST-INFO took 689ms
[task 2020-07-22T10:58:13.714Z] 10:58:13 INFO - >>>>>>>
[task 2020-07-22T10:58:13.714Z] 10:58:13 INFO - PID 8852 | [8852, Main Thread] WARNING: Failed to get directory to cache.: file /builds/worker/checkouts/gecko/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 121
[task 2020-07-22T10:58:13.714Z] 10:58:13 INFO - PID 8852 | [8852, Main Thread] WARNING: Failed to get directory to cache.: file /builds/worker/checkouts/gecko/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 121
[task 2020-07-22T10:58:13.715Z] 10:58:13 INFO - PID 8852 | [8852, Main Thread] WARNING: Failed to get directory to cache.: file /builds/worker/checkouts/gecko/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 121
[task 2020-07-22T10:58:13.715Z] 10:58:13 INFO - PID 8852 | [8852, Main Thread] WARNING: Failed to get directory to cache.: file /builds/worker/checkouts/gecko/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 121
[task 2020-07-22T10:58:13.715Z] 10:58:13 INFO - PID 8852 | [8852, Main Thread] WARNING: Failed to get directory to cache.: file /builds/worker/checkouts/gecko/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 121
[task 2020-07-22T10:58:13.716Z] 10:58:13 INFO - PID 8852 | [8852, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp, line 2914
[task 2020-07-22T10:58:13.716Z] 10:58:13 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2020-07-22T10:58:13.716Z] 10:58:13 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - running event loop
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - toolkit/components/processtools/tests/xpcshell/test_process_kill.js | Starting setup
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - (xpcshell/head.js) | test setup pending (2)
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - PID 8852 | [8852, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, kKnownEsrVersion) failed with result 0x80004002 (NS_NOINTERFACE): file /builds/worker/checkouts/gecko/toolkit/components/resistfingerprinting/nsRFPService.cpp, line 673
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - (xpcshell/head.js) | test run_next_test 1 pending (2)
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - (xpcshell/head.js) | test setup finished (2)
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - toolkit/components/processtools/tests/xpcshell/test_process_kill.js | Starting test_subprocess_kill
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - (xpcshell/head.js) | test test_subprocess_kill pending (2)
[task 2020-07-22T10:58:13.717Z] 10:58:13 INFO - (xpcshell/head.js) | test run_next_test 1 finished (2)

Comment 13

[Natalia Csoregi [:nataliaCs]]

Also build bustage on Windows 2012 AArch64: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310684322&repo=autoland&lineNumber=18934

Comment 14

[David Teller [:Yoric] - still alive but not very active]

Thanks, investigating!

Comment 15

[David Teller [:Yoric] - still alive but not very active]

(In reply to Natalia Csoregi [:nataliaCs] from comment #13)

Also build bustage on Windows 2012 AArch64: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310684322&repo=autoland&lineNumber=18934

That's in a third-party library upon which I do not depend and that I do not touch in my commit, so I plead innocent on that one.

Comment 16

[David Teller [:Yoric] - still alive but not very active]

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #15)

(In reply to Natalia Csoregi [:nataliaCs] from comment #13)

Also build bustage on Windows 2012 AArch64: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310684322&repo=autoland&lineNumber=18934

That's in a third-party library upon which I do not depend and that I do not touch in my commit, so I plead innocent on that one.

Or maybe not. Investigating.

Comment 17

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 Investigating windows failure;r?nobody

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:Yoric, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 19

[Neha Kochar [:neha]]

Doesn't block any fission milestone.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a91e04073f9f
nsIProcessToolsService;r=lina
https://hg.mozilla.org/integration/autoland/rev/aab5984f73db
Test that nsIProcessToolsService::kill doesn't cause crash reports;r=gsvelto

Comment 21

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on processtools/src/lib.rs

Backout link: https://hg.mozilla.org/integration/autoland/rev/a3a6401cd56fb7434c7cda660bb9c609be632479
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315776013&repo=autoland&lineNumber=33596

Comment 22

[David Teller [:Yoric] - still alive but not very active]

Thanks, investigating!

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2608db2fa8be
nsIProcessToolsService;r=lina
https://hg.mozilla.org/integration/autoland/rev/14456b365a11
Test that nsIProcessToolsService::kill doesn't cause crash reports;r=gsvelto

Comment 24

[Razvan Maries]

Backed out for build bustages.

Push with faiulres: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=BeRGURHiTP-yCimsgaOT2g.0&resultStatus=testfailed%2Cbusted%2Cexception&revision=14456b365a11251b10f36a056dbd1c95b78ef93b&failure_classification_id=2

Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315840601&repo=autoland&lineNumber=51203

Backout: https://hg.mozilla.org/integration/autoland/rev/79789de9b29cbbcfb3ab22fa474c84705ffeed2d

Comment 25

[David Teller [:Yoric] - still alive but not very active]

The underlying issue is... irritating.
It's a minor Rust papercut, I guess, but which turns into a build bustage.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/569b0d731bad
nsIProcessToolsService;r=lina
https://hg.mozilla.org/integration/autoland/rev/52cfbd2cc59e
Test that nsIProcessToolsService::kill doesn't cause crash reports;r=gsvelto

Comment 27

[Chris Peterson [:cpeterson]]

Tracking about:processes bug for Fission M7 Beta

Comment 28

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/569b0d731bad
https://hg.mozilla.org/mozilla-central/rev/52cfbd2cc59e

Comment 29

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - Highlight whole processes instead of single tabs;r?florian

Depends on D82555

Comment 30

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1635494 - Reorganize process groups in about:processes;r?florian

Depends on D91134

Comment 31

[Florian Quèze [:florian]]

Why are there two new patches for review in a bug that's closed as FIXED? Could you please describe what they do? Thanks!

Comment 32

[David Teller [:Yoric] - still alive but not very active]

Oops, phabricator/lando auto-closed it when I landed the first two patches.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3461db24677e
Kill processes from about:processes;r=florian,flod

Comment 34

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3461db24677e

Comment 35

[David Teller [:Yoric] - still alive but not very active]

Ok, this time, it's officially fixed!