From bbf14730222408292a2e00dd15c7b4ccc74cba8d Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Thu, 23 May 2024 13:30:37 -0500 Subject: [PATCH] [PM-5295] Improve autofill collection of page details performance (#9063) * [PM-5295] Improve autofill collection of page details performance * [PM-5295] Reworking implementation to leverage requestIdleCallback instead of requestAnimationFrame * [PM-5295] Reworking implementation to leverage requestIdleCallback instead of requestAnimationFrame * [PM-5295] Incorporating documentation for added methods * [PM-5295] Reworking how we handle collection of shadowRoot elements * [PM-5295] Fixing jest tests relating to the defined pseudo selector * [PM-5295] Fixing jest tests relating to the defined pseudo selector * [PM-5295] Refactoring * [PM-5295] Refactoring * [PM-5295] Refactoring * [PM-5295] Starting the work to set up the tree walker strategy under a feature flag * [PM-5295] Incorporating methodology for triggering a fallback to the TreeWalker API if issues arise with the deepQuery approach * [PM-5295] Fixing jest test --- .../autofill/content/autofill-init.spec.ts | 18 +- .../collect-autofill-content.service.ts | 8 +- .../collect-autofill-content.service.spec.ts | 142 +++--- .../collect-autofill-content.service.ts | 479 +++++++++++++----- .../insert-autofill-content.service.spec.ts | 7 + .../insert-autofill-content.service.ts | 9 +- .../src/autofill/spec/testing-utils.ts | 29 ++ apps/browser/src/autofill/utils/index.ts | 6 +- .../src/background/runtime.background.ts | 11 +- libs/common/src/enums/feature-flag.enum.ts | 2 + 10 files changed, 481 insertions(+), 230 deletions(-) diff --git a/apps/browser/src/autofill/content/autofill-init.spec.ts b/apps/browser/src/autofill/content/autofill-init.spec.ts index 8f1a8bf992..13515f6de2 100644 --- a/apps/browser/src/autofill/content/autofill-init.spec.ts +++ b/apps/browser/src/autofill/content/autofill-init.spec.ts @@ -6,7 +6,11 @@ import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants" import AutofillPageDetails from "../models/autofill-page-details"; import AutofillScript from "../models/autofill-script"; import AutofillOverlayContentService from "../services/autofill-overlay-content.service"; -import { flushPromises, sendExtensionRuntimeMessage } from "../spec/testing-utils"; +import { + flushPromises, + mockQuerySelectorAllDefinedCall, + sendExtensionRuntimeMessage, +} from "../spec/testing-utils"; import { RedirectFocusDirection } from "../utils/autofill-overlay.enum"; import { AutofillExtensionMessage } from "./abstractions/autofill-init"; @@ -16,6 +20,7 @@ describe("AutofillInit", () => { let autofillInit: AutofillInit; const autofillOverlayContentService = mock(); const originalDocumentReadyState = document.readyState; + const mockQuerySelectorAll = mockQuerySelectorAllDefinedCall(); beforeEach(() => { chrome.runtime.connect = jest.fn().mockReturnValue({ @@ -36,6 +41,10 @@ describe("AutofillInit", () => { }); }); + afterAll(() => { + mockQuerySelectorAll.mockRestore(); + }); + describe("init", () => { it("sets up the extension message listeners", () => { jest.spyOn(autofillInit as any, "setupExtensionMessageListeners"); @@ -200,7 +209,12 @@ describe("AutofillInit", () => { expect(autofillInit["collectAutofillContentService"].getPageDetails).toHaveBeenCalled(); expect(sendResponse).toBeCalledWith(pageDetails); - expect(chrome.runtime.sendMessage).not.toHaveBeenCalled(); + expect(chrome.runtime.sendMessage).not.toHaveBeenCalledWith({ + command: "collectPageDetailsResponse", + tab: message.tab, + details: pageDetails, + sender: message.sender, + }); }); }); diff --git a/apps/browser/src/autofill/services/abstractions/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/abstractions/collect-autofill-content.service.ts index 46ad615059..d17d7842dc 100644 --- a/apps/browser/src/autofill/services/abstractions/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/abstractions/collect-autofill-content.service.ts @@ -17,11 +17,11 @@ type UpdateAutofillDataAttributeParams = { interface CollectAutofillContentService { getPageDetails(): Promise; getAutofillFieldElementByOpid(opid: string): HTMLElement | null; - queryAllTreeWalkerNodes( - rootNode: Node, - filterCallback: CallableFunction, + deepQueryElements( + root: Document | ShadowRoot | Element, + selector: string, isObservingShadowRoot?: boolean, - ): Node[]; + ): T[]; destroy(): void; } diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts index 22a856c25d..a44fcd0b79 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts @@ -3,6 +3,7 @@ import { mock } from "jest-mock-extended"; import AutofillField from "../models/autofill-field"; import AutofillForm from "../models/autofill-form"; import { createAutofillFieldMock, createAutofillFormMock } from "../spec/autofill-mocks"; +import { mockQuerySelectorAllDefinedCall } from "../spec/testing-utils"; import { ElementWithOpId, FillableFormFieldElement, @@ -23,13 +24,17 @@ const mockLoginForm = ` `; +const waitForIdleCallback = () => new Promise((resolve) => globalThis.requestIdleCallback(resolve)); + describe("CollectAutofillContentService", () => { const domElementVisibilityService = new DomElementVisibilityService(); const autofillOverlayContentService = new AutofillOverlayContentService(); let collectAutofillContentService: CollectAutofillContentService; const mockIntersectionObserver = mock(); + const mockQuerySelectorAll = mockQuerySelectorAllDefinedCall(); beforeEach(() => { + globalThis.requestIdleCallback = jest.fn((cb, options) => setTimeout(cb, 100)); document.body.innerHTML = mockLoginForm; collectAutofillContentService = new CollectAutofillContentService( domElementVisibilityService, @@ -40,9 +45,14 @@ describe("CollectAutofillContentService", () => { afterEach(() => { jest.clearAllMocks(); + jest.restoreAllMocks(); document.body.innerHTML = ""; }); + afterAll(() => { + mockQuerySelectorAll.mockRestore(); + }); + describe("getPageDetails", () => { beforeEach(() => { jest @@ -437,6 +447,51 @@ describe("CollectAutofillContentService", () => { }); }); + describe("deepQueryElements", () => { + beforeEach(() => { + collectAutofillContentService["mutationObserver"] = mock(); + }); + + it("queries form field elements that are nested within a ShadowDOM", () => { + const root = document.createElement("div"); + const shadowRoot = root.attachShadow({ mode: "open" }); + const form = document.createElement("form"); + const input = document.createElement("input"); + input.type = "text"; + form.appendChild(input); + shadowRoot.appendChild(form); + + const formFieldElements = collectAutofillContentService.deepQueryElements( + shadowRoot, + "input", + true, + ); + + expect(formFieldElements).toStrictEqual([input]); + }); + + it("queries form field elements that are nested within multiple ShadowDOM elements", () => { + const root = document.createElement("div"); + const shadowRoot1 = root.attachShadow({ mode: "open" }); + const root2 = document.createElement("div"); + const shadowRoot2 = root2.attachShadow({ mode: "open" }); + const form = document.createElement("form"); + const input = document.createElement("input"); + input.type = "text"; + form.appendChild(input); + shadowRoot2.appendChild(form); + shadowRoot1.appendChild(root2); + + const formFieldElements = collectAutofillContentService.deepQueryElements( + shadowRoot1, + "input", + true, + ); + + expect(formFieldElements).toStrictEqual([input]); + }); + }); + describe("buildAutofillFormsData", () => { it("will not attempt to gather data from a cached form element", () => { const documentTitle = "Test Page"; @@ -1993,17 +2048,6 @@ describe("CollectAutofillContentService", () => { expect(shadowRoot).toEqual(null); }); - it("returns null if the passed node contains children elements", () => { - const element = document.createElement("div"); - element.innerHTML = "

Hello, world!

"; - const shadowRoot = collectAutofillContentService["getShadowRoot"](element); - - // eslint-disable-next-line - // @ts-ignore - expect(chrome.dom.openOrClosedShadowRoot).not.toBeCalled(); - expect(shadowRoot).toEqual(null); - }); - it("returns an open shadow root if the passed node has a shadowDOM element", () => { const element = document.createElement("div"); element.attachShadow({ mode: "open" }); @@ -2023,50 +2067,6 @@ describe("CollectAutofillContentService", () => { }); }); - describe("buildTreeWalkerNodesQueryResults", () => { - it("will recursively call itself if a shadowDOM element is found and will observe the element for mutations", () => { - collectAutofillContentService["mutationObserver"] = mock({ - observe: jest.fn(), - }); - jest.spyOn(collectAutofillContentService as any, "buildTreeWalkerNodesQueryResults"); - const shadowRoot = document.createElement("div"); - jest - .spyOn(collectAutofillContentService as any, "getShadowRoot") - .mockReturnValueOnce(shadowRoot); - const callbackFilter = jest.fn(); - - collectAutofillContentService["buildTreeWalkerNodesQueryResults"]( - document.body, - [], - callbackFilter, - true, - ); - - expect(collectAutofillContentService["buildTreeWalkerNodesQueryResults"]).toBeCalledTimes(2); - expect(collectAutofillContentService["mutationObserver"].observe).toBeCalled(); - }); - - it("will not observe the shadowDOM element if required to skip", () => { - collectAutofillContentService["mutationObserver"] = mock({ - observe: jest.fn(), - }); - const shadowRoot = document.createElement("div"); - jest - .spyOn(collectAutofillContentService as any, "getShadowRoot") - .mockReturnValueOnce(shadowRoot); - const callbackFilter = jest.fn(); - - collectAutofillContentService["buildTreeWalkerNodesQueryResults"]( - document.body, - [], - callbackFilter, - false, - ); - - expect(collectAutofillContentService["mutationObserver"].observe).not.toBeCalled(); - }); - }); - describe("setupMutationObserver", () => { it("sets up a mutation observer and observes the document element", () => { jest.spyOn(MutationObserver.prototype, "observe"); @@ -2079,7 +2079,7 @@ describe("CollectAutofillContentService", () => { }); describe("handleMutationObserverMutation", () => { - it("will set the domRecentlyMutated value to true and the noFieldsFound value to false if a form or field node has been added ", () => { + it("will set the domRecentlyMutated value to true and the noFieldsFound value to false if a form or field node has been added ", async () => { const form = document.createElement("form"); document.body.appendChild(form); const addedNodes = document.querySelectorAll("form"); @@ -2102,6 +2102,7 @@ describe("CollectAutofillContentService", () => { jest.spyOn(collectAutofillContentService as any, "isAutofillElementNodeMutated"); collectAutofillContentService["handleMutationObserverMutation"]([mutationRecord]); + await waitForIdleCallback(); expect(collectAutofillContentService["domRecentlyMutated"]).toEqual(true); expect(collectAutofillContentService["noFieldsFound"]).toEqual(false); @@ -2114,7 +2115,7 @@ describe("CollectAutofillContentService", () => { ); }); - it("removes cached autofill elements that are nested within a removed node", () => { + it("removes cached autofill elements that are nested within a removed node", async () => { const form = document.createElement("form") as ElementWithOpId; const usernameInput = document.createElement("input") as ElementWithOpId; usernameInput.setAttribute("type", "text"); @@ -2145,12 +2146,13 @@ describe("CollectAutofillContentService", () => { target: document.body, }, ]); + await waitForIdleCallback(); expect(collectAutofillContentService["autofillFormElements"].size).toEqual(0); expect(collectAutofillContentService["autofillFieldElements"].size).toEqual(0); }); - it("will handle updating the autofill element if any attribute mutations are encountered", () => { + it("will handle updating the autofill element if any attribute mutations are encountered", async () => { const mutationRecord: MutationRecord = { type: "attributes", addedNodes: null, @@ -2169,6 +2171,7 @@ describe("CollectAutofillContentService", () => { jest.spyOn(collectAutofillContentService as any, "handleAutofillElementAttributeMutation"); collectAutofillContentService["handleMutationObserverMutation"]([mutationRecord]); + await waitForIdleCallback(); expect(collectAutofillContentService["domRecentlyMutated"]).toEqual(false); expect(collectAutofillContentService["noFieldsFound"]).toEqual(true); @@ -2255,29 +2258,6 @@ describe("CollectAutofillContentService", () => { expect(collectAutofillContentService["buildAutofillFieldItem"]).not.toBeCalled(); }); - - it("builds the autofill field item to ensure the overlay listeners are set", () => { - document.body.innerHTML = ` -
- - -
- `; - - const inputElement = document.getElementById( - "username-id", - ) as ElementWithOpId; - inputElement.setAttribute("type", "password"); - const nodes = [inputElement]; - jest.spyOn(collectAutofillContentService as any, "buildAutofillFieldItem"); - - collectAutofillContentService["setupOverlayListenersOnMutatedElements"](nodes); - - expect(collectAutofillContentService["buildAutofillFieldItem"]).toBeCalledWith( - inputElement, - -1, - ); - }); }); describe("deleteCachedAutofillElement", () => { diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 909d858430..8747065578 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -15,10 +15,12 @@ import { elementIsLabelElement, elementIsSelectElement, elementIsSpanElement, - nodeIsFormElement, nodeIsElement, elementIsInputElement, elementIsTextAreaElement, + nodeIsFormElement, + nodeIsInputElement, + sendExtensionMessage, } from "../utils"; import { AutofillOverlayContentService } from "./abstractions/autofill-overlay-content.service"; @@ -42,7 +44,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte private elementInitializingIntersectionObserver: Set = new Set(); private mutationObserver: MutationObserver; private updateAutofillElementsAfterMutationTimeout: number | NodeJS.Timeout; + private mutationsQueue: MutationRecord[][] = []; private readonly updateAfterMutationTimeoutDelay = 1000; + private readonly formFieldQueryString; + private readonly nonInputFormFieldTags = new Set(["textarea", "select"]); private readonly ignoredInputTypes = new Set([ "hidden", "submit", @@ -51,6 +56,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte "image", "file", ]); + private useTreeWalkerStrategyFlagSet = false; constructor( domElementVisibilityService: DomElementVisibilityService, @@ -58,6 +64,17 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte ) { this.domElementVisibilityService = domElementVisibilityService; this.autofillOverlayContentService = autofillOverlayContentService; + + let inputQuery = "input:not([data-bwignore])"; + for (const type of this.ignoredInputTypes) { + inputQuery += `:not([type="${type}"])`; + } + this.formFieldQueryString = `${inputQuery}, textarea:not([data-bwignore]), select:not([data-bwignore]), span[data-bwautofill]`; + + void sendExtensionMessage("getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag").then( + (useTreeWalkerStrategyFlag) => + (this.useTreeWalkerStrategyFlagSet = !!useTreeWalkerStrategyFlag?.result), + ); } /** @@ -136,28 +153,86 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte } /** - * Queries the DOM for all the nodes that match the given filter callback - * and returns a collection of nodes. - * @param {Node} rootNode - * @param {Function} filterCallback - * @param {boolean} isObservingShadowRoot - * @returns {Node[]} + * Queries all elements in the DOM that match the given query string. + * Also, recursively queries all shadow roots for the element. + * + * @param root - The root element to start the query from + * @param queryString - The query string to match elements against + * @param isObservingShadowRoot - Determines whether to observe shadow roots */ - queryAllTreeWalkerNodes( - rootNode: Node, - filterCallback: CallableFunction, - isObservingShadowRoot = true, - ): Node[] { - const treeWalkerQueryResults: Node[] = []; + deepQueryElements( + root: Document | ShadowRoot | Element, + queryString: string, + isObservingShadowRoot = false, + ): T[] { + let elements = this.queryElements(root, queryString); + const shadowRoots = this.recursivelyQueryShadowRoots(root, isObservingShadowRoot); + for (let index = 0; index < shadowRoots.length; index++) { + const shadowRoot = shadowRoots[index]; + elements = elements.concat(this.queryElements(shadowRoot, queryString)); + } - this.buildTreeWalkerNodesQueryResults( - rootNode, - treeWalkerQueryResults, - filterCallback, - isObservingShadowRoot, - ); + return elements; + } - return treeWalkerQueryResults; + /** + * Queries the DOM for elements based on the given query string. + * + * @param root - The root element to start the query from + * @param queryString - The query string to match elements against + */ + private queryElements(root: Document | ShadowRoot | Element, queryString: string): T[] { + if (!root.querySelector(queryString)) { + return []; + } + + return Array.from(root.querySelectorAll(queryString)) as T[]; + } + + /** + * Recursively queries all shadow roots found within the given root element. + * Will also set up a mutation observer on the shadow root if the + * `isObservingShadowRoot` parameter is set to true. + * + * @param root - The root element to start the query from + * @param isObservingShadowRoot - Determines whether to observe shadow roots + */ + private recursivelyQueryShadowRoots( + root: Document | ShadowRoot | Element, + isObservingShadowRoot = false, + ): ShadowRoot[] { + let shadowRoots = this.queryShadowRoots(root); + for (let index = 0; index < shadowRoots.length; index++) { + const shadowRoot = shadowRoots[index]; + shadowRoots = shadowRoots.concat(this.recursivelyQueryShadowRoots(shadowRoot)); + if (isObservingShadowRoot) { + this.mutationObserver.observe(shadowRoot, { + attributes: true, + childList: true, + subtree: true, + }); + } + } + + return shadowRoots; + } + + /** + * Queries any immediate shadow roots found within the given root element. + * + * @param root - The root element to start the query from + */ + private queryShadowRoots(root: Document | ShadowRoot | Element): ShadowRoot[] { + const shadowRoots: ShadowRoot[] = []; + const potentialShadowRoots = root.querySelectorAll(":defined"); + for (let index = 0; index < potentialShadowRoots.length; index++) { + const shadowRoot = this.getShadowRoot(potentialShadowRoots[index]); + if (shadowRoot) { + shadowRoots.push(shadowRoot); + } + } + + return shadowRoots; } /** @@ -294,11 +369,12 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte fieldsLimit?: number, previouslyFoundFormFieldElements?: FormFieldElement[], ): FormFieldElement[] { - const formFieldElements = - previouslyFoundFormFieldElements || - (this.queryAllTreeWalkerNodes(document.documentElement, (node: Node) => - this.isNodeFormFieldElement(node), - ) as FormFieldElement[]); + let formFieldElements = previouslyFoundFormFieldElements; + if (!formFieldElements) { + formFieldElements = this.useTreeWalkerStrategyFlagSet + ? this.queryTreeWalkerForAutofillFormFieldElements() + : this.deepQueryElements(document, this.formFieldQueryString, true); + } if (!fieldsLimit || formFieldElements.length <= fieldsLimit) { return formFieldElements; @@ -371,7 +447,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte if (!autofillFieldBase.viewable) { this.elementInitializingIntersectionObserver.add(element); - this.intersectionObserver.observe(element); + this.intersectionObserver?.observe(element); } if (elementIsSpanElement(element)) { @@ -864,28 +940,33 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte * Queries all potential form and field elements from the DOM and returns * a collection of form and field elements. Leverages the TreeWalker API * to deep query Shadow DOM elements. - * @returns {{formElements: Node[], formFieldElements: Node[]}} - * @private */ private queryAutofillFormAndFieldElements(): { - formElements: Node[]; - formFieldElements: Node[]; + formElements: HTMLFormElement[]; + formFieldElements: FormFieldElement[]; } { - const formElements: Node[] = []; - const formFieldElements: Node[] = []; - this.queryAllTreeWalkerNodes(document.documentElement, (node: Node) => { - if (nodeIsFormElement(node)) { - formElements.push(node); - return true; + if (this.useTreeWalkerStrategyFlagSet) { + return this.queryTreeWalkerForAutofillFormAndFieldElements(); + } + + const queriedElements = this.deepQueryElements( + document, + `form, ${this.formFieldQueryString}`, + true, + ); + const formElements: HTMLFormElement[] = []; + const formFieldElements: FormFieldElement[] = []; + for (let index = 0; index < queriedElements.length; index++) { + const element = queriedElements[index]; + if (elementIsFormElement(element)) { + formElements.push(element); + continue; } - if (this.isNodeFormFieldElement(node)) { - formFieldElements.push(node); - return true; + if (this.isNodeFormFieldElement(element)) { + formFieldElements.push(element); } - - return false; - }); + } return { formElements, formFieldElements }; } @@ -916,7 +997,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte return true; } - return ["textarea", "select"].includes(nodeTagName) && !nodeHasBwIgnoreAttribute; + return this.nonInputFormFieldTags.has(nodeTagName) && !nodeHasBwIgnoreAttribute; } /** @@ -928,7 +1009,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte * @param {Node} node */ private getShadowRoot(node: Node): ShadowRoot | null { - if (!nodeIsElement(node) || node.childNodes.length !== 0) { + if (!nodeIsElement(node)) { return null; } @@ -947,51 +1028,6 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte return (node as any).openOrClosedShadowRoot; } - /** - * Recursively builds a collection of nodes that match the given filter callback. - * If a node has a ShadowRoot, it will be observed for mutations. - * @param {Node} rootNode - * @param {Node[]} treeWalkerQueryResults - * @param {Function} filterCallback - * @param {boolean} isObservingShadowRoot - * @private - */ - private buildTreeWalkerNodesQueryResults( - rootNode: Node, - treeWalkerQueryResults: Node[], - filterCallback: CallableFunction, - isObservingShadowRoot: boolean, - ) { - const treeWalker = document?.createTreeWalker(rootNode, NodeFilter.SHOW_ELEMENT); - let currentNode = treeWalker?.currentNode; - - while (currentNode) { - if (filterCallback(currentNode)) { - treeWalkerQueryResults.push(currentNode); - } - - const nodeShadowRoot = this.getShadowRoot(currentNode); - if (nodeShadowRoot) { - if (isObservingShadowRoot) { - this.mutationObserver.observe(nodeShadowRoot, { - attributes: true, - childList: true, - subtree: true, - }); - } - - this.buildTreeWalkerNodesQueryResults( - nodeShadowRoot, - treeWalkerQueryResults, - filterCallback, - isObservingShadowRoot, - ); - } - - currentNode = treeWalker?.nextNode(); - } - } - /** * Sets up a mutation observer on the body of the document. Observes changes to * DOM elements to ensure we have an updated set of autofill field data. @@ -1020,29 +1056,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte return; } - for (let mutationsIndex = 0; mutationsIndex < mutations.length; mutationsIndex++) { - const mutation = mutations[mutationsIndex]; - if ( - mutation.type === "childList" && - (this.isAutofillElementNodeMutated(mutation.removedNodes, true) || - this.isAutofillElementNodeMutated(mutation.addedNodes)) - ) { - this.domRecentlyMutated = true; - if (this.autofillOverlayContentService) { - this.autofillOverlayContentService.pageDetailsUpdateRequired = true; - } - this.noFieldsFound = false; - continue; - } - - if (mutation.type === "attributes") { - this.handleAutofillElementAttributeMutation(mutation); - } - } - - if (this.domRecentlyMutated) { - this.updateAutofillElementsAfterMutation(); + if (!this.mutationsQueue.length) { + globalThis.requestIdleCallback(this.processMutations, { timeout: 500 }); } + this.mutationsQueue.push(mutations); }; /** @@ -1065,12 +1082,54 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte this.updateAutofillElementsAfterMutation(); } + /** + * Handles the processing of all mutations in the mutations queue. Will trigger + * within an idle callback to help with performance and prevent excessive updates. + */ + private processMutations = () => { + for (let queueIndex = 0; queueIndex < this.mutationsQueue.length; queueIndex++) { + this.processMutationRecord(this.mutationsQueue[queueIndex]); + } + + if (this.domRecentlyMutated) { + this.updateAutofillElementsAfterMutation(); + } + + this.mutationsQueue = []; + }; + + /** + * Processes a mutation record and updates the autofill elements if necessary. + * + * @param mutations - The mutation record to process + */ + private processMutationRecord(mutations: MutationRecord[]) { + for (let mutationIndex = 0; mutationIndex < mutations.length; mutationIndex++) { + const mutation = mutations[mutationIndex]; + if ( + mutation.type === "childList" && + (this.isAutofillElementNodeMutated(mutation.removedNodes, true) || + this.isAutofillElementNodeMutated(mutation.addedNodes)) + ) { + this.domRecentlyMutated = true; + if (this.autofillOverlayContentService) { + this.autofillOverlayContentService.pageDetailsUpdateRequired = true; + } + this.noFieldsFound = false; + continue; + } + + if (mutation.type === "attributes") { + this.handleAutofillElementAttributeMutation(mutation); + } + } + } + /** * Checks if the passed nodes either contain or are autofill elements. - * @param {NodeList} nodes - * @param {boolean} isRemovingNodes - * @returns {boolean} - * @private + * + * @param nodes - The nodes to check + * @param isRemovingNodes - Whether the nodes are being removed */ private isAutofillElementNodeMutated(nodes: NodeList, isRemovingNodes = false): boolean { if (!nodes.length) { @@ -1078,34 +1137,41 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte } let isElementMutated = false; - const mutatedElements: Node[] = []; + let mutatedElements: HTMLElement[] = []; for (let index = 0; index < nodes.length; index++) { const node = nodes[index]; if (!nodeIsElement(node)) { continue; } - const autofillElementNodes = this.queryAllTreeWalkerNodes( - node, - (walkerNode: Node) => - nodeIsFormElement(walkerNode) || this.isNodeFormFieldElement(walkerNode), - ) as HTMLElement[]; + if ( + !this.useTreeWalkerStrategyFlagSet && + (nodeIsFormElement(node) || this.isNodeFormFieldElement(node)) + ) { + mutatedElements.push(node as HTMLElement); + } - if (autofillElementNodes.length) { + const autofillElements = this.useTreeWalkerStrategyFlagSet + ? this.queryTreeWalkerForMutatedElements(node) + : this.deepQueryElements(node, `form, ${this.formFieldQueryString}`, true); + if (autofillElements.length) { + mutatedElements = mutatedElements.concat(autofillElements); + } + + if (mutatedElements.length) { isElementMutated = true; - mutatedElements.push(...autofillElementNodes); } } if (isRemovingNodes) { for (let elementIndex = 0; elementIndex < mutatedElements.length; elementIndex++) { - const node = mutatedElements[elementIndex]; + const element = mutatedElements[elementIndex]; this.deleteCachedAutofillElement( - node as ElementWithOpId | ElementWithOpId, + element as ElementWithOpId | ElementWithOpId, ); } } else if (this.autofillOverlayContentService) { - setTimeout(() => this.setupOverlayListenersOnMutatedElements(mutatedElements), 1000); + this.setupOverlayListenersOnMutatedElements(mutatedElements); } return isElementMutated; @@ -1122,15 +1188,18 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte for (let elementIndex = 0; elementIndex < mutatedElements.length; elementIndex++) { const node = mutatedElements[elementIndex]; if ( - this.isNodeFormFieldElement(node) && - !this.autofillFieldElements.get(node as ElementWithOpId) + !this.isNodeFormFieldElement(node) || + this.autofillFieldElements.get(node as ElementWithOpId) ) { + continue; + } + + globalThis.requestIdleCallback( // We are setting this item to a -1 index because we do not know its position in the DOM. // This value should be updated with the next call to collect page details. - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.buildAutofillFieldItem(node as ElementWithOpId, -1); - } + () => void this.buildAutofillFieldItem(node as ElementWithOpId, -1), + { timeout: 1000 }, + ); } } @@ -1360,7 +1429,7 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte cachedAutofillFieldElement, ); - this.intersectionObserver.unobserve(entry.target); + this.intersectionObserver?.unobserve(entry.target); } }; @@ -1375,6 +1444,150 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte this.mutationObserver?.disconnect(); this.intersectionObserver?.disconnect(); } + + /** + * Queries the DOM for all the nodes that match the given filter callback + * and returns a collection of nodes. + * @param rootNode + * @param filterCallback + * @param isObservingShadowRoot + * + * @deprecated - This method remains as a fallback in the case that the deepQuery implementation fails. + */ + private queryAllTreeWalkerNodes( + rootNode: Node, + filterCallback: CallableFunction, + isObservingShadowRoot = true, + ): Node[] { + const treeWalkerQueryResults: Node[] = []; + + this.buildTreeWalkerNodesQueryResults( + rootNode, + treeWalkerQueryResults, + filterCallback, + isObservingShadowRoot, + ); + + return treeWalkerQueryResults; + } + + /** + * Recursively builds a collection of nodes that match the given filter callback. + * If a node has a ShadowRoot, it will be observed for mutations. + * + * @param rootNode + * @param treeWalkerQueryResults + * @param filterCallback + * + * @deprecated - This method remains as a fallback in the case that the deepQuery implementation fails. + */ + private buildTreeWalkerNodesQueryResults( + rootNode: Node, + treeWalkerQueryResults: Node[], + filterCallback: CallableFunction, + isObservingShadowRoot: boolean, + ) { + const treeWalker = document?.createTreeWalker(rootNode, NodeFilter.SHOW_ELEMENT); + let currentNode = treeWalker?.currentNode; + + while (currentNode) { + if (filterCallback(currentNode)) { + treeWalkerQueryResults.push(currentNode); + } + + const nodeShadowRoot = this.getShadowRoot(currentNode); + if (nodeShadowRoot) { + if (isObservingShadowRoot) { + this.mutationObserver.observe(nodeShadowRoot, { + attributes: true, + childList: true, + subtree: true, + }); + } + + this.buildTreeWalkerNodesQueryResults( + nodeShadowRoot, + treeWalkerQueryResults, + filterCallback, + isObservingShadowRoot, + ); + } + + currentNode = treeWalker?.nextNode(); + } + } + + /** + * @deprecated - This method remains as a fallback in the case that the deepQuery implementation fails. + */ + private queryTreeWalkerForAutofillFormAndFieldElements(): { + formElements: HTMLFormElement[]; + formFieldElements: FormFieldElement[]; + } { + const formElements: HTMLFormElement[] = []; + const formFieldElements: FormFieldElement[] = []; + this.queryAllTreeWalkerNodes(document.documentElement, (node: Node) => { + if (nodeIsFormElement(node)) { + formElements.push(node); + return true; + } + + if (this.isNodeFormFieldElement(node)) { + formFieldElements.push(node as FormFieldElement); + return true; + } + + return false; + }); + + return { formElements, formFieldElements }; + } + + /** + * @deprecated - This method remains as a fallback in the case that the deepQuery implementation fails. + */ + private queryTreeWalkerForAutofillFormFieldElements(): FormFieldElement[] { + return this.queryAllTreeWalkerNodes(document.documentElement, (node: Node) => + this.isNodeFormFieldElement(node), + ) as FormFieldElement[]; + } + + /** + * @deprecated - This method remains as a fallback in the case that the deepQuery implementation fails. + * + * @param node - The node to query + */ + private queryTreeWalkerForMutatedElements(node: Node): HTMLElement[] { + return this.queryAllTreeWalkerNodes( + node, + (walkerNode: Node) => + nodeIsFormElement(walkerNode) || this.isNodeFormFieldElement(walkerNode), + ) as HTMLElement[]; + } + + /** + * @deprecated - This method remains as a fallback in the case that the deepQuery implementation fails. + */ + private queryTreeWalkerForPasswordElements(): HTMLElement[] { + return this.queryAllTreeWalkerNodes( + document.documentElement, + (node: Node) => nodeIsInputElement(node) && node.type === "password", + false, + ) as HTMLElement[]; + } + + /** + * This is a temporary method to maintain a fallback strategy for the tree walker API + * + * @deprecated - This method remains as a fallback in the case that the deepQuery implementation fails. + */ + isPasswordFieldWithinDocument(): boolean { + if (this.useTreeWalkerStrategyFlagSet) { + return Boolean(this.queryTreeWalkerForPasswordElements()?.length); + } + + return Boolean(this.deepQueryElements(document, `input[type="password"]`)?.length); + } } export default CollectAutofillContentService; diff --git a/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts index 72bbfbf2e2..6ee5171e58 100644 --- a/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts @@ -1,6 +1,7 @@ import { EVENTS } from "@bitwarden/common/autofill/constants"; import AutofillScript, { FillScript, FillScriptActions } from "../models/autofill-script"; +import { mockQuerySelectorAllDefinedCall } from "../spec/testing-utils"; import { FillableFormFieldElement, FormElementWithAttribute, FormFieldElement } from "../types"; import AutofillOverlayContentService from "./autofill-overlay-content.service"; @@ -71,6 +72,7 @@ describe("InsertAutofillContentService", () => { ); let insertAutofillContentService: InsertAutofillContentService; let fillScript: AutofillScript; + const mockQuerySelectorAll = mockQuerySelectorAllDefinedCall(); beforeEach(() => { document.body.innerHTML = mockLoginForm; @@ -99,11 +101,16 @@ describe("InsertAutofillContentService", () => { afterEach(() => { jest.restoreAllMocks(); + jest.clearAllTimers(); windowLocationSpy.mockRestore(); confirmSpy.mockRestore(); document.body.innerHTML = ""; }); + afterAll(() => { + mockQuerySelectorAll.mockRestore(); + }); + describe("fillForm", () => { it("returns early if the passed fill script does not have a script property", async () => { fillScript.script = []; diff --git a/apps/browser/src/autofill/services/insert-autofill-content.service.ts b/apps/browser/src/autofill/services/insert-autofill-content.service.ts index 5cfa8091c4..e475ea4bbc 100644 --- a/apps/browser/src/autofill/services/insert-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/insert-autofill-content.service.ts @@ -7,7 +7,6 @@ import { elementIsInputElement, elementIsSelectElement, elementIsTextAreaElement, - nodeIsInputElement, } from "../utils"; import { InsertAutofillContentService as InsertAutofillContentServiceInterface } from "./abstractions/insert-autofill-content.service"; @@ -101,13 +100,7 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf * @private */ private isPasswordFieldWithinDocument(): boolean { - return Boolean( - this.collectAutofillContentService.queryAllTreeWalkerNodes( - document.documentElement, - (node: Node) => nodeIsInputElement(node) && node.type === "password", - false, - )?.length, - ); + return this.collectAutofillContentService.isPasswordFieldWithinDocument(); } /** diff --git a/apps/browser/src/autofill/spec/testing-utils.ts b/apps/browser/src/autofill/spec/testing-utils.ts index 15f3121c2b..6d07e6606e 100644 --- a/apps/browser/src/autofill/spec/testing-utils.ts +++ b/apps/browser/src/autofill/spec/testing-utils.ts @@ -98,6 +98,34 @@ function triggerTabOnRemovedEvent(tabId: number, removeInfo: chrome.tabs.TabRemo }); } +function mockQuerySelectorAllDefinedCall() { + const originalDocumentQuerySelectorAll = document.querySelectorAll; + document.querySelectorAll = function (selector: string) { + return originalDocumentQuerySelectorAll.call( + document, + selector === ":defined" ? "*" : selector, + ); + }; + + const originalShadowRootQuerySelectorAll = ShadowRoot.prototype.querySelectorAll; + ShadowRoot.prototype.querySelectorAll = function (selector: string) { + return originalShadowRootQuerySelectorAll.call(this, selector === ":defined" ? "*" : selector); + }; + + const originalElementQuerySelectorAll = Element.prototype.querySelectorAll; + Element.prototype.querySelectorAll = function (selector: string) { + return originalElementQuerySelectorAll.call(this, selector === ":defined" ? "*" : selector); + }; + + return { + mockRestore: () => { + document.querySelectorAll = originalDocumentQuerySelectorAll; + ShadowRoot.prototype.querySelectorAll = originalShadowRootQuerySelectorAll; + Element.prototype.querySelectorAll = originalElementQuerySelectorAll; + }, + }; +} + export { triggerTestFailure, flushPromises, @@ -111,4 +139,5 @@ export { triggerTabOnReplacedEvent, triggerTabOnUpdatedEvent, triggerTabOnRemovedEvent, + mockQuerySelectorAllDefinedCall, }; diff --git a/apps/browser/src/autofill/utils/index.ts b/apps/browser/src/autofill/utils/index.ts index 72e7f9ab62..cbb0a862e3 100644 --- a/apps/browser/src/autofill/utils/index.ts +++ b/apps/browser/src/autofill/utils/index.ts @@ -240,7 +240,11 @@ function elementIsDescriptionTermElement(element: Element): element is HTMLEleme * @param node - The node to check. */ function nodeIsElement(node: Node): node is Element { - return node?.nodeType === Node.ELEMENT_NODE; + if (!node) { + return false; + } + + return node.nodeType === Node.ELEMENT_NODE; } /** diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 388060d88c..7ddd5302ac 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -4,6 +4,7 @@ import { NotificationsService } from "@bitwarden/common/abstractions/notificatio import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -65,7 +66,10 @@ export default class RuntimeBackground { sender: chrome.runtime.MessageSender, sendResponse: (response: any) => void, ) => { - const messagesWithResponse = ["biometricUnlock"]; + const messagesWithResponse = [ + "biometricUnlock", + "getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag", + ]; if (messagesWithResponse.includes(msg.command)) { this.processMessageWithSender(msg, sender).then( @@ -177,6 +181,11 @@ export default class RuntimeBackground { const result = await this.main.biometricUnlock(); return result; } + case "getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag": { + return await this.configService.getFeatureFlag( + FeatureFlag.UseTreeWalkerApiForPageDetailsCollection, + ); + } } } diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index ef8c4d61e4..dd400da7bb 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -17,6 +17,7 @@ export enum FeatureFlag { EnableDeleteProvider = "AC-1218-delete-provider", ExtensionRefresh = "extension-refresh", RestrictProviderAccess = "restrict-provider-access", + UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -44,6 +45,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.EnableDeleteProvider]: FALSE, [FeatureFlag.ExtensionRefresh]: FALSE, [FeatureFlag.RestrictProviderAccess]: FALSE, + [FeatureFlag.UseTreeWalkerApiForPageDetailsCollection]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue;