1788596 - Get rid of UTILITY_AUDIO_DECODING_GENERIC

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P2)

Description

[:gerard-majax]

We now have not much justification for keeping a different sandbox UTILITY_AUDIO_DECODING_GENERIC except on Linux where we have to allow a few calls for ffmpeg. We would also need to change how we send telemetry / crash report SandboxingKind and move to sending the actors on the process

Comment 1

[:gerard-majax]

Attachment: Bug 1788596 - Merge UtilityAudioDecoderSandboxPolicy into UtilitySandboxPolicy r?jld!

Comment 2

[:gerard-majax]

Attachment: Bug 1788596 - Remove UTILITY_AUDIO_DECODING_GENERIC r?nika!

Comment 3

[:gerard-majax]

Attachment: Bug 1788596 - Use Utility process actor names for crash annotations r?gsvelto!

Comment 4

[:gerard-majax]

Attachment: Bug 1788596 - Remove extra lock() on UtilityProcessChild r?nika!

Depends on D156286

Comment 5

[:gerard-majax]

Attachment: Bug 1788596 - Remove RecvNewContentRemoteDecoderManager on wrong class r?alwu!

Depends on D156482

Comment 6

[:gerard-majax]

Attachment: Bug 1788596 - Release UtilityAudioDecoderChild on process shutdown r?alwu!

Depends on D156483

Comment 7

[:gerard-majax]

Attachment: Bug 1788596 - Use CanSend() instead of boolean r?nika!

Depends on D156484

Comment 8

[:gerard-majax]

Attachment: Bug 1788596 - Remove Utility AudioDecoder process naming to keep generic name r?alwu!

Depends on D156485

Comment 9

[:gerard-majax]

Attachment: Bug 1788596 - Release UtilityProcessManager at the right moment r?nika!

Depends on D156486

Comment 10

[:gerard-majax]

Attachment: Bug 1788596 - Kill audioDecoder_Generic if hanging r?nika!

Depends on D156847

Comment 11

[:gerard-majax]

Attachment: Bug 1788596 - Assert CleanShutown() of audioDecoder_* actor shuts down fine r?nika!

Depends on D157479

Comment 12

[:gerard-majax]

Comment on attachment 9292807 [details]
Bug 1788596 - Use Utility process actor names for crash annotations r?gsvelto!

1. What questions will you answer with this data?

What IPC actors were alive on the process at the time of the crash

2. Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements? Some example responses:

We need it to help debug

3. What alternative methods did you consider to answer these questions? Why were they not sufficient?

There is no alternative. We were collecting sandbox identifier, but now this is not relevant and we need to move to IPC actors name

4. Can current instrumentation answer these questions?

No

5. List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data collection categories found on the Mozilla wiki.

Note that the data steward reviewing your request will characterize your data collection based on the highest (and most sensitive) category.

<table>
<tr>
<td>Measurement Description</td>
<td>Data Collection Category</td>
<td>Tracking Bug #</td>
</tr>
<tr>
<td>IPC actors name</td>
<td>1 - Technical data</td>
<td>1788596</td>
</tr>
</table>

6. Please provide a link to the documentation for this data collection which describes the ultimate data set in a public, complete, and accurate way.

Documentation is in
https://hg.mozilla.org/mozilla-central/file/tip/toolkit/crashreporter/CrashAnnotations.yaml

7. How long will this data be collected? Choose one of the following:

• I want to permanently monitor this data. (me)

8. What populations will you measure?

• All crahes

9. If this data collection is default on, what is the opt-out mechanism for users?

No mechanism is required for crash report data.

10. Please provide a general description of how you will analyze this data.

Crash debugging

11. Where do you intend to share the results of your analysis?

N/A

12. Is there a third-party tool (i.e. not Glean or Telemetry) that you are proposing to use for this data collection? If so:

No

Comment 13

[Will Kahn-Greene [:willkg] ET needinfo? me]

Comment on attachment 9292807 [details]
Bug 1788596 - Use Utility process actor names for crash annotations r?gsvelto!

