Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: tsify web-contents #24325

Merged
merged 10 commits into from Jul 6, 2020
Merged

chore: tsify web-contents #24325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4be6319
chore: tsify web-contents
nornagon Jun 26, 2020
b4e6ed5
export function
nornagon Jun 26, 2020
58a6300
also tsify navigation-controller
nornagon Jun 26, 2020
3b7ec89
whoops
nornagon Jun 26, 2020
ad40b28
fix crash
nornagon Jun 29, 2020
61dd57c
different approach for navigation controller
nornagon Jun 29, 2020
e74e4f7
stop accessing webContents.webContents
nornagon Jul 1, 2020
5ce4dce
Merge remote-tracking branch 'origin/master' into tsify-web-contents
nornagon Jul 1, 2020
8363ea2
more removing of webContents.webContents
nornagon Jul 1, 2020
5d9c995
fix postMessage
nornagon Jul 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions filenames.auto.gni
View file
Open in desktop
Expand Up @@ -222,7 +222,7 @@ auto_filenames = {
"lib/browser/api/view.ts",
"lib/browser/api/views/image-view.ts",
"lib/browser/api/web-contents-view.ts",
"lib/browser/api/web-contents.js",
"lib/browser/api/web-contents.ts",
"lib/browser/chrome-extension-shim.ts",
"lib/browser/default-menu.ts",
"lib/browser/desktop-capturer.ts",
Expand All @@ -234,7 +234,7 @@ auto_filenames = {
"lib/browser/ipc-main-internal-utils.ts",
"lib/browser/ipc-main-internal.ts",
"lib/browser/message-port-main.ts",
"lib/browser/navigation-controller.js",
"lib/browser/navigation-controller.ts",
"lib/browser/remote/objects-registry.ts",
"lib/browser/remote/server.ts",
"lib/browser/rpc-server.ts",
Expand Down
180 changes: 95 additions & 85 deletions lib/browser/api/web-contents.js → lib/browser/api/web-contents.ts
View file
Open in desktop
@@ -1,17 +1,15 @@
'use strict';

const { EventEmitter } = require('events');
const electron = require('electron');
const path = require('path');
const url = require('url');
const { app, ipcMain, session } = electron;

const { internalWindowOpen } = require('@electron/internal/browser/guest-window-manager');
const NavigationController = require('@electron/internal/browser/navigation-controller');
const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal');
const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils');
const { parseFeatures } = require('@electron/internal/common/parse-features-string');
const { MessagePortMain } = require('@electron/internal/browser/message-port-main');
import { app, ipcMain, session, deprecate } from 'electron';
import type { MenuItem, MenuItemConstructorOptions, WebContentsInternal } from 'electron';

import * as url from 'url';
import * as path from 'path';
import { internalWindowOpen } from '../guest-window-manager';
import { NavigationController } from '../navigation-controller';
import { ipcMainInternal } from '../ipc-main-internal';
import * as ipcMainUtils from '../ipc-main-internal-utils';
import { parseFeatures } from '../../common/parse-features-string';
import { MessagePortMain } from '../message-port-main';
import { EventEmitter } from 'events';

// session is not used here, the purpose is to make sure session is initalized
// before the webContents module.
Expand All @@ -23,8 +21,18 @@ const getNextId = function () {
return ++nextId;
};

/* eslint-disable camelcase */
type MediaSize = {
name: string,
custom_display_name: string,
height_microns: number,
width_microns: number,
is_default?: 'true',
}
/* eslint-enable camelcase */

// Stock page sizes
const PDFPageSizes = {
const PDFPageSizes: Record<string, MediaSize> = {
A5: {
custom_display_name: 'A5',
height_microns: 210000,
Expand Down Expand Up @@ -67,8 +75,8 @@ const PDFPageSizes = {
// Default printing setting
const defaultPrintingSetting = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of all the as casts below we should type this as Electron.WhateverThePrintingSettingsInterfaceIs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there exists any type in electron.d.ts that matches this.

$ rg '\bpageRange\b'
lib/browser/api/web-contents.ts
78:  pageRange: [] as {from: number, to: number}[],
279:    printSettings.pageRange = [{
$

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nornagon That's because according to the docs it's pageRanges

Copy link
Member Author

@nornagon nornagon Jul 2, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also true of several other members of this struct:

$ rg '\bshouldPrintBackgrounds\b'
lib/browser/api/web-contents.ts
84:  shouldPrintBackgrounds: false,
258:    printSettings.shouldPrintBackgrounds = options.printBackground;
$ rg '\bheaderFooterEnabled\b'
lib/browser/api/web-contents.ts
81:  headerFooterEnabled: false,
287:    printSettings.headerFooterEnabled = true;
$ rg '\bgenerateDraftData\b'
lib/browser/api/web-contents.ts
96:  generateDraftData: true,
$

I don't think the struct you're thinking of is the same as this struct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the type of this struct is related to these

// Customizable.
pageRange: [],
mediaSize: {},
pageRange: [] as {from: number, to: number}[],
mediaSize: {} as MediaSize,
landscape: false,
headerFooterEnabled: false,
marginsType: 0,
Expand All @@ -93,18 +101,18 @@ const defaultPrintingSetting = {
copies: 1,
// 2 = color - see ColorModel in //printing/print_job_constants.h
color: 2,
collate: true
collate: true,
printerType: 2,
title: undefined as string | undefined,
url: undefined as string | undefined
};

// JavaScript implementations of WebContents.
const binding = process._linkedBinding('electron_browser_web_contents');
const { WebContents } = binding;
const { WebContents } = binding as { WebContents: { prototype: WebContentsInternal } };

Object.setPrototypeOf(NavigationController.prototype, EventEmitter.prototype);
Object.setPrototypeOf(WebContents.prototype, NavigationController.prototype);
Object.setPrototypeOf(WebContents.prototype, EventEmitter.prototype);

// WebContents::send(channel, args..)
// WebContents::sendToAll(channel, args..)
WebContents.prototype.send = function (channel, ...args) {
if (typeof channel !== 'string') {
throw new Error('Missing required channel argument');
Expand All @@ -123,17 +131,6 @@ WebContents.prototype.postMessage = function (...args) {
this._postMessage(...args);
};

WebContents.prototype.sendToAll = function (channel, ...args) {
if (typeof channel !== 'string') {
throw new Error('Missing required channel argument');
}

const internal = false;
const sendToAll = true;

return this._send(internal, sendToAll, channel, args);
};

WebContents.prototype._sendInternal = function (channel, ...args) {
if (typeof channel !== 'string') {
throw new Error('Missing required channel argument');
Expand Down Expand Up @@ -185,15 +182,15 @@ const webFrameMethods = [
'insertText',
'removeInsertedCSS',
'setVisualZoomLevelLimits'
];
] as ('insertCSS' | 'insertText' | 'removeInsertedCSS' | 'setVisualZoomLevelLimits')[];

for (const method of webFrameMethods) {
WebContents.prototype[method] = function (...args) {
WebContents.prototype[method] = function (...args: any[]): Promise<any> {
return ipcMainUtils.invokeInWebContents(this, false, 'ELECTRON_INTERNAL_RENDERER_WEB_FRAME_METHOD', method, ...args);
};
}

const waitTillCanExecuteJavaScript = async (webContents) => {
const waitTillCanExecuteJavaScript = async (webContents: WebContentsInternal) => {
if (webContents.getURL() && !webContents.isLoadingMainFrame()) return;

return new Promise((resolve) => {
Expand Down Expand Up @@ -326,7 +323,7 @@ WebContents.prototype.printToPDF = function (options) {
height_microns: Math.ceil(pageSize.height),
width_microns: Math.ceil(pageSize.width)
};
} else if (PDFPageSizes[pageSize]) {
} else if (Object.prototype.hasOwnProperty.call(PDFPageSizes, pageSize)) {
printSettings.mediaSize = PDFPageSizes[pageSize];
} else {
const error = new Error(`Unsupported pageSize: ${pageSize}`);
Expand Down Expand Up @@ -360,14 +357,14 @@ WebContents.prototype.print = function (options = {}, callback) {
throw new Error('height and width properties are required for pageSize');
}
// Dimensions in Microns - 1 meter = 10^6 microns
options.mediaSize = {
(options as any).mediaSize = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These any casts shouldn't be needed if the original options object is typed correctly

name: 'CUSTOM',
custom_display_name: 'Custom',
height_microns: Math.ceil(pageSize.height),
width_microns: Math.ceil(pageSize.width)
};
} else if (PDFPageSizes[pageSize]) {
options.mediaSize = PDFPageSizes[pageSize];
(options as any).mediaSize = PDFPageSizes[pageSize];
} else {
throw new Error(`Unsupported pageSize: ${pageSize}`);
}
Expand Down Expand Up @@ -410,23 +407,23 @@ WebContents.prototype.loadFile = function (filePath, options = {}) {
}));
};

const addReplyToEvent = (event) => {
event.reply = (...args) => {
const addReplyToEvent = (event: any) => {
event.reply = (...args: any[]) => {
event.sender.sendToFrame(event.frameId, ...args);
};
};

const addReplyInternalToEvent = (event) => {
const addReplyInternalToEvent = (event: any) => {
Object.defineProperty(event, '_replyInternal', {
configurable: false,
enumerable: false,
value: (...args) => {
value: (...args: any[]) => {
event.sender._sendToFrameInternal(event.frameId, ...args);
}
});
};

const addReturnValueToEvent = (event) => {
const addReturnValueToEvent = (event: any) => {
Object.defineProperty(event, 'returnValue', {
set: (value) => event.sendReply([value]),
get: () => {}
Expand All @@ -436,14 +433,30 @@ const addReturnValueToEvent = (event) => {
// Add JavaScript wrappers for WebContents class.
WebContents.prototype._init = function () {
// The navigation controller.
NavigationController.call(this, this);
const navigationController = new NavigationController(this);
this.loadURL = navigationController.loadURL.bind(navigationController);
nornagon marked this conversation as resolved.
Show resolved Hide resolved
this.getURL = navigationController.getURL.bind(navigationController);
this.stop = navigationController.stop.bind(navigationController);
this.reload = navigationController.reload.bind(navigationController);
this.reloadIgnoringCache = navigationController.reloadIgnoringCache.bind(navigationController);
this.canGoBack = navigationController.canGoBack.bind(navigationController);
this.canGoForward = navigationController.canGoForward.bind(navigationController);
this.canGoToIndex = navigationController.canGoToIndex.bind(navigationController);
this.canGoToOffset = navigationController.canGoToOffset.bind(navigationController);
this.clearHistory = navigationController.clearHistory.bind(navigationController);
this.goBack = navigationController.goBack.bind(navigationController);
this.goForward = navigationController.goForward.bind(navigationController);
this.goToIndex = navigationController.goToIndex.bind(navigationController);
this.goToOffset = navigationController.goToOffset.bind(navigationController);
this.getActiveIndex = navigationController.getActiveIndex.bind(navigationController);
this.length = navigationController.length.bind(navigationController);

// Every remote callback from renderer process would add a listener to the
// render-view-deleted event, so ignore the listeners warning.
this.setMaxListeners(0);

// Dispatch IPC messages to the ipc module.
this.on('-ipc-message', function (event, internal, channel, args) {
this.on('-ipc-message' as any, function (this: WebContentsInternal, event: any, internal: boolean, channel: string, args: any[]) {
if (internal) {
addReplyInternalToEvent(event);
ipcMainInternal.emit(channel, event, ...args);
Expand All @@ -454,21 +467,21 @@ WebContents.prototype._init = function () {
}
});

this.on('-ipc-invoke', function (event, internal, channel, args) {
event._reply = (result) => event.sendReply({ result });
event._throw = (error) => {
this.on('-ipc-invoke' as any, function (event: any, internal: boolean, channel: string, args: any[]) {
event._reply = (result: any) => event.sendReply({ result });
event._throw = (error: Error) => {
console.error(`Error occurred in handler for '${channel}':`, error);
event.sendReply({ error: error.toString() });
};
const target = internal ? ipcMainInternal : ipcMain;
if (target._invokeHandlers.has(channel)) {
target._invokeHandlers.get(channel)(event, ...args);
if ((target as any)._invokeHandlers.has(channel)) {
(target as any)._invokeHandlers.get(channel)(event, ...args);
} else {
event._throw(`No handler registered for '${channel}'`);
}
});

this.on('-ipc-message-sync', function (event, internal, channel, args) {
this.on('-ipc-message-sync' as any, function (this: WebContentsInternal, event: any, internal: boolean, channel: string, args: any[]) {
addReturnValueToEvent(event);
if (internal) {
addReplyInternalToEvent(event);
Expand All @@ -480,15 +493,15 @@ WebContents.prototype._init = function () {
}
});

this.on('-ipc-ports', function (event, internal, channel, message, ports) {
this.on('-ipc-ports' as any, function (event: any, internal: boolean, channel: string, message: any, ports: any[]) {
event.ports = ports.map(p => new MessagePortMain(p));
ipcMain.emit(channel, event, message);
});

// Handle context menu action request from pepper plugin.
this.on('pepper-context-menu', function (event, params, callback) {
this.on('pepper-context-menu' as any, function (event: any, params: {x: number, y: number, menu: Array<(MenuItemConstructorOptions) | (MenuItem)>}, callback: () => void) {
// Access Menu via electron.Menu to prevent circular require.
const menu = electron.Menu.buildFromTemplate(params.menu);
const menu = require('electron').Menu.buildFromTemplate(params.menu);
menu.popup({
window: event.sender.getOwnerBrowserWindow(),
x: params.x,
Expand All @@ -506,14 +519,14 @@ WebContents.prototype._init = function () {
});

// The devtools requests the webContents to reload.
this.on('devtools-reload-page', function () {
this.on('devtools-reload-page', function (this: WebContentsInternal) {
this.reload();
});

if (this.getType() !== 'remote') {
// Make new windows requested by links behave like "window.open".
this.on('-new-window', (event, url, frameName, disposition,
rawFeatures, referrer, postData) => {
this.on('-new-window' as any, (event: any, url: string, frameName: string, disposition: string,
rawFeatures: string, referrer: string, postData: string) => {
const { options, webPreferences, additionalFeatures } = parseFeatures(rawFeatures);
const mergedOptions = {
show: true,
Expand All @@ -529,9 +542,9 @@ WebContents.prototype._init = function () {

// Create a new browser window for the native implementation of
// "window.open", used in sandbox and nativeWindowOpen mode.
this.on('-add-new-contents', (event, webContents, disposition,
userGesture, left, top, width, height, url, frameName,
referrer, rawFeatures, postData) => {
this.on('-add-new-contents' as any, (event: any, webContents: WebContentsInternal, disposition: string,
userGesture: boolean, left: number, top: number, width: number, height: number, url: string, frameName: string,
referrer: string, rawFeatures: string, postData: string) => {
if ((disposition !== 'foreground-tab' && disposition !== 'new-window' &&
disposition !== 'background-tab')) {
event.preventDefault();
Expand All @@ -554,7 +567,7 @@ WebContents.prototype._init = function () {

const prefs = this.getWebPreferences() || {};
if (prefs.webviewTag && prefs.contextIsolation) {
electron.deprecate.log('Security Warning: A WebContents was just created with both webviewTag and contextIsolation enabled. This combination is fundamentally less secure and effectively bypasses the protections of contextIsolation. We strongly recommend you move away from webviews to OOPIF or BrowserView in order for your app to be more secure');
deprecate.log('Security Warning: A WebContents was just created with both webviewTag and contextIsolation enabled. This combination is fundamentally less secure and effectively bypasses the protections of contextIsolation. We strongly recommend you move away from webviews to OOPIF or BrowserView in order for your app to be more secure');
}
}

Expand Down Expand Up @@ -599,28 +612,25 @@ WebContents.prototype._init = function () {
};

// Public APIs.
module.exports = {
create (options = {}) {
return binding.create(options);
},

fromId (id) {
return binding.fromId(id);
},
export function create (options = {}) {
return binding.create(options);
}

getFocusedWebContents () {
let focused = null;
for (const contents of binding.getAllWebContents()) {
if (!contents.isFocused()) continue;
if (focused == null) focused = contents;
// Return webview web contents which may be embedded inside another
// web contents that is also reporting as focused
if (contents.getType() === 'webview') return contents;
}
return focused;
},
export function fromId (id: string) {
return binding.fromId(id);
}

getAllWebContents () {
return binding.getAllWebContents();
export function getFocusedWebContents () {
let focused = null;
for (const contents of binding.getAllWebContents()) {
if (!contents.isFocused()) continue;
if (focused == null) focused = contents;
// Return webview web contents which may be embedded inside another
// web contents that is also reporting as focused
if (contents.getType() === 'webview') return contents;
}
};
return focused;
}
export function getAllWebContents () {
return binding.getAllWebContents();
}
2 changes: 1 addition & 1 deletion lib/browser/guest-window-manager.js
View file
Open in desktop
Expand Up @@ -56,7 +56,7 @@ const mergeBrowserWindowOptions = function (embedder, options) {
let parentOptions = embedder.browserWindowOptions;

// if parent's visibility is available, that overrides 'show' flag (#12125)
const win = BrowserWindow.fromWebContents(embedder.webContents);
const win = BrowserWindow.fromWebContents(embedder);
if (win != null) {
parentOptions = {
...win.getBounds(),
Expand Down