feat: replace BrowserView with WebContentsView (#40759)

* feat: replace BrowserView with WebContentsView

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* Update appveyor.yml

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Jeremy Rose <jeremya@chromium.org>

spec/api-browser-view-spec.ts

@@ -13,7 +13,7 @@ describe('BrowserView module', () => {
   let view: BrowserView;
 
   beforeEach(() => {
-    expect(webContents.getAllWebContents()).to.have.length(0);
+    expect(webContents.getAllWebContents().length).to.equal(0, 'expected no webContents to exist');
     w = new BrowserWindow({
       show: false,
       width: 400,
@@ -37,7 +37,7 @@ describe('BrowserView module', () => {
       await p;
     }
 
-    expect(webContents.getAllWebContents()).to.have.length(0);
+    expect(webContents.getAllWebContents().length).to.equal(0, 'expected no webContents to exist');
   });
 
   it('sets the correct class name on the prototype', () => {
@@ -49,21 +49,28 @@ describe('BrowserView module', () => {
     await wc.loadURL('about:blank');
 
     view = new BrowserView({ webContents: wc } as any);
+    expect(view.webContents === wc).to.be.true('view.webContents === wc');
 
     expect(view.webContents.getURL()).to.equal('about:blank');
   });
 
+  it('has type browserView', () => {
+    view = new BrowserView();
+    expect(view.webContents.getType()).to.equal('browserView');
+  });
+
   describe('BrowserView.setBackgroundColor()', () => {
     it('does not throw for valid args', () => {
       view = new BrowserView();
       view.setBackgroundColor('#000');
     });
 
-    it('throws for invalid args', () => {
+    // We now treat invalid args as "no background".
+    it('does not throw for invalid args', () => {
       view = new BrowserView();
       expect(() => {
-        view.setBackgroundColor(null as any);
-      }).to.throw(/conversion failure/);
+        view.setBackgroundColor({} as any);
+      }).not.to.throw();
     });
 
     // Linux and arm64 platforms (WOA and macOS) do not return any capture sources
@@ -128,7 +135,148 @@ describe('BrowserView module', () => {
       view = new BrowserView();
       expect(() => {
         view.setAutoResize(null as any);
-      }).to.throw(/conversion failure/);
+      }).to.throw(/Invalid auto resize options/);
+    });
+
+    it('does not resize when the BrowserView has no AutoResize', () => {
+      view = new BrowserView();
+      w.addBrowserView(view);
+      view.setBounds({ x: 0, y: 0, width: 400, height: 200 });
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 400,
+        height: 200
+      });
+      w.setSize(800, 400);
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 400,
+        height: 200
+      });
+    });
+
+    it('resizes horizontally when the window is resized horizontally', () => {
+      view = new BrowserView();
+      view.setAutoResize({ width: true, height: false });
+      w.addBrowserView(view);
+      view.setBounds({ x: 0, y: 0, width: 400, height: 200 });
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 400,
+        height: 200
+      });
+      w.setSize(800, 400);
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 800,
+        height: 200
+      });
+    });
+
+    it('resizes vertically when the window is resized vertically', () => {
+      view = new BrowserView();
+      view.setAutoResize({ width: false, height: true });
+      w.addBrowserView(view);
+      view.setBounds({ x: 0, y: 0, width: 200, height: 400 });
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 200,
+        height: 400
+      });
+      w.setSize(400, 800);
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 200,
+        height: 800
+      });
+    });
+
+    it('resizes both vertically and horizontally when the window is resized', () => {
+      view = new BrowserView();
+      view.setAutoResize({ width: true, height: true });
+      w.addBrowserView(view);
+      view.setBounds({ x: 0, y: 0, width: 400, height: 400 });
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 400,
+        height: 400
+      });
+      w.setSize(800, 800);
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 800,
+        height: 800
+      });
+    });
+
+    it('resizes proportionally', () => {
+      view = new BrowserView();
+      view.setAutoResize({ width: true, height: false });
+      w.addBrowserView(view);
+      view.setBounds({ x: 0, y: 0, width: 200, height: 100 });
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 200,
+        height: 100
+      });
+      w.setSize(800, 400);
+      expect(view.getBounds()).to.deep.equal({
+        x: 0,
+        y: 0,
+        width: 600,
+        height: 100
+      });
+    });
+
+    it('does not move x if horizontal: false', () => {
+      view = new BrowserView();
+      view.setAutoResize({ width: true });
+      w.addBrowserView(view);
+      view.setBounds({ x: 200, y: 0, width: 200, height: 100 });
+      w.setSize(800, 400);
+      expect(view.getBounds()).to.deep.equal({
+        x: 200,
+        y: 0,
+        width: 600,
+        height: 100
+      });
+    });
+
+    it('moves x if horizontal: true', () => {
+      view = new BrowserView();
+      view.setAutoResize({ horizontal: true });
+      w.addBrowserView(view);
+      view.setBounds({ x: 200, y: 0, width: 200, height: 100 });
+      w.setSize(800, 400);
+      expect(view.getBounds()).to.deep.equal({
+        x: 400,
+        y: 0,
+        width: 400,
+        height: 100
+      });
+    });
+
+    it('moves x if horizontal: true width: true', () => {
+      view = new BrowserView();
+      view.setAutoResize({ horizontal: true, width: true });
+      w.addBrowserView(view);
+      view.setBounds({ x: 200, y: 0, width: 200, height: 100 });
+      w.setSize(800, 400);
+      expect(view.getBounds()).to.deep.equal({
+        x: 400,
+        y: 0,
+        width: 400,
+        height: 100
+      });
     });
   });
 
