Browse Source

refactor: use WeakMap instead of hidden V8 properties to store WebViewImpl (#29049)

Milan Burda 4 years ago
parent
commit
49ef1fe342

+ 1 - 1
lib/isolated_renderer/init.ts

@@ -11,5 +11,5 @@ const webViewImpl = v8Util.getHiddenValue(isolatedWorld, 'web-view-impl');
 if (webViewImpl) {
   // Must setup the WebView element in main world.
   const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element') as typeof webViewElementModule;
-  setupWebView(v8Util, webViewImpl as any);
+  setupWebView(webViewImpl as any);
 }

+ 2 - 1
lib/renderer/web-view/web-view-constants.ts

@@ -18,5 +18,6 @@ export const enum WEB_VIEW_CONSTANTS {
   // Error messages.
   ERROR_MSG_ALREADY_NAVIGATED = 'The object has already navigated, so its partition cannot be changed.',
   ERROR_MSG_INVALID_PARTITION_ATTRIBUTE = 'Invalid partition attribute.',
-  ERROR_MSG_INVALID_PRELOAD_ATTRIBUTE = 'Only "file:" protocol is supported in "preload" attribute.'
+  ERROR_MSG_INVALID_PRELOAD_ATTRIBUTE = 'Only "file:" protocol is supported in "preload" attribute.',
+  ERROR_MSG_NOT_ATTACHED = 'The WebView must be attached to the DOM and the dom-ready event emitted before this method can be called.'
 }

+ 18 - 10
lib/renderer/web-view/web-view-element.ts

@@ -12,10 +12,10 @@ import { WEB_VIEW_CONSTANTS } from '@electron/internal/renderer/web-view/web-vie
 import type * as webViewImplModule from '@electron/internal/renderer/web-view/web-view-impl';
 import type { SrcAttribute } from '@electron/internal/renderer/web-view/web-view-attributes';
 
-type IWebViewImpl = webViewImplModule.WebViewImpl;
+const internals = new WeakMap<HTMLElement, webViewImplModule.WebViewImpl>();
 
 // Return a WebViewElement class that is defined in this context.
-const defineWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof webViewImplModule) => {
+const defineWebViewElement = (webViewImpl: typeof webViewImplModule) => {
   const { guestViewInternal, WebViewImpl } = webViewImpl;
   return class WebViewElement extends HTMLElement {
     static get observedAttributes () {
@@ -44,11 +44,19 @@ const defineWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof
         Object.assign(event, props);
         return internal.webviewNode.dispatchEvent(event);
       };
-      v8Util.setHiddenValue(this, 'internal', internal);
+      internals.set(this, internal);
+    }
+
+    getWebContentsId () {
+      const internal = internals.get(this);
+      if (!internal || !internal.guestInstanceId) {
+        throw new Error(WEB_VIEW_CONSTANTS.ERROR_MSG_NOT_ATTACHED);
+      }
+      return internal.guestInstanceId;
     }
 
     connectedCallback () {
-      const internal = v8Util.getHiddenValue<IWebViewImpl>(this, 'internal');
+      const internal = internals.get(this);
       if (!internal) {
         return;
       }
@@ -60,14 +68,14 @@ const defineWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof
     }
 
     attributeChangedCallback (name: string, oldValue: any, newValue: any) {
-      const internal = v8Util.getHiddenValue<IWebViewImpl>(this, 'internal');
+      const internal = internals.get(this);
       if (internal) {
         internal.handleWebviewAttributeMutation(name, oldValue, newValue);
       }
     }
 
     disconnectedCallback () {
-      const internal = v8Util.getHiddenValue<IWebViewImpl>(this, 'internal');
+      const internal = internals.get(this);
       if (!internal) {
         return;
       }
@@ -82,10 +90,10 @@ const defineWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof
 };
 
 // Register <webview> custom element.
-const registerWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof webViewImplModule) => {
+const registerWebViewElement = (webViewImpl: typeof webViewImplModule) => {
   // I wish eslint wasn't so stupid, but it is
   // eslint-disable-next-line
-  const WebViewElement = defineWebViewElement(v8Util, webViewImpl) as unknown as typeof ElectronInternal.WebViewElement
+  const WebViewElement = defineWebViewElement(webViewImpl) as unknown as typeof ElectronInternal.WebViewElement
 
   webViewImpl.setupMethods(WebViewElement);
 
@@ -108,14 +116,14 @@ const registerWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeo
 };
 
 // Prepare to register the <webview> element.
-export const setupWebView = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof webViewImplModule) => {
+export const setupWebView = (webViewImpl: typeof webViewImplModule) => {
   const useCapture = true;
   const listener = (event: Event) => {
     if (document.readyState === 'loading') {
       return;
     }
 
-    registerWebViewElement(v8Util, webViewImpl);
+    registerWebViewElement(webViewImpl);
 
     window.removeEventListener(event.type, listener, useCapture);
   };

+ 1 - 8
lib/renderer/web-view/web-view-impl.ts

@@ -64,6 +64,7 @@ export class WebViewImpl {
     iframeElement.style.flex = '1 1 auto';
     iframeElement.style.width = '100%';
     iframeElement.style.border = '0';
+    // used by RendererClientBase::IsWebViewFrame
     v8Util.setHiddenValue(iframeElement, 'internal', this);
     return iframeElement;
   }
@@ -210,14 +211,6 @@ export class WebViewImpl {
 // I wish eslint wasn't so stupid, but it is
 // eslint-disable-next-line
 export const setupMethods = (WebViewElement: typeof ElectronInternal.WebViewElement) => {
-  WebViewElement.prototype.getWebContentsId = function () {
-    const internal = v8Util.getHiddenValue<WebViewImpl>(this, 'internal');
-    if (!internal.guestInstanceId) {
-      throw new Error('The WebView must be attached to the DOM and the dom-ready event emitted before this method can be called.');
-    }
-    return internal.guestInstanceId;
-  };
-
   // Focusing the webview should move page focus to the underlying iframe.
   WebViewElement.prototype.focus = function () {
     this.contentWindow.focus();

+ 1 - 1
lib/renderer/web-view/web-view-init.ts

@@ -27,7 +27,7 @@ export function webViewInit (contextIsolation: boolean, webviewTag: boolean, gue
       v8Util.setHiddenValue(window, 'web-view-impl', webViewImplModule);
     } else {
       const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element') as typeof webViewElement;
-      setupWebView(v8Util, webViewImplModule);
+      setupWebView(webViewImplModule);
     }
   }