feat: add a new contextBridge module

[MarshallOfSound]

What is this?

This PR adds a new renderer-side module called contextBridge that allows you to expose an API synchronously from the isolated context to the main world (and optionally back again) to achieve the following goals.

1. Make it really easy for folks currently doing the window.api = {} pattern to enable contextIsolation. The migration becomes bindAPIInMainWorld('api', {}).
2. Make it possible for synchronous APIs to exist between the main world and the isolated world. Some app logic is heavily tied to being synchronous and rather than ask folks to refactor their entire app we can just make this possible.
3. Kind of tied to (1) this would solve the "developer experience" issues we briefly touched on at summit around enabling contextIsolation. Once this is in, working and tested enabling contextIsolation by default no longer seems like a daunting task.

How?

The docs outline a lot of how this works from a users perspective so I'd recommend reading those. From a Behind The Scenes perspective it is all implemented using our current converter and binding infrastructure.

Primitives / Values get converted to base::Value and then converted back to v8::Value but in the other context. Functions get stored as v8::Global and then proxied into a bound cpp method that calls the original function in context A. Then does the primitive move trick and moves it to be the return value for context B. Their is special logic to proxy promise return values but you can see that both in the docs and the source code.

TODO:

• Freeze the API object that gets bound to ensure it is immutable in the other world
  • Document that the API object is frozen and immutable
• Function arguments need to be proxied
• Only expose debugGC in a testing build
• Code clean up
• CI Tests that it works
• CI Tests that we don't leak prototypes across the bridge
• CI Tests that sending functions isn't a memory leak
• Naming? Main World vs Main Process -- How do we make that clear?
• Prevent recursion destroying us.
  • Keep a weakmap of objects as we traverse, use already proxied versions if we can
  • Throw an error if we handle a thing with a recursion depth >= 10000?
• Use the V8 serializer - Copy from @nornagon PR and resolve conflicts with whoever lands first

Notes: Added new contextBridge module to make it easier to communicate between an isolated context and the main world

[felixrieseberg]

1. This thing will allow us to explain context isolation in a way that won't have developers wanting to look for a different job. It'll literally be night and day. I can't express enough how big of a boon for contextIsolation this is.

2. I'm not a big fan of using mainWorld as I'm afraid that it'll get confused with the main process. I don't have a better solution right now but I'd like to brainstorm some alternatives to see if this is the best we can come up with.

[codebytere]

I think since contextIsolation is mentioned in a BW constructor option and isn't super obvious from just a link to browser-window.md that it would be worthwhile to lower that barrier and re-copy it here or move it into a standalone document we could link to

[nornagon]

Can we implement the primitives that would be necessary to build this as a 3rd-party library and experiment with the specific API independently of the Electron release cycle until we can settle on an API that's good? I have some reservations about the API as written (particularly regarding the "reverse bindings" part), and I don't want to enshrine this in Electron without giving it a bit of a test-drive first.

[MarshallOfSound]

Can we implement the primitives that would be necessary to build this as a 3rd-party library and experiment with the specific API independently of the Electron release cycle until we can settle on an API that's good?

To keep this answer short, no, the way this is written it requires access to v8 contexts that native modules don't have access to let alone raw JS. Also to be fair this is already very barebones, we don't prescribe API shape or structure, just "give us some functions and we'll put them in the other context".

I have some reservations about the API as written (particularly regarding the "reverse bindings" part), and I don't want to enshrine this in Electron without giving it a bit of a test-drive first.

This API is flagged as "experimental" to make it clear the API (a) is new and relatively untested and (b) may be changed in the future. If you have specific usability / API concerns I'd love to hear the specifics so that they can be addressed.

[nornagon]

Add a short paragraph explaining why the reader might want to use this. What problems does this solve?

[nornagon]

This didn't get addressed...?

[nornagon]

i thought i commented on it before but it seems to have gotten lost; can we use a struct for this instead of std::tuple?

