Closed Bug 1479645 Opened 6 years ago Closed 6 years ago

Don't unwrap CPOWs in middleman processes

Categories

(Core Graveyard :: Web Replay, defect)

defect
Not set
normal

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Unwrapping CPOWs in middleman processes doesn't work because they don't have a CPOW manager.  Because the message will also be received in the recording child of the middleman, avoiding this unwrapping shouldn't lead to any leaks.
Attachment #8996173 - Flags: review?(continuation)
Comment on attachment 8996173 [details] [diff] [review]
patch

Review of attachment 8996173 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense. Does the Unwrap in CrossProcessCpowHolder::ToObject() just not get called?
Attachment #8996173 - Flags: review?(continuation) → review+
Maybe you could assert in JavaScriptShared::Unwrap() that we're not in the middleman process.
(In reply to Andrew McCreight [:mccr8] (away Aug 6 - 10) from comment #1)
> Comment on attachment 8996173 [details] [diff] [review]
> patch
> 
> Review of attachment 8996173 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Makes sense. Does the Unwrap in CrossProcessCpowHolder::ToObject() just not
> get called?

Yeah, we receive async messages in both middleman and recording processes, but most messages are ignored by the message manager in the middleman (per DirectMessageToMiddleman in dom/base/nsFrameMessageManager.cpp).  I think that we run into the issue in this bug when one of those ignored messages contains a CPOW.

(In reply to Andrew McCreight [:mccr8] (away Aug 6 - 10) from comment #2)
> Maybe you could assert in JavaScriptShared::Unwrap() that we're not in the
> middleman process.

Sure, I'll add an assert.
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0f58c6bf907
Don't unwrap CPOWs in middleman processes, r=mccr8.
https://hg.mozilla.org/mozilla-central/rev/b0f58c6bf907
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: