966467 - [e10s] Content processes should get different widgets than the main process

Status: VERIFIED FIXED

(Core :: Widget, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

There are a number of services and components that get created in all processes but that don't make sense in content processes. Some examples:

nsIDragService
nsIClipboard
nsIFilePicker
all the stuff related to printing

These things make OS calls that won't be available to content processes once they're sandboxed. Even now they sometimes crash. It would be better to avoid instantiating these services in child processes.

In addition, we often want to expose other versions of these services in content processes. For example, we already have nsFilePickerProxy, which just forwards all method calls to the parent. Pretty soon we're going to have other proxy services. The way nsFilePickerProxy is exposed is kind of hacky, and it would be nice to make clean interfaces for all this.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: xpcom-process-selector

This first patch allows us to selectively expose services to either the content process or the main process.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: disable-services

This patch disables a bunch of services that we don't want in content processes.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: content-widgets

This patch adds a file widgets/xpwidgets/nsContentProcessWidgetFactory.cpp. Its purpose is to create all the widgets that are cross-platform and meant only for content processes. To start with, it just contains the existing nsFilePickerProxy.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: clipboard-changes

This patch adds nsClipboardProxy, which I copied from the old Android widget code. I also expanded it a little to handle the selection clipboard that X has. It only handles text data in the clipboard, but we can fix that later.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8368815 [details] [diff] [review]
xpcom-process-selector

It would be nice to extend this in a followup to allow chrome.manifest-registered JS components to be able to be marked CONTENT_PROCESS_ONLY/MAIN_PROCESS_ONLY (for e.g. bug 930456).

Comment 7

[Benjamin Smedberg]

Comment on attachment 8368815 [details] [diff] [review]
xpcom-process-selector

How does this affect B2G? That has or will have main/app/content processes at least for the browser. Which processes will be used for each of those?

Comment 8

[Benjamin Smedberg]

Jorge this changes the layout of CIDEntry, which means that binary components cannot be compiled to work with multiple versions. We need to announce this to Skype/Norton toolbar at least and put it in the developer notes.

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

B2G uses GeckoProcessType_Content processes for everything that's not the main process. My strategy for dealing with this is:
1. Don't use these process selectors in the gonk widget code.
2. Exclude the nsContentProcessWidgetFactory.cpp code from B2G using an #ifdef (I had to add that after I posted the patch).

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a3c277cc5acc
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cca3c1af0066
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a43fb54ca20e
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2925006239ec

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #8)
> Jorge this changes the layout of CIDEntry, which means that binary
> components cannot be compiled to work with multiple versions. We need to
> announce this to Skype/Norton toolbar at least and put it in the developer
> notes.

Do we not version this somehow?

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a3c277cc5acc
https://hg.mozilla.org/mozilla-central/rev/cca3c1af0066
https://hg.mozilla.org/mozilla-central/rev/a43fb54ca20e
https://hg.mozilla.org/mozilla-central/rev/2925006239ec

Comment 13

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #8)
> Jorge this changes the layout of CIDEntry, which means that binary
> components cannot be compiled to work with multiple versions.

Can't work with 29 and 30, or can't ever work with multiple versions again?

Comment 15

[Paul Silaghi, QA [:pauly]]

Verified in the duplicate https://bugzilla.mozilla.org/show_bug.cgi?id=936089#c10