refactor: only create webContents after 'will-attach-webview' (#35016)

refactor: only create webContents after 'will-attach-webview' (#30311)

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

lib/browser/guest-view-manager.ts

@@ -7,7 +7,7 @@ import { webViewEvents } from '@electron/internal/browser/web-view-events';
 import { IPC_MESSAGES } from '@electron/internal/common/ipc-messages';
 
 interface GuestInstance {
-  elementInstanceId?: number;
+  elementInstanceId: number;
   visibilityState?: VisibilityState;
   embedder: Electron.WebContents;
   guest: Electron.WebContents;
@@ -46,6 +46,7 @@ function makeWebPreferences (embedder: Electron.WebContents, params: Record<stri
     webSecurity: !params.disablewebsecurity,
     enableBlinkFeatures: params.blinkfeatures,
     disableBlinkFeatures: params.disableblinkfeatures,
+    partition: params.partition,
     ...parsedWebPreferences
   };
 
@@ -87,14 +88,26 @@ function makeLoadURLOptions (params: Record<string, any>) {
 
 // Create a new guest instance.
 const createGuest = function (embedder: Electron.WebContents, embedderFrameId: number, elementInstanceId: number, params: Record<string, any>) {
+  const webPreferences = makeWebPreferences(embedder, params);
+  const event = eventBinding.createWithSender(embedder);
+
+  const { instanceId } = params;
+
+  embedder.emit('will-attach-webview', event, webPreferences, params);
+  if (event.defaultPrevented) {
+    return -1;
+  }
+
   // eslint-disable-next-line no-undef
   const guest = (webContents as typeof ElectronInternal.WebContents).create({
+    ...webPreferences,
     type: 'webview',
-    partition: params.partition,
     embedder
   });
+
   const guestInstanceId = guest.id;
   guestInstances.set(guestInstanceId, {
+    elementInstanceId,
     guest,
     embedder
   });
@@ -108,19 +121,16 @@ 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) {
-    const params = this.attachParams!;
-    delete this.attachParams;
-
     const previouslyAttached = this.viewInstanceId != null;
-    this.viewInstanceId = params.instanceId;
+    this.viewInstanceId = instanceId;
 
     // Only load URL and set size on first attach
     if (previouslyAttached) {
       return;
     }
 
     if (params.src) {
-      this.loadURL(params.src, params.opts);
+      this.loadURL(params.src, makeLoadURLOptions(params));
     }
     embedder.emit('did-attach-webview', event, guest);
   });
@@ -173,78 +183,25 @@ const createGuest = function (embedder: Electron.WebContents, embedderFrameId: n
     }
   });
 
-  if (attachGuest(embedder, embedderFrameId, elementInstanceId, guestInstanceId, params)) {
-    return guestInstanceId;
-  }
-
-  return -1;
-};
-
-// Attach the guest to an element of embedder.
-const attachGuest = function (embedder: Electron.WebContents, embedderFrameId: number, elementInstanceId: number, guestInstanceId: number, params: Record<string, any>) {
   // Destroy the old guest when attaching.
   const key = `${embedder.id}-${elementInstanceId}`;
   const oldGuestInstanceId = embedderElementsMap.get(key);
   if (oldGuestInstanceId != null) {
-    // Reattachment to the same guest is just a no-op.
-    if (oldGuestInstanceId === guestInstanceId) {
-      return false;
-    }
-
     const oldGuestInstance = guestInstances.get(oldGuestInstanceId);
     if (oldGuestInstance) {
       oldGuestInstance.guest.detachFromOuterFrame();
     }
   }
 
-  const guestInstance = guestInstances.get(guestInstanceId);
-  // If this isn't a valid guest instance then do nothing.
-  if (!guestInstance) {
-    console.error(new Error(`Guest attach failed: Invalid guestInstanceId ${guestInstanceId}`));
-    return false;
-  }
-  const { guest } = guestInstance;
-  if (guest.hostWebContents !== embedder) {
-    console.error(new Error(`Guest attach failed: Access denied to guestInstanceId ${guestInstanceId}`));
-    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 !== instanceId) {
-      webViewManager.removeGuest(guestInstance.embedder, guestInstanceId);
-      guestInstance.embedder._sendInternal(`${IPC_MESSAGES.GUEST_VIEW_INTERNAL_DESTROY_GUEST}-${guest.viewInstanceId}`);
-    }
-  }
-
-  const webPreferences = makeWebPreferences(embedder, params);
-
-  const event = eventBinding.createWithSender(embedder);
-  embedder.emit('will-attach-webview', event, webPreferences, params);
-  if (event.defaultPrevented) {
-    if (guest.viewInstanceId == null) guest.viewInstanceId = instanceId;
-    guest.destroy();
-    return false;
-  }
-
-  guest.attachParams = { instanceId, src: params.src, opts: makeLoadURLOptions(params) };
   embedderElementsMap.set(key, guestInstanceId);
-
   guest.setEmbedder(embedder);
-  guestInstance.embedder = embedder;
-  guestInstance.elementInstanceId = elementInstanceId;
 
   watchEmbedder(embedder);
 
   webViewManager.addGuest(guestInstanceId, embedder, guest, webPreferences);
   guest.attachToIframe(embedder, embedderFrameId);