@@ -288,6 +436,16 @@ describe('BrowserView module', () => {
       w2.close();
       w2.destroy();
     });
+
+    it('does not cause a crash when used for view with destroyed web contents', async () => {
+      const w2 = new BrowserWindow({ show: false });
+      const view = new BrowserView();
+      view.webContents.close();
+      w2.addBrowserView(view);
+      w2.webContents.loadURL('about:blank');
+      await once(w2.webContents, 'did-finish-load');
+      w2.close();
+    });
   });
 
   describe('BrowserWindow.removeBrowserView()', () => {

spec/api-browser-window-spec.ts

@@ -149,7 +149,7 @@ describe('BrowserWindow module', () => {
     it('should emit beforeunload handler', async () => {
       await w.loadFile(path.join(__dirname, 'fixtures', 'api', 'beforeunload-false.html'));
       w.close();
-      await once(w.webContents, 'before-unload-fired');
+      await once(w.webContents, '-before-unload-fired');
     });
 
     it('should not crash when keyboard event is sent before closing', async () => {
@@ -241,7 +241,7 @@ describe('BrowserWindow module', () => {
     it('should emit beforeunload event', async function () {
       await w.loadFile(path.join(__dirname, 'fixtures', 'api', 'beforeunload-false.html'));
       w.webContents.executeJavaScript('window.close()', true);
-      await once(w.webContents, 'before-unload-fired');
+      await once(w.webContents, '-before-unload-fired');
     });
   });
 
@@ -4052,14 +4052,14 @@ describe('BrowserWindow module', () => {
     it('returning false would prevent close', async () => {
       await w.loadFile(path.join(__dirname, 'fixtures', 'api', 'beforeunload-false.html'));
       w.close();
-      const [, proceed] = await once(w.webContents, 'before-unload-fired');
+      const [, proceed] = await once(w.webContents, '-before-unload-fired');
       expect(proceed).to.equal(false);
     });
 
     it('returning empty string would prevent close', async () => {
       await w.loadFile(path.join(__dirname, 'fixtures', 'api', 'beforeunload-empty-string.html'));
       w.close();
-      const [, proceed] = await once(w.webContents, 'before-unload-fired');
+      const [, proceed] = await once(w.webContents, '-before-unload-fired');
       expect(proceed).to.equal(false);
     });
 
@@ -4075,9 +4075,9 @@ describe('BrowserWindow module', () => {
       // the SuddenTerminationStatus message have been flushed.
       await once(w.webContents, 'console-message');
       w.close();
-      await once(w.webContents, 'before-unload-fired');
+      await once(w.webContents, '-before-unload-fired');
       w.close();
-      await once(w.webContents, 'before-unload-fired');
+      await once(w.webContents, '-before-unload-fired');
 
       w.webContents.removeListener('destroyed', destroyListener);
       const wait = once(w, 'closed');
@@ -4097,7 +4097,7 @@ describe('BrowserWindow module', () => {
       // the SuddenTerminationStatus message have been flushed.
       await once(w.webContents, 'console-message');
       w.reload();
-      // Chromium does not emit 'before-unload-fired' on WebContents for
+      // Chromium does not emit '-before-unload-fired' on WebContents for
       // navigations, so we have to use other ways to know if beforeunload
       // is fired.
       await emittedUntil(w.webContents, 'console-message', isBeforeUnload);
@@ -4121,7 +4121,7 @@ describe('BrowserWindow module', () => {
       // the SuddenTerminationStatus message have been flushed.
       await once(w.webContents, 'console-message');
       w.loadURL('about:blank');
-      // Chromium does not emit 'before-unload-fired' on WebContents for
+      // Chromium does not emit '-before-unload-fired' on WebContents for
       // navigations, so we have to use other ways to know if beforeunload
       // is fired.
       await emittedUntil(w.webContents, 'console-message', isBeforeUnload);

spec/api-web-contents-view-spec.ts

@@ -1,18 +1,39 @@
-import { closeWindow } from './lib/window-helpers';
+import { closeAllWindows } from './lib/window-helpers';
+import { expect } from 'chai';
 
-import { BaseWindow, WebContentsView } from 'electron/main';
+import { BaseWindow, View, WebContentsView } from 'electron/main';
+import { once } from 'node:events';
 
 describe('WebContentsView', () => {
-  let w: BaseWindow;
+  afterEach(closeAllWindows);
 
-  afterEach(async () => {
-    await closeWindow(w as any);
-    w = null as unknown as BaseWindow;
+  it('can be instantiated with no arguments', () => {
+    // eslint-disable-next-line no-new
+    new WebContentsView();
+  });
+
+  it('can be instantiated with no webPreferences', () => {
+    // eslint-disable-next-line no-new
+    new WebContentsView({});
   });
 
   it('can be used as content view', () => {
-    w = new BaseWindow({ show: false });
-    w.setContentView(new WebContentsView({}));
+    const w = new BaseWindow({ show: false });
+    w.setContentView(new WebContentsView());
+  });
+
+  it('can be removed after a close', async () => {
+    const w = new BaseWindow({ show: false });
+    const v = new View();
+    const wcv = new WebContentsView();
+    w.setContentView(v);
+    v.addChildView(wcv);
+    await wcv.webContents.loadURL('about:blank');
+    const destroyed = once(wcv.webContents, 'destroyed');
+    wcv.webContents.executeJavaScript('window.close()');
+    await destroyed;
+    expect(wcv.webContents.isDestroyed()).to.be.true();
+    v.removeChildView(wcv);
   });
 
   function triggerGCByAllocation () {
@@ -25,7 +46,7 @@ describe('WebContentsView', () => {
 
   it('doesn\'t crash when GCed during allocation', (done) => {
     // eslint-disable-next-line no-new
-    new WebContentsView({});
+    new WebContentsView();
     setTimeout(() => {
       // NB. the crash we're testing for is the lack of a current `v8::Context`
       // when emitting an event in WebContents's destructor. V8 is inconsistent
@@ -38,4 +59,94 @@ describe('WebContentsView', () => {
       done();
     });
   });
+
+  describe('visibilityState', () => {
+    it('is initially hidden', async () => {
+      const v = new WebContentsView();
+      await v.webContents.loadURL('data:text/html,<script>initialVisibility = document.visibilityState</script>');
+      expect(await v.webContents.executeJavaScript('initialVisibility')).to.equal('hidden');
+    });
+
+    it('becomes visibile when attached', async () => {
+      const v = new WebContentsView();
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('hidden');
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", resolve))');
+      // Ensure that the above listener has been registered before we add the
+      // view to the window, or else the visibilitychange event might be
+      // dispatched before the listener is registered.
+      // executeJavaScript calls are sequential so if this one's finished then
+      // the previous one must also have been finished :)
+      await v.webContents.executeJavaScript('undefined');
+      const w = new BaseWindow();
+      w.setContentView(v);
+      await p;
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+    });
+
+    it('is initially visible if load happens after attach', async () => {
+      const w = new BaseWindow();
+      const v = new WebContentsView();
+      w.contentView = v;
+      await v.webContents.loadURL('data:text/html,<script>initialVisibility = document.visibilityState</script>');
+      expect(await v.webContents.executeJavaScript('initialVisibility')).to.equal('visible');
+    });
+
+    it('becomes hidden when parent window is hidden', async () => {
+      const w = new BaseWindow();
+      const v = new WebContentsView();
+      w.setContentView(v);
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", resolve))');
+      // We have to wait until the listener above is fully registered before hiding the window.
+      // On Windows, the executeJavaScript and the visibilitychange can happen out of order
+      // without this.
+      await v.webContents.executeJavaScript('0');
+      w.hide();
+      await p;
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('hidden');
+    });
+
+    it('becomes visible when parent window is shown', async () => {
+      const w = new BaseWindow({ show: false });
+      const v = new WebContentsView();
+      w.setContentView(v);
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('hidden');
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", resolve))');
+      // We have to wait until the listener above is fully registered before hiding the window.
+      // On Windows, the executeJavaScript and the visibilitychange can happen out of order
+      // without this.
+      await v.webContents.executeJavaScript('0');
+      w.show();
+      await p;
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+    });
+
+    it('does not change when view is moved between two visible windows', async () => {
+      const w = new BaseWindow();
+      const v = new WebContentsView();
+      w.setContentView(v);
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", () => resolve(document.visibilityState)))');
+      // Ensure the listener has been registered.
+      await v.webContents.executeJavaScript('undefined');
+      const w2 = new BaseWindow();
+      w2.setContentView(v);
+      // Wait for the visibility state to settle as "visible".
+      // On macOS one visibilitychange event is fired but visibilityState
+      // remains "visible". On Win/Linux, two visibilitychange events are
+      // fired, a "hidden" and a "visible" one. Reconcile these two models
+      // by waiting until at least one event has been fired, and then waiting
+      // until the visibility state settles as "visible".
+      let visibilityState = await p;
+      for (let attempts = 0; visibilityState !== 'visible' && attempts < 10; attempts++) {
+        visibilityState = await v.webContents.executeJavaScript('new Promise(resolve => document.visibilityState === "visible" ? resolve("visible") : document.addEventListener("visibilitychange", () => resolve(document.visibilityState)))');
+      }
+      expect(visibilityState).to.equal('visible');
+    });
+  });
 });

spec/fixtures/crash-cases/webcontentsview-create-leak-exit/index.js

@@ -1,7 +1,7 @@
 const { WebContentsView, app } = require('electron');
 app.whenReady().then(function () {
   // eslint-disable-next-line no-new
-  new WebContentsView({});
+  new WebContentsView();
 
   app.quit();
 });

spec/fixtures/crash-cases/webview-remove-on-wc-close/index.html

@@ -1,32 +1,6 @@
-<!DOCTYPE html>
-<html lang="en">
-
-<head>
-  <meta charset="UTF-8">
-  <meta name="viewport" content="width=device-width, initial-scale=1.0">
-  <title>Document</title>
-  <style>
-    .webview {
-      border: 1px solid black;
-    }
-  </style>
-</head>
-
-<body>
-  <button class="close-btn">close webview</button>
-  <webview class="webview" src="./webview.html"></webview>
-  <script>
-    const close = document.querySelector('.close-btn')
-    const webview = document.querySelector('.webview')
-
-    webview.addEventListener('close', () => {
-      webview.parentNode.removeChild(webview)
-    })
-
-    close.addEventListener('click', () => {
-      webview.executeJavaScript('window.close()', true)
-    })
-  </script>
-</body>
-
-</html>
+<button id="closeBtn">close webview</button>
+<webview id="webview" src="webview.html"></webview>
+<script>
+  webview.onclose = () => webview.remove()
+  closeBtn.onclick = () => webview.executeJavaScript('window.close()', true)
+</script>

spec/fixtures/crash-cases/webview-remove-on-wc-close/index.js

@@ -19,7 +19,7 @@ app.whenReady().then(() => {
     });
 
     contents.on('did-finish-load', () => {
-      win.webContents.executeJavaScript('document.querySelector(\'.close-btn\').click()');
+      win.webContents.executeJavaScript('closeBtn.click()');
     });
 
     contents.on('will-prevent-unload', event => {

spec/fixtures/crash-cases/webview-remove-on-wc-close/webview.html

@@ -1,19 +1,6 @@
-<!DOCTYPE html>
-<html lang="en">
-
-<head>
-  <meta charset="UTF-8">
-  <meta name="viewport" content="width=device-width, initial-scale=1.0">
-  <title>Document</title>
-</head>
-
-<body>
-  <h1>webview page</h1>
-  <script>
-    window.addEventListener('beforeunload', event => {
-      event.returnValue = 'test'
-    })
-  </script>
-</body>
-
-</html>
+<h1>webview page</h1>
+<script>
+  window.addEventListener('beforeunload', event => {
+    event.returnValue = 'test'
+  })
+</script>

spec/lib/window-helpers.ts

@@ -1,10 +1,10 @@
 import { expect } from 'chai';
-import { BrowserWindow } from 'electron/main';
+import { BaseWindow, BrowserWindow } from 'electron/main';
 import { once } from 'node:events';
 
-async function ensureWindowIsClosed (window: BrowserWindow | null) {
+async function ensureWindowIsClosed (window: BaseWindow | null) {
   if (window && !window.isDestroyed()) {
-    if (window.webContents && !window.webContents.isDestroyed()) {
+    if (window instanceof BrowserWindow && window.webContents && !window.webContents.isDestroyed()) {
       // If a window isn't destroyed already, and it has non-destroyed WebContents,
       // then calling destroy() won't immediately destroy it, as it may have
       // <webview> children which need to be destroyed first. In that case, we
@@ -23,13 +23,13 @@ async function ensureWindowIsClosed (window: BrowserWindow | null) {
 }
 
 export const closeWindow = async (
-  window: BrowserWindow | null = null,
+  window: BaseWindow | null = null,
   { assertNotWindows } = { assertNotWindows: true }
 ) => {
   await ensureWindowIsClosed(window);
 
   if (assertNotWindows) {
-    const windows = BrowserWindow.getAllWindows();
+    const windows = BaseWindow.getAllWindows();
     try {
       expect(windows).to.have.lengthOf(0);
     } finally {
@@ -41,7 +41,7 @@ export const closeWindow = async (
 };
 
 export async function closeAllWindows () {
-  for (const w of BrowserWindow.getAllWindows()) {
+  for (const w of BaseWindow.getAllWindows()) {
     await closeWindow(w, { assertNotWindows: false });
   }
 }

spec/visibility-state-spec.ts

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 import * as cp from 'node:child_process';
-import { BrowserWindow, BrowserWindowConstructorOptions, ipcMain } from 'electron/main';
+import { BaseWindow, BrowserWindow, BrowserWindowConstructorOptions, ipcMain, WebContents, WebContentsView } from 'electron/main';
 import * as path from 'node:path';
 
 import { closeWindow } from './lib/window-helpers';
@@ -11,16 +11,16 @@ import { setTimeout } from 'node:timers/promises';
 // visibilityState specs pass on linux with a real window manager but on CI
 // the environment does not let these specs pass
 ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
-  let w: BrowserWindow;
+  let w: BaseWindow & {webContents: WebContents};
 
   afterEach(() => {
     return closeWindow(w);
   });
 
-  const load = () => w.loadFile(path.resolve(__dirname, 'fixtures', 'chromium', 'visibilitystate.html'));
+  const load = () => w.webContents.loadFile(path.resolve(__dirname, 'fixtures', 'chromium', 'visibilitystate.html'));
 
   const itWithOptions = (name: string, options: BrowserWindowConstructorOptions, fn: Mocha.Func) => {
-    return it(name, async function (...args) {
+    it(name, async function (...args) {
       w = new BrowserWindow({
         ...options,
         paintWhenInitiallyHidden: false,
@@ -32,6 +32,16 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
       });
       await Promise.resolve(fn.apply(this, args));
     });
+
+    it(name + ' with BaseWindow', async function (...args) {
+      const baseWindow = new BaseWindow({
+        ...options
+      });
+      const wcv = new WebContentsView({ webPreferences: { ...(options.webPreferences ?? {}), nodeIntegration: true, contextIsolation: false } });
+      baseWindow.contentView = wcv;
+      w = Object.assign(baseWindow, { webContents: wcv.webContents });
+      await Promise.resolve(fn.apply(this, args));
+    });
   };
 
   itWithOptions('should be visible when the window is initially shown by default', {}, async () => {