(In reply to Alexandre LISSY :gerard-majax from comment #12)

Comment on attachment 9292807 [details]
Bug 1788596 - Use Utility process actor names for crash annotations r?gsvelto!

DATA COLLECTION REVIEW RESPONSE:

1. Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes.

2. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes.

3. If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes.

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1: technical

5. Is the data collection request for default-on or default-off?

Default-off. (crash report)

6. Does the instrumentation include the addition of any new identifiers?

No.

7. Is the data collection covered by the existing Firefox privacy notice?

Yes.

8. Does there need to be a check-in in the future to determine whether to renew the data?

No.

9. Does the data collection use a third-party collection tool?

No.

Everything looks good!

Comment 14

[:gerard-majax]

Attachment: Bug 1788596 - Release UtilityProcessChild later to allow mShutdownBlockers to act r?nika!

Depends on D157480

Comment 15

[:gerard-majax]

Attachment: Bug 1788596 - Force audio element load to avoid edge case on media error handling r?alwu!

Depends on D157480

Comment 16

[:gerard-majax]

The last attempts were kinda clean and stable in term of intermittents:

• https://treeherder.mozilla.org/jobs?repo=try&revision=795c4d0337f24c9127499eca5d3e922382b9a56d
• https://treeherder.mozilla.org/jobs?repo=try&revision=f9c9c773f388c8c7fa5899f8ce452c6287e4d588

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63ee00ea9be6
Merge UtilityAudioDecoderSandboxPolicy into UtilitySandboxPolicy r=jld
https://hg.mozilla.org/integration/autoland/rev/3e7125be66fa
Remove UTILITY_AUDIO_DECODING_GENERIC r=nika,media-playback-reviewers,alwu
https://hg.mozilla.org/integration/autoland/rev/93f50c2f0b9e
Use Utility process actor names for crash annotations r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/8b27ee4d4168
Remove extra lock() on UtilityProcessChild r=nika
https://hg.mozilla.org/integration/autoland/rev/9040533dabe1
Remove RecvNewContentRemoteDecoderManager on wrong class r=alwu
https://hg.mozilla.org/integration/autoland/rev/4ebb8837ee1a
Release UtilityAudioDecoderChild on process shutdown r=alwu
https://hg.mozilla.org/integration/autoland/rev/2fc674146915
Use CanSend() instead of boolean r=nika
https://hg.mozilla.org/integration/autoland/rev/077fd3a987ca
Remove Utility AudioDecoder process naming to keep generic name r=alwu
https://hg.mozilla.org/integration/autoland/rev/397e6c6587f3
Release UtilityProcessManager at the right moment r=nika
https://hg.mozilla.org/integration/autoland/rev/e9d29218beba
Kill audioDecoder_Generic if hanging r=nika
https://hg.mozilla.org/integration/autoland/rev/1d1d15dbe44c
Release UtilityProcessChild later to allow mShutdownBlockers to act r=nika
https://hg.mozilla.org/integration/autoland/rev/9d4a5c557191
Assert CleanShutown() of audioDecoder_* actor shuts down fine r=nika
https://hg.mozilla.org/integration/autoland/rev/338c18d01cfd
Force audio element load to avoid edge case on media error handling r=alwu

Comment 18

[Sandor Molnar[:smolnar]]

Backed out for causing browser-chrome failures in security/sandbox/test/browser_sandbox_test.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/605cc3e4f2555c8f9473b7fa7c5ffe7e0c4b6ee3

Push with failures

Failure log

Comment 19

[:gerard-majax]

(In reply to Sandor Molnar from comment #18)

Backed out for causing browser-chrome failures in security/sandbox/test/browser_sandbox_test.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/605cc3e4f2555c8f9473b7fa7c5ffe7e0c4b6ee3

Push with failures

Failure log

That was just a missing change I forgot on sandbox tests, it should now be addressed.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3dd59224f38b
Merge UtilityAudioDecoderSandboxPolicy into UtilitySandboxPolicy r=jld
https://hg.mozilla.org/integration/autoland/rev/9a87f108548b
Remove UTILITY_AUDIO_DECODING_GENERIC r=nika,media-playback-reviewers,alwu
https://hg.mozilla.org/integration/autoland/rev/e99c7089bf93
Use Utility process actor names for crash annotations r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/1be7af1214cf
Remove extra lock() on UtilityProcessChild r=nika
https://hg.mozilla.org/integration/autoland/rev/a67c4ea1c8b3
Remove RecvNewContentRemoteDecoderManager on wrong class r=alwu
https://hg.mozilla.org/integration/autoland/rev/61dd9a9eb714
Release UtilityAudioDecoderChild on process shutdown r=alwu
https://hg.mozilla.org/integration/autoland/rev/f87c5fb2c36f
Use CanSend() instead of boolean r=nika
https://hg.mozilla.org/integration/autoland/rev/7e2ad8c47afb
Remove Utility AudioDecoder process naming to keep generic name r=alwu
https://hg.mozilla.org/integration/autoland/rev/d50fd0551159
Release UtilityProcessManager at the right moment r=nika
https://hg.mozilla.org/integration/autoland/rev/2221a97ebe97
Kill audioDecoder_Generic if hanging r=nika
https://hg.mozilla.org/integration/autoland/rev/707e4c9c8801
Release UtilityProcessChild later to allow mShutdownBlockers to act r=nika
https://hg.mozilla.org/integration/autoland/rev/1f64776a859a
Assert CleanShutown() of audioDecoder_* actor shuts down fine r=nika
https://hg.mozilla.org/integration/autoland/rev/620c85305800
Force audio element load to avoid edge case on media error handling r=alwu

Comment 21

[Sandor Molnar[:smolnar]]

Backed out for causing build bustage in toolkit/components/processtools/ProcInfo_common.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/596f4e86d30a3ea5f427847d483bcc87c0c0319a

Push with failures

Failure log

ERROR -  /builds/worker/checkouts/gecko/toolkit/components/processtools/ProcInfo_common.cpp:57:1: error: control reaches end of non-void function [-Werror=return-type]

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cfe28a83389
Merge UtilityAudioDecoderSandboxPolicy into UtilitySandboxPolicy r=jld
https://hg.mozilla.org/integration/autoland/rev/177f31e3f220
Remove UTILITY_AUDIO_DECODING_GENERIC r=nika,media-playback-reviewers,alwu
https://hg.mozilla.org/integration/autoland/rev/2ec48e535022
Use Utility process actor names for crash annotations r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/cf889ed1b472
Remove extra lock() on UtilityProcessChild r=nika
https://hg.mozilla.org/integration/autoland/rev/9957bea79dfc
Remove RecvNewContentRemoteDecoderManager on wrong class r=alwu
https://hg.mozilla.org/integration/autoland/rev/d8445c606d16
Release UtilityAudioDecoderChild on process shutdown r=alwu
https://hg.mozilla.org/integration/autoland/rev/1e0846e3f250
Use CanSend() instead of boolean r=nika
https://hg.mozilla.org/integration/autoland/rev/9196c8e260ef
Remove Utility AudioDecoder process naming to keep generic name r=alwu
https://hg.mozilla.org/integration/autoland/rev/bd09c870984f
Release UtilityProcessManager at the right moment r=nika
https://hg.mozilla.org/integration/autoland/rev/d45c5110187f
Kill audioDecoder_Generic if hanging r=nika
https://hg.mozilla.org/integration/autoland/rev/46d63e86fcfb
Release UtilityProcessChild later to allow mShutdownBlockers to act r=nika
https://hg.mozilla.org/integration/autoland/rev/d40e8b5496eb
Assert CleanShutown() of audioDecoder_* actor shuts down fine r=nika
https://hg.mozilla.org/integration/autoland/rev/83f35e186dd9
Force audio element load to avoid edge case on media error handling r=alwu

Comment 23

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/7cfe28a83389
https://hg.mozilla.org/mozilla-central/rev/177f31e3f220
https://hg.mozilla.org/mozilla-central/rev/2ec48e535022
https://hg.mozilla.org/mozilla-central/rev/cf889ed1b472
https://hg.mozilla.org/mozilla-central/rev/9957bea79dfc
https://hg.mozilla.org/mozilla-central/rev/d8445c606d16
https://hg.mozilla.org/mozilla-central/rev/1e0846e3f250
https://hg.mozilla.org/mozilla-central/rev/9196c8e260ef
https://hg.mozilla.org/mozilla-central/rev/bd09c870984f
https://hg.mozilla.org/mozilla-central/rev/d45c5110187f
https://hg.mozilla.org/mozilla-central/rev/46d63e86fcfb
https://hg.mozilla.org/mozilla-central/rev/d40e8b5496eb
https://hg.mozilla.org/mozilla-central/rev/83f35e186dd9

Comment 24

[:gerard-majax]

Attachment: 0001-Bug-1788596-Release-UtilityProcessManager-at-the-rig.patch (beta uplift)

Comment 25

[:gerard-majax]

Attachment: 0002-Bug-1788596-Release-UtilityAudioDecoderChild-on-proc.patch (beta uplift)

Comment 26

[:gerard-majax]

Attachment: 0001-Bug-1788596-Release-UtilityProcessManager-at-the-rig.patch (release uplift)

Comment 27

[:gerard-majax]

Attachment: 0002-Bug-1788596-Release-UtilityAudioDecoderChild-on-proc.patch (release uplift)