-  return true;
+
+  return guestInstanceId;
 };
 
 // Remove an guest-embedder relationship.

lib/common/ipc-messages.ts

@@ -9,7 +9,6 @@ export const enum IPC_MESSAGES {
 
   GUEST_INSTANCE_VISIBILITY_CHANGE = 'GUEST_INSTANCE_VISIBILITY_CHANGE',
 
-  GUEST_VIEW_INTERNAL_DESTROY_GUEST = 'GUEST_VIEW_INTERNAL_DESTROY_GUEST',
   GUEST_VIEW_INTERNAL_DISPATCH_EVENT = 'GUEST_VIEW_INTERNAL_DISPATCH_EVENT',
 
   GUEST_VIEW_MANAGER_CREATE_AND_ATTACH_GUEST = 'GUEST_VIEW_MANAGER_CREATE_AND_ATTACH_GUEST',

lib/renderer/web-view/guest-view-internal.ts

@@ -6,19 +6,13 @@ const { mainFrame: webFrame } = process._linkedBinding('electron_renderer_web_fr
 
 export interface GuestViewDelegate {
   dispatchEvent (eventName: string, props: Record<string, any>): void;
-  reset(): void;
 }
 
 const DEPRECATED_EVENTS: Record<string, string> = {
   'page-title-updated': 'page-title-set'
 } as const;
 
 export function registerEvents (viewInstanceId: number, delegate: GuestViewDelegate) {
-  ipcRendererInternal.on(`${IPC_MESSAGES.GUEST_VIEW_INTERNAL_DESTROY_GUEST}-${viewInstanceId}`, function () {
-    delegate.reset();
-    delegate.dispatchEvent('destroyed', {});
-  });
-
   ipcRendererInternal.on(`${IPC_MESSAGES.GUEST_VIEW_INTERNAL_DISPATCH_EVENT}-${viewInstanceId}`, function (event, eventName, props) {
     if (DEPRECATED_EVENTS[eventName] != null) {
       delegate.dispatchEvent(DEPRECATED_EVENTS[eventName], props);
@@ -29,7 +23,6 @@ export function registerEvents (viewInstanceId: number, delegate: GuestViewDeleg
 }
 
 export function deregisterEvents (viewInstanceId: number) {
-  ipcRendererInternal.removeAllListeners(`${IPC_MESSAGES.GUEST_VIEW_INTERNAL_DESTROY_GUEST}-${viewInstanceId}`);
   ipcRendererInternal.removeAllListeners(`${IPC_MESSAGES.GUEST_VIEW_INTERNAL_DISPATCH_EVENT}-${viewInstanceId}`);
 }
 

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

@@ -55,8 +55,7 @@ const defineWebViewElement = (hooks: WebViewImplHooks) => {
       }
       if (!internal.elementAttached) {
         hooks.guestViewInternal.registerEvents(internal.viewInstanceId, {
-          dispatchEvent: internal.dispatchEvent.bind(internal),
-          reset: internal.reset.bind(internal)
+          dispatchEvent: internal.dispatchEvent.bind(internal)
         });
         internal.elementAttached = true;
         (internal.attributes.get(WEB_VIEW_CONSTANTS.ATTRIBUTE_SRC) as SrcAttribute).parse();

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

@@ -191,7 +191,7 @@ export class WebViewImpl {
 
   attachGuestInstance (guestInstanceId: number) {
     if (guestInstanceId === -1) {
-      // Do nothing
+      this.dispatchEvent('destroyed');
       return;
     }
 

shell/browser/api/electron_api_web_view_manager.cc

@@ -28,10 +28,6 @@ void AddGuest(int guest_instance_id,
     electron::WebContentsZoomController::FromWebContents(guest_web_contents)
         ->SetDefaultZoomFactor(zoom_factor);
   }
-
-  WebContentsPreferences::From(guest_web_contents)->Merge(options);
-  // Trigger re-calculation of webkit prefs.
-  guest_web_contents->NotifyPreferencesChanged();
 }
 
 void RemoveGuest(content::WebContents* embedder, int guest_instance_id) {

shell/browser/web_contents_preferences.cc

@@ -176,11 +176,7 @@ void WebContentsPreferences::Clear() {
 void WebContentsPreferences::SetFromDictionary(
     const gin_helper::Dictionary& web_preferences) {
   Clear();
-  Merge(web_preferences);
-}
 
-void WebContentsPreferences::Merge(
-    const gin_helper::Dictionary& web_preferences) {
   web_preferences.Get(options::kPlugins, &plugins_);
   web_preferences.Get(options::kExperimentalFeatures, &experimental_features_);
   web_preferences.Get(options::kNodeIntegration, &node_integration_);

shell/browser/web_contents_preferences.h

@@ -40,8 +40,6 @@ class WebContentsPreferences
   WebContentsPreferences(const WebContentsPreferences&) = delete;
   WebContentsPreferences& operator=(const WebContentsPreferences&) = delete;
 
-  void Merge(const gin_helper::Dictionary& new_web_preferences);
-
   void SetFromDictionary(const gin_helper::Dictionary& new_web_preferences);
 
   // Append command paramters according to preferences.

typings/internal-electron.d.ts

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