1513101 - Re-add L section for HANDLES_DUP_BROKER to RDD Win sanbox to fix mochitest crashes

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P2)

Description

[Michael Froman [:mjf]]

This rule was removed, but later, when trying to pref on RDD/AV1 we saw mochitest assertion crashes on Win in crash reporter code[1].  Re-adding this rule fixes the crashes.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae8137554156f3fb1f2a2900dd9b082d3011b3cb&selectedJob=215749193

Comment 1

[Michael Froman [:mjf]]

Attachment: Bug 1513101 - re-add L section for HANDLES_DUP_BROKER to RDD Win sanbox to fix mochitest crashes. r?bobowen!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb9676e832d2
re-add L section for HANDLES_DUP_BROKER to RDD Win sanbox to fix mochitest crashes. r=bobowen

Comment 3

[Bob Owen (:bobowen)]

I'm already getting frustrated about not having comments relating to a bug in one place ...

> In D14109#353763, @mjf wrote:
>> In D14109#352229, @bobowen wrote:
>> I don't want to delay this, but please would you file a follow-up to investigate and hopefully fix this.
>> I'm guessing that this happens because we fail to allocate the shared memory here:
>> https://searchfox.org/mozilla-central/rev/fd62b95c187a40b328d9e7fd9d848833a6942b57/ipc/glue/CrashReporterClient.h#40
>
> So I'm clear, do think this is something that RDD is doing incorrectly (and we should investigate as a A/V playback bug) or something that is a broader issue (and is a sandbox/crashreporting bug)?

Well, I guess RDD might be able to check and submit on the correct thread, but I actually think this is probably a sandbox/crashreporter thing.
This may well be existing code that the sandbox breaks (or would break without this rule), so I'm happy for it to be filed under Security: Process Sandboxing.

Comment 4

[Michael Froman [:mjf]]

(In reply to Bob Owen (:bobowen) from comment #3)
> Well, I guess RDD might be able to check and submit on the correct thread,
> but I actually think this is probably a sandbox/crashreporter thing.
> This may well be existing code that the sandbox breaks (or would break
> without this rule), so I'm happy for it to be filed under Security: Process
> Sandboxing.
Thank you for the clarification!  Created Bug 1513348.

Comment 5

[Cristina Coroiu [:ccoroiu]]

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

Comment 6

[Nils Ohlmeier [:drno]]

Comment on attachment 9030342 [details]
Bug 1513101 - re-add L section for HANDLES_DUP_BROKER to RDD Win sanbox to fix mochitest crashes. r?bobowen!

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: This is prerequisite for uplifting bug 1452146, which finally allows Firefox users to watch video content with the new AV1 codec.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1452146

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It only activates a feature in the sandbox code used in other parts of Firefox already.

String changes made/needed: N/A

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9030342 [details]
Bug 1513101 - re-add L section for HANDLES_DUP_BROKER to RDD Win sanbox to fix mochitest crashes. r?bobowen!

[Triage Comment]
Needed for flipping the AV1 pref on Windows. Approved for 65.0b5.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1ad308897e69