From 015fe573556f1503b5fe9d42cf13f48f8803609a Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Fri, 24 Jun 2022 12:17:13 +0200 Subject: [PATCH] refactor(tracker): get rid of instanceof checks in observer (use nodeName and nodeType guards) --- tracker/tracker/src/main/app/context.ts | 99 ------------------- tracker/tracker/src/main/app/guards.ts | 5 +- .../tracker/src/main/app/observer/observer.ts | 62 ++++++------ .../src/main/app/observer/top_observer.ts | 11 ++- 4 files changed, 41 insertions(+), 136 deletions(-) delete mode 100644 tracker/tracker/src/main/app/context.ts diff --git a/tracker/tracker/src/main/app/context.ts b/tracker/tracker/src/main/app/context.ts deleted file mode 100644 index 87e4df20a..000000000 --- a/tracker/tracker/src/main/app/context.ts +++ /dev/null @@ -1,99 +0,0 @@ -// TODO: global type -export interface Window extends globalThis.Window { - HTMLInputElement: typeof HTMLInputElement, - HTMLLinkElement: typeof HTMLLinkElement, - HTMLStyleElement: typeof HTMLStyleElement, - SVGStyleElement: typeof SVGStyleElement, - HTMLIFrameElement: typeof HTMLIFrameElement, - Text: typeof Text, - Element: typeof Element, - ShadowRoot: typeof ShadowRoot, - //parent: Window, -} - -type WindowConstructor = - Document | - Element | - Text | - ShadowRoot | - HTMLInputElement | - HTMLLinkElement | - HTMLStyleElement | - HTMLIFrameElement - -// type ConstructorNames = -// 'Element' | -// 'Text' | -// 'HTMLInputElement' | -// 'HTMLLinkElement' | -// 'HTMLStyleElement' | -// 'HTMLIFrameElement' -type Constructor = { new (...args: any[]): T , name: string }; - - // TODO: we need a type expert here so we won't have to ignore the lines - // TODO: use it everywhere (static function; export from which file? <-- global Window typing required) - // TODO: most efficient and common way - // Problem: on YouTube there is context[constr.name] undefined for constr=ShadowDom due to some minimisations -export function isInstance(node: Node, constr: Constructor): node is T { - const doc = node.ownerDocument; - if (!doc) { // null if Document - return constr.name === 'Document'; - } - let context: Window = - // @ts-ignore (for EI, Safary) - doc.parentWindow || - doc.defaultView; // TODO: smart global typing for Window object - while(context !== window) { - // @ts-ignore - if (context[constr.name] && node instanceof context[constr.name]) { - return true - } - // @ts-ignore - context = context.parent || window - } - // @ts-ignore - return context[constr.name] ? node instanceof context[constr.name] : node instanceof constr -} - -// TODO: ensure 1. it works in every cases (iframes/detached nodes) and 2. the most efficient -export function inDocument(node: Node) { - const doc = node.ownerDocument - if (!doc) { return true } // Document - let current: Node | null = node - while(current) { - if (current === doc) { - return true - } else if(isInstance(current, ShadowRoot)) { - current = current.host - } else { - current = current.parentNode - } - } - return false -} - -// export function inDocument(node: Node): boolean { -// // @ts-ignore compatability -// if (node.getRootNode) { -// let root: Node -// while ((root = node.getRootNode()) !== node) { -// //// -// } -// } - -// const doc = node.ownerDocument -// if (!doc) { return false } -// if (doc.contains(node)) { return true } -// let context: Window = -// // @ts-ignore (for EI, Safary) -// doc.parentWindow || -// doc.defaultView; -// while(context.parent && context.parent !== context) { -// if (context.document.contains(node)) { -// return true -// } -// // @ts-ignore -// context = context.parent -// } -// return false; -// } diff --git a/tracker/tracker/src/main/app/guards.ts b/tracker/tracker/src/main/app/guards.ts index 270ff52dd..c3f36d398 100644 --- a/tracker/tracker/src/main/app/guards.ts +++ b/tracker/tracker/src/main/app/guards.ts @@ -24,9 +24,10 @@ type TagTypeMap = { SELECT: HTMLSelectElement LABEL: HTMLLabelElement IFRAME: HTMLIFrameElement - STYLE: HTMLStyleElement | SVGStyleElement + STYLE: HTMLStyleElement + style: SVGStyleElement LINK: HTMLLinkElement } export function hasTag(el: Node, tagName: T): el is TagTypeMap[typeof tagName] { - return el.nodeName.toUpperCase() === tagName + return el.nodeName === tagName } diff --git a/tracker/tracker/src/main/app/observer/observer.ts b/tracker/tracker/src/main/app/observer/observer.ts index ba6e46895..2283fac56 100644 --- a/tracker/tracker/src/main/app/observer/observer.ts +++ b/tracker/tracker/src/main/app/observer/observer.ts @@ -10,20 +10,17 @@ import { RemoveNode, } from "../../../common/messages.js"; import App from "../index.js"; -import { isInstance, inDocument } from "../context.js"; - - -function isSVGElement(node: Element): node is SVGElement { - return node.namespaceURI === 'http://www.w3.org/2000/svg'; -} +import { + isRootNode, + isTextNode, + isElementNode, + isSVGElement, + hasTag, +} from "../guards.js"; function isIgnored(node: Node): boolean { - if (isInstance(node, Text)) { - return false; - } - if (!isInstance(node, Element)) { - return true; - } + if (isTextNode(node)) { return false } + if (!isElementNode(node)) { return true } const tag = node.tagName.toUpperCase(); if (tag === 'LINK') { const rel = node.getAttribute('rel'); @@ -39,16 +36,17 @@ function isIgnored(node: Node): boolean { ); } -function isRootNode(node: Node): boolean { - return isInstance(node, Document) || isInstance(node, ShadowRoot); +function isObservable(node: Node): boolean { + if (isRootNode(node)) { return true } + return !isIgnored(node) } -function isObservable(node: Node): boolean { - if (isRootNode(node)) { - return true; - } - return !isIgnored(node); -} + +/* + TODO: + - fix unbinding logic + send all removals first (ensure sequence is correct) + - use document as a 0-node in the upper context +*/ export default abstract class Observer { private readonly observer: MutationObserver; @@ -61,11 +59,11 @@ export default abstract class Observer { constructor(protected readonly app: App, protected readonly isTopContext = false) { this.observer = new MutationObserver( this.app.safe((mutations) => { - for (const mutation of mutations) { + for (const mutation of mutations) { // mutations order is sequential const target = mutation.target; const type = mutation.type; - if (!isObservable(target) || !inDocument(target)) { + if (!isObservable(target)) { continue; } if (type === 'childList') { @@ -147,7 +145,7 @@ export default abstract class Observer { } if ( name === 'value' && - isInstance(node, HTMLInputElement) && + hasTag(node, "INPUT") && node.type !== 'button' && node.type !== 'reset' && node.type !== 'submit' @@ -158,7 +156,7 @@ export default abstract class Observer { this.app.send(new RemoveNodeAttribute(id, name)); return; } - if (name === 'style' || name === 'href' && isInstance(node, HTMLLinkElement)) { + if (name === 'style' || name === 'href' && hasTag(node, "LINK")) { this.app.send(new SetNodeAttributeURLBased(id, name, value, this.app.getBaseHref())); return; } @@ -169,7 +167,7 @@ export default abstract class Observer { } private sendNodeData(id: number, parentElement: Element, data: string): void { - if (isInstance(parentElement, HTMLStyleElement) || isInstance(parentElement, SVGStyleElement)) { + if (hasTag(parentElement, "STYLE") || hasTag(parentElement, "style")) { this.app.send(new SetCSSDataURLBased(id, data, this.app.getBaseHref())); return; } @@ -214,6 +212,7 @@ export default abstract class Observer { } } + // A top-consumption function on the infinite lists test. (~1% of performance resources) private _commitNode(id: number, node: Node): boolean { if (isRootNode(node)) { return true; @@ -224,7 +223,7 @@ export default abstract class Observer { // Disable parent check for the upper context HTMLHtmlElement, because it is root there... (before) // TODO: get rid of "special" cases (there is an issue with CreateDocument altered behaviour though) // TODO: Clean the logic (though now it workd fine) - if (!isInstance(node, HTMLHtmlElement) || !this.isTopContext) { + if (!hasTag(node, "HTML") || !this.isTopContext) { if (parent === null) { this.unbindNode(node); return false; @@ -243,6 +242,7 @@ export default abstract class Observer { return false; } } + // From here parentID === undefined if node is top context HTML node let sibling = node.previousSibling; while (sibling !== null) { const siblingID = this.app.nodes.getID(sibling); @@ -254,7 +254,7 @@ export default abstract class Observer { sibling = sibling.previousSibling; } if (sibling === null) { - this.indexes[id] = 0; // + this.indexes[id] = 0; } const isNew = this.recents[id]; const index = this.indexes[id]; @@ -262,7 +262,7 @@ export default abstract class Observer { throw 'commitNode: missing node index'; } if (isNew === true) { - if (isInstance(node, Element)) { + if (isElementNode(node)) { let el: Element = node if (parentID !== undefined) { if (this.app.sanitizer.isMaskedContainer(id)) { @@ -287,7 +287,7 @@ export default abstract class Observer { const attr = el.attributes[i]; this.sendNodeAttribute(id, el, attr.nodeName, attr.value); } - } else if (isInstance(node, Text)) { + } else if (isTextNode(node)) { // for text node id != 0, hence parentID !== undefined and parent is Element this.app.send(new CreateTextNode(id, parentID as number, index)); this.sendNodeData(id, parent as Element, node.data); @@ -299,7 +299,7 @@ export default abstract class Observer { } const attr = this.attributesList[id]; if (attr !== undefined) { - if (!isInstance(node, Element)) { + if (!isElementNode(node)) { throw 'commitNode: node is not an element'; } for (const name of attr) { @@ -307,7 +307,7 @@ export default abstract class Observer { } } if (this.textSet.has(id)) { - if (!isInstance(node, Text)) { + if (!isTextNode(node)) { throw 'commitNode: node is not a text'; } // for text node id != 0, hence parent is Element diff --git a/tracker/tracker/src/main/app/observer/top_observer.ts b/tracker/tracker/src/main/app/observer/top_observer.ts index cab84b162..469ad2388 100644 --- a/tracker/tracker/src/main/app/observer/top_observer.ts +++ b/tracker/tracker/src/main/app/observer/top_observer.ts @@ -1,6 +1,9 @@ import Observer from "./observer.js"; -import { isInstance } from "../context.js"; -import type { Window } from "../context.js"; +import { + isElementNode, + hasTag, +} from "../guards.js"; + import IFrameObserver from "./iframe_observer.js"; import ShadowRootObserver from "./shadow_root_observer.js"; @@ -24,7 +27,7 @@ export default class TopObserver extends Observer { // IFrames this.app.nodes.attachNodeCallback(node => { - if (isInstance(node, HTMLIFrameElement) && + if (hasTag(node, "IFRAME") && ((this.options.captureIFrames && !hasOpenreplayAttribute(node, "obscured")) || hasOpenreplayAttribute(node, "capture")) ) { @@ -34,7 +37,7 @@ export default class TopObserver extends Observer { // ShadowDOM this.app.nodes.attachNodeCallback(node => { - if (isInstance(node, Element) && node.shadowRoot !== null) { + if (isElementNode(node) && node.shadowRoot !== null) { this.handleShadowRoot(node.shadowRoot) } })