Convert PContentBridge to use endpoints

RESOLVED FIXED in Firefox 54

Status

()

Core
DOM: Content Processes
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

unspecified
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

10 months ago
Right now it contains:
  bridges PContent, PContent;

This should be replaced with the usage of endpoints.
(Assignee)

Updated

10 months ago
Assignee: nobody → continuation
(Assignee)

Updated

10 months ago
Depends on: 1334328
(Assignee)

Comment 1

10 months ago
try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=77698207146ec22bd281e020a87ed3021ae8f5d0

I'm not sure if we have any tests for this. If we do, hopefully try will run them. I mostly followed what my plugin changes did, so hopefully it is okay.
Comment hidden (mozreview-request)
(Assignee)

Updated

10 months ago
Attachment #8830987 - Flags: review?(kchen)
(Assignee)

Comment 3

10 months ago
I may have a simpler way to do this, so I'm cancelling review for now.
(Assignee)

Comment 4

10 months ago
Rather than sending a separate message back to the child with the endpoint, this returns the endpoint back with the initial sync call. A nice side effect is that this allows me to get rid of the odd ContentChild::mLastBridge field which was used to pass back a value in a very indirect way across the bridge call.
Comment hidden (mozreview-request)

Comment 6

10 months ago
mozreview-review
Comment on attachment 8830987 [details]
Bug 1333917 - Make ContentBridge use endpoints, not bridges.

https://reviewboard.mozilla.org/r/107646/#review110222

LGTM. This is not activly tested. To test you can run mochitest with --nested_oop switch locally (hopefully it still works). Project Mortar is using this so soon or later will know if it breaks anything.

::: dom/ipc/ContentParent.cpp:1021
(Diff revision 2)
>    }
> -  if (!child->SendBridgeToChildProcess(cpId)) {
> +  Endpoint<PContentBridgeParent> endpoint;
> +  if (!child->SendBridgeToChildProcess(cpId, &endpoint)) {
>      return nullptr;
>    }

Currently we always create a bridge after creating the child process. Maybe we can combine the CreateChildProcess message and BridgeToChildProcess message so we can avoid one sync message.
Attachment #8830987 - Flags: review?(kchen) → review+

Comment 7

10 months ago
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc0e726a478f
Make ContentBridge use endpoints, not bridges. r=kanru

Comment 8

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fc0e726a478f
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.