fix: will-attach-webview handler modifying params.instanceId does not break <webview> (#32429)

Co-authored-by: Milan Burda <milan.burda@gmail.com>

lib/browser/guest-view-manager.ts

@@ -74,6 +74,17 @@ function makeWebPreferences (embedder: Electron.WebContents, params: Record<stri
   return webPreferences;
 }
 
+function makeLoadURLOptions (params: Record<string, any>) {
+  const opts: Electron.LoadURLOptions = {};
+  if (params.httpreferrer) {
+    opts.httpReferrer = params.httpreferrer;
+  }
+  if (params.useragent) {
+    opts.userAgent = params.useragent;
+  }
+  return opts;
+}
+
 // Create a new guest instance.
 const createGuest = function (embedder: Electron.WebContents, embedderFrameId: number, elementInstanceId: number, params: Record<string, any>) {
   // eslint-disable-next-line no-undef
@@ -97,7 +108,7 @@ const createGuest = function (embedder: Electron.WebContents, embedderFrameId: n
 
   // Init guest web view after attached.
   guest.once('did-attach' as any, function (this: Electron.WebContents, event: Electron.Event) {
-    params = this.attachParams!;
+    const params = this.attachParams!;
     delete this.attachParams;
 
     const previouslyAttached = this.viewInstanceId != null;
@@ -109,14 +120,7 @@ const createGuest = function (embedder: Electron.WebContents, embedderFrameId: n
     }
 
     if (params.src) {
-      const opts: Electron.LoadURLOptions = {};
-      if (params.httpreferrer) {
-        opts.httpReferrer = params.httpreferrer;
-      }
-      if (params.useragent) {
-        opts.userAgent = params.useragent;
-      }
-      this.loadURL(params.src, opts);
+      this.loadURL(params.src, params.opts);
     }
     embedder.emit('did-attach-webview', event, guest);
   });
@@ -205,13 +209,15 @@ const attachGuest = function (embedder: Electron.WebContents, embedderFrameId: n
     return false;
   }
 
+  const { instanceId } = params;
+
   // If this guest is already attached to an element then remove it
   if (guestInstance.elementInstanceId) {
     const oldKey = `${guestInstance.embedder.id}-${guestInstance.elementInstanceId}`;
     embedderElementsMap.delete(oldKey);
 
     // Remove guest from embedder if moving across web views
-    if (guest.viewInstanceId !== params.instanceId) {
+    if (guest.viewInstanceId !== instanceId) {
       webViewManager.removeGuest(guestInstance.embedder, guestInstanceId);
       guestInstance.embedder._sendInternal(`${IPC_MESSAGES.GUEST_VIEW_INTERNAL_DESTROY_GUEST}-${guest.viewInstanceId}`);
     }
@@ -222,12 +228,12 @@ const attachGuest = function (embedder: Electron.WebContents, embedderFrameId: n
   const event = eventBinding.createWithSender(embedder);
   embedder.emit('will-attach-webview', event, webPreferences, params);
   if (event.defaultPrevented) {
-    if (guest.viewInstanceId == null) guest.viewInstanceId = params.instanceId;
+    if (guest.viewInstanceId == null) guest.viewInstanceId = instanceId;
     guest.destroy();
     return false;
   }
 
-  guest.attachParams = params;
+  guest.attachParams = { instanceId, src: params.src, opts: makeLoadURLOptions(params) };
   embedderElementsMap.set(key, guestInstanceId);
 
   guest.setEmbedder(embedder);

lib/renderer/web-view/web-view-impl.ts

@@ -25,7 +25,6 @@ export class WebViewImpl {
   public hasFocus = false
   public internalInstanceId?: number;
   public resizeObserver?: ResizeObserver;
-  public userAgentOverride?: string;
   public viewInstanceId: number
 
   // on* Event handlers.
@@ -180,8 +179,7 @@ export class WebViewImpl {
 
   buildParams () {
     const params: Record<string, any> = {
-      instanceId: this.viewInstanceId,
-      userAgentOverride: this.userAgentOverride
+      instanceId: this.viewInstanceId
     };
 
     for (const [attributeName, attribute] of this.attributes) {

spec/static/main.js

@@ -138,6 +138,12 @@ ipcMain.on('prevent-next-will-attach-webview', (event) => {
   event.sender.once('will-attach-webview', event => event.preventDefault());
 });
 
+ipcMain.on('break-next-will-attach-webview', (event, id) => {
+  event.sender.once('will-attach-webview', (event, webPreferences, params) => {
+    params.instanceId = null;
+  });
+});
+
 ipcMain.on('disable-node-on-next-will-attach-webview', (event, id) => {
   event.sender.once('will-attach-webview', (event, webPreferences, params) => {
     params.src = `file://${path.join(__dirname, '..', 'fixtures', 'pages', 'c.html')}`;

spec/webview-spec.js

@@ -1164,6 +1164,14 @@ describe('<webview> tag', function () {
       });
     });
 
+    it('handler modifying params.instanceId does not break <webview>', async () => {
+      ipcRenderer.send('break-next-will-attach-webview');
+
+      await startLoadingWebViewAndWaitForMessage(webview, {
+        src: `file://${fixtures}/pages/a.html`
+      });
+    });
+
     it('supports preventing a webview from being created', async () => {
       ipcRenderer.send('prevent-next-will-attach-webview');
 

typings/internal-electron.d.ts

@@ -80,7 +80,7 @@ declare namespace Electron {
     attachToIframe(embedderWebContents: Electron.WebContents, embedderFrameId: number): void;
     detachFromOuterFrame(): void;
     setEmbedder(embedder: Electron.WebContents): void;
-    attachParams?: Record<string, any>;
+    attachParams?: { instanceId: number; src: string, opts: LoadURLOptions };
     viewInstanceId: number;
   }