1375022 - Consider removing the Msg_GetClipboard sync IPC

Status: NEW

(Core :: DOM: Copy & Paste and Drag & Drop, enhancement, P2)

Description

[Nika Layzell [:nika] (ni? for response)]

Currently, we have a sync IPC called Msg_GetClipboard which is called when the content process needs the contents of the clipboard for pasting purposes. This is called internally within nsClipboardProxy::GetData.

This currently means that the content process can query the clipboard at any time, however, we only ever request the clipboard's contents after:
a) Typing the Ctrl-V paste shortcut
b) Selecting the paste context menu
c) Selecting the paste action from another menu in the browser UI
d) Execute document.execCommand("paste") (which is currently not exposed to content)

To my understanding, all of these actions (except execCommand) are initiated within the parent process, and then sent to the content process, which then performs a sync IPC call to fetch the data again. I believe that they should be relatively straightforward to identify within the parent process before they are sent down.

The current use of sync IPC for GetClipboard has a few drawbacks:
a) It's slow (nobody likes a sync IPC), especially one which can fire multiple times for a single paste event.
b) You can't construct blobs over sync IPC from parent to child right now (this is blocking work like bug 1308007).
c) The content process is able to read the clipboard whenever it wants to, rather than only when it should be given access to its contents due to a user-initiated paste event.

This makes me think that it would be a good idea to put in the work to send down the clipboard data when sending messages to the content process to tell it about any clipboard paste triggers, and change the content process to read from the data which was sent down instead of using a sync IPC. 

There is one downside which I can think of, which is that this will make it harder to expose `document.execCommand("paste")` to websites (behind a permission of course) if we decide that that is valuable, as we will need to change the code for that method to probably spin a nested event loop in order to fetch the clipboard data we need. It's definitely not impossible, however.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Ehsan, I'm curious what your thoughts are about how hard this would be to execute. I am not certain where I would even start looking if I wanted to hook on the parent side, but I imagine that it is possible.

Comment 2

[Andrea Marchesini [:baku]]

We can check if there is a principal with clipboardRead permission, and in that case we can broadcast the clipboard content to each child process having that principal each time the clipboard content changes. In this way, if we have to proceed with the pasting, the data is already known by the process. I don't know if this is possible to do on any platform.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to Andrea Marchesini [:baku] from comment #2)
> We can check if there is a principal with clipboardRead permission, and in
> that case we can broadcast the clipboard content to each child process
> having that principal each time the clipboard content changes. In this way,
> if we have to proceed with the pasting, the data is already known by the
> process. I don't know if this is possible to do on any platform.

I'm not sure that doing the processing to be able to pass the clipboard data to the content process every time the clipboard changes is a good idea. This processing can be quite slow :-/

I'd much rather keep our processing to only times when we are actually going to need access to the data.

Comment 4

[(no longer active)]

(In reply to Michael Layzell [:mystor] from comment #0)
> This makes me think that it would be a good idea to put in the work to send
> down the clipboard data when sending messages to the content process to tell
> it about any clipboard paste triggers, and change the content process to
> read from the data which was sent down instead of using a sync IPC. 

The problem is that you'd need to run code inside the child process before it is ready to make a GetData() call as far as I can recall right now, so reading the contents of the clipboard in the parent in advance can be wasteful.  Here is an example: the parent for example may detect a Ctrl+V event, read the contents of the clipboard, send it to the child, and then the child may run the paste event handler on the page which may just preventDefault() the event in which case we wouldn't have needed to read the contents of the clipboard in the first place...

I think before thinking of approaches like this, it would be nice to investigate what it would take to move our clipboard access to another thread, so that we can sync IPC to a non-PContent protocol (for example a PBackground protocol or a custom top level protocol. )  Of course OS APIs may restrict what we can do here.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Definitely no sync IPC on PBackground. That is used for so many different things that sync operations there shouldn't be allowed.

Comment 6

[Sean Feng [:sefeng]]

The GTK3 manual said GTK+, however, is not thread safe. You should only use GTK+ and GDK from the thread gtk_init() and gtk_main() were called on. This is usually referred to as the “main thread”.,

So it may not be a good idea to move the clipboard access to a different thread.