Closed Bug 1513101 Opened 11 months ago Closed 10 months ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox65 --- fixed
firefox66 --- fixed

People

(Reporter: mjf, Assigned: mjf)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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
Assignee: nobody → mfroman
Rank: 10
Priority: -- → P2
Pushed by mfroman@mozilla.com:
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
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.
(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.
https://hg.mozilla.org/mozilla-central/rev/cb9676e832d2
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
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
Attachment #9030342 - Flags: approval-mozilla-beta?
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.
Attachment #9030342 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify-
Blocks: RDD
You need to log in before you can comment on or make changes to this bug.