From f072da6250621b88eb9bf760bfdd8cc8d981bb89 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Tue, 3 Aug 2021 19:08:49 +0200 Subject: [PATCH] refactor: only create webContents after 'will-attach-webview' (#30311) --- lib/browser/guest-view-manager.ts | 81 +++++-------------- lib/common/ipc-messages.ts | 1 - lib/renderer/web-view/guest-view-internal.ts | 7 -- lib/renderer/web-view/web-view-element.ts | 3 +- lib/renderer/web-view/web-view-impl.ts | 2 +- .../api/electron_api_web_view_manager.cc | 4 - shell/browser/web_contents_preferences.cc | 4 - shell/browser/web_contents_preferences.h | 2 - typings/internal-electron.d.ts | 1 - 9 files changed, 21 insertions(+), 84 deletions(-) diff --git a/lib/browser/guest-view-manager.ts b/lib/browser/guest-view-manager.ts index c973586e807d5..2793a1caf224c 100644 --- a/lib/browser/guest-view-manager.ts +++ b/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; @@ -45,6 +45,7 @@ function makeWebPreferences (embedder: Electron.WebContents, params: Record) { // Create a new guest instance. const createGuest = function (embedder: Electron.WebContents, embedderFrameId: number, elementInstanceId: number, params: Record) { + 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 }); @@ -107,11 +120,8 @@ 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) { @@ -119,7 +129,7 @@ const createGuest = function (embedder: Electron.WebContents, embedderFrameId: n } if (params.src) { - this.loadURL(params.src, params.opts); + this.loadURL(params.src, makeLoadURLOptions(params)); } embedder.emit('did-attach-webview', event, guest); }); @@ -172,78 +182,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) { // 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. diff --git a/lib/common/ipc-messages.ts b/lib/common/ipc-messages.ts index b920c79c48c59..9566916fd2c2b 100644 --- a/lib/common/ipc-messages.ts +++ b/lib/common/ipc-messages.ts @@ -8,7 +8,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', diff --git a/lib/renderer/web-view/guest-view-internal.ts b/lib/renderer/web-view/guest-view-internal.ts index d8b9b61c135f6..bd8a5a774a4d0 100644 --- a/lib/renderer/web-view/guest-view-internal.ts +++ b/lib/renderer/web-view/guest-view-internal.ts @@ -6,7 +6,6 @@ const { mainFrame: webFrame } = process._linkedBinding('electron_renderer_web_fr export interface GuestViewDelegate { dispatchEvent (eventName: string, props: Record): void; - reset(): void; } const DEPRECATED_EVENTS: Record = { @@ -14,11 +13,6 @@ const DEPRECATED_EVENTS: Record = { } 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}`); } diff --git a/lib/renderer/web-view/web-view-element.ts b/lib/renderer/web-view/web-view-element.ts index 84ac5a9ba79c4..ee290ce09b0fc 100644 --- a/lib/renderer/web-view/web-view-element.ts +++ b/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(); diff --git a/lib/renderer/web-view/web-view-impl.ts b/lib/renderer/web-view/web-view-impl.ts index 691b477e1bdb2..f3cad33534539 100644 --- a/lib/renderer/web-view/web-view-impl.ts +++ b/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; } diff --git a/shell/browser/api/electron_api_web_view_manager.cc b/shell/browser/api/electron_api_web_view_manager.cc index 7758c59cd5fd6..2bf03616b711c 100644 --- a/shell/browser/api/electron_api_web_view_manager.cc +++ b/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) { diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index e54772a0bfce2..edaba137ca1cc 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/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_); diff --git a/shell/browser/web_contents_preferences.h b/shell/browser/web_contents_preferences.h index 912540ac0aad0..afe0626c3220c 100644 --- a/shell/browser/web_contents_preferences.h +++ b/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. diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index 666775c16a257..7f6d4258bc112 100644 --- a/typings/internal-electron.d.ts +++ b/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; }