Suggested change

	std::tuple<v8::Global<v8::Function>, v8::Global<v8::Context>>;
	struct FunctionContextPair {
	v8::Global<v8::Function> function;
	v8::Global<v8::Context> context;
	};

[nornagon]

here too

Suggested change

	using WeakGlobalPair = std::tuple<v8::Global<v8::Value>, v8::Global<v8::Value>>;
	struct WeakGlobalPair {
	v8::Global<v8::Value> value_in_isolated_context;
	v8::Global<v8::Value> value_in_main_context;
	};

[nornagon]

Approving with above comments with the consideration that this code doesn't affect apps that don't call contextBridge.exposeInMainWorld. I still have some concerns about this implementation, but in the interests of moving this forward, let's land and iterate.

[release-clerk]

Release Notes Persisted

Added new contextBridge module to make it easier to communicate between an isolated context and the main world

[trop]

@MarshallOfSound has manually backported this PR to "6-0-x", please check out #20639

[trop]

@MarshallOfSound has manually backported this PR to "6-1-x", please check out #20639

[trop]

@MarshallOfSound has manually backported this PR to "7-0-x", please check out #20789

[trop]

@MarshallOfSound has manually backported this PR to "7-1-x", please check out #20789

[reZach]

@MarshallOfSound, since you wrote this code, can you think of a case where the values/objects passed through the contextBridge do not fully "come out" in the other context? I'll try to parse through your commits too when I have time.

ipcRenderer does not have access to the .on(channel, args) method after coming through the contextBridge.
preload.js

const {
    contextBridge,
    ipcRenderer
} = require("electron");

contextBridge.exposeInMainWorld(
    "electron", {
        ipcRenderer: ipcRenderer
    }
);

index.html

<!doctype html>
<html lang="en-US">
  <head>
    <meta charset="UTF-8">
    <title>Getting Started!</title>
  </head>
  <body>
    <script>
        console.log("In index.html");
        try {
          window.electron.ipcRenderer.on("event2", (a, b) => {});
        } catch (err){
          // log error
          console.error(err); // logs error; ".on" is not a method
        }
    </script>
  </body>
</html>

[MarshallOfSound]

@reZach This is all in the documentation, prototypes are not sent over the bridge.

https://electronjs.org/docs/api/context-bridge#parameter--error--return-type-support

Also as a side note, exposing the entire ipcRnederer module kind of defeats the purpose of context isolation. You should expose the minimal required APIs, not wildcard style APIs

[reZach]

Thank you for clarifying that @MarshallOfSound. I missed that bit!

There is no other way to configure the ipcRenderer's listeners that are based on logic in the renderer process without setting contextIsolation=false. I could copy a minimal object via contextBridge, but what I tested, the entire set of ipcRenderer properties of the existing instance must be sent along or else the ipcRenderer doesn't work in the renderer process.

The entire ipcRenderer instance is ideal to have in the renderer process because electron libraries might need to configure listeners based on library data itself. Ie. a call couldn't exist like this in the preload script because the preload script doesn't have access to this data.

Desired

renderer

class LibraryClass {
    constructor(myValue = 1) {
        this.classValue = myValue;

        window.electron.ipcRenderer.on("response", (IpcRendererEvent, args) => {
            if (args.success) this.classValue++;
        });
    }

    send() {
        window.electron.ipcRenderer.send("request", {
            data: this.classValue
        });
    }
}

main

let win = new BrowserWindow({...});

ipcMain.on("request", (IpcMainEvent, args) => {
    // Use node module (ie. "fs");
    // perhaps save value of args.data to file with a timestamp

    win.webContents.send("response", {
        success: true
    });
});

I've posted a $230 bounty for this issue. I imagine as library authors switch to use contextIsolation, they will run into this same problem too.

Workaround

For the time being I'm setting contextIsolation to false, and my preload script looks something like this:

const { ipcRenderer } = require("electron");

window.electron= {
    ipcRenderer
};