Use UniquePtr with IPC::Message more

NEW
Assigned to

Status

()

Core
IPC
P3
normal
2 years ago
11 months ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox48 affected)

Details

(Whiteboard: btpp-active)

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
There's a bunch of places where raw pointers to IPC::Message are passed around, including in IPDL-generated code. UniquePtr would be better, because it would be easier to figure out what is going on, and we've had leaks in the past due to not cleaning things up properly on error paths.
(Assignee)

Comment 1

2 years ago
Created attachment 8744975 [details] [diff] [review]
Use UniquePtr more in IPC code. WIP

This patch kind of spiralled out of control, and it is only lightly tested.
(Assignee)

Comment 2

2 years ago
With this patch, the IPDL generated code for Send methods looks like:

    mozilla::UniquePtr<IPC::Message> msg__ = PContent::Msg_PBrowserConstructor(MSG_ROUTING_CONTROL);

    Write(actor, (msg__).get(), false);
    Write(tabId, (msg__).get());

    [...]

    bool sendok__ = (mChannel).Send(mozilla::Move(msg__));

I suppose I should change the Write() methods to take const UniquePtr<Message>&. (I was using the last paragraph of the comment on UniquePtr to decide what types to use.)
Whiteboard: btpp-active
(Assignee)

Comment 3

2 years ago
This is green on L64 debug at least:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=35c404513bf7

One sketchy part is where I call into OnMessageReceivedFromLink() with a UniquePtr, such as in ThreadLink::EchoMessage():
  mChan->OnMessageReceivedFromLink(Move(*aMsg));

I also would like to figure out how to split this up a bit. The patch also needs various cleaning up, like using mozilla:: where it isn't actually needed.

Updated

11 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.