Speed up tests and other perf improvements (#381)

* Mock getComputedStyles to speed up tests Signed-off-by: Emanuele Feliziani <[email protected]> * Batch form file reading Signed-off-by: Emanuele Feliziani <[email protected]> * Memoize isCCForm Signed-off-by: Emanuele Feliziani <[email protected]> * Minimise impact of Array.from Signed-off-by: Emanuele Feliziani <[email protected]> * Limit test logs to surface failures Signed-off-by: Emanuele Feliziani <[email protected]> * Centralize scan stopping and disconnect observer Signed-off-by: Emanuele Feliziani <[email protected]> * Update snapshots and commit compiled files Signed-off-by: Emanuele Feliziani <[email protected]> * Commit compiled code Signed-off-by: Emanuele Feliziani <[email protected]> * Add performance logging capability Signed-off-by: Emanuele Feliziani <[email protected]> # Conflicts: # src/Scanner.js * Commit files Signed-off-by: Emanuele Feliziani <[email protected]> * Add comment Signed-off-by: Emanuele Feliziani <[email protected]> * Minor change Signed-off-by: Emanuele Feliziani <[email protected]> * Improve stopping the scanner Signed-off-by: Emanuele Feliziani <[email protected]> * Update privacy pages urls Signed-off-by: Emanuele Feliziani <[email protected]> * Improved performance logging Signed-off-by: Emanuele Feliziani <[email protected]> * Don't trim empty strings Signed-off-by: Emanuele Feliziani <[email protected]> * Minor improvement to getText Signed-off-by: Emanuele Feliziani <[email protected]> * Don't repeat isFormLikelyToBeUsedAsPageWrapper Signed-off-by: Emanuele Feliziani <[email protected]> * Improve for searching loop Signed-off-by: Emanuele Feliziani <[email protected]> * Commit compiled files Signed-off-by: Emanuele Feliziani <[email protected]> --------- Signed-off-by: Emanuele Feliziani <[email protected]>
duckduckgo · Sep 15, 2023 · 21aa20e · 21aa20e
1 parent e19e000
commit 21aa20e
Show file tree

Hide file tree

Showing 19 changed files with 1,111 additions and 469 deletions.
diff --git a/dist/autofill-debug.js b/dist/autofill-debug.js
diff --git a/dist/autofill.js b/dist/autofill.js
diff --git a/docs/runtime.windows.md b/docs/runtime.windows.md
@@ -1,6 +1,6 @@
 ## Links 
 
-- [Privacy Test Pages, Form Submissions](https://privacy-test-pages.glitch.me/autofill/form-submission.html)
+- [Privacy Test Pages, Form Submissions](https://privacy-test-pages.site/autofill/form-submission.html)
 
 ## `getRuntimeConfiguration()`
 

diff --git a/jest.config.js b/jest.config.js
@@ -14,7 +14,7 @@ module.exports = {
     testEnvironment: './jest-test-environment.js',
 
     // Indicates whether each individual test should be reported during the run
-    verbose: true,
+    verbose: false,
 
     // ensure snapshots are in a JSON format
     snapshotFormat: {

diff --git a/jest.setup.js b/jest.setup.js
@@ -111,3 +111,19 @@ Object.defineProperty(window.HTMLElement.prototype, 'offsetHeight', {
         return this._jsdomMockOffsetHeight || 0
     }
 })
+
+// getComputedStyle is super slow on jsdom, by providing this mock we speed tests up significantly
+const defaultStyle = {
+    display: 'block',
+    visibility: 'visible',
+    opacity: '1',
+    paddingRight: '10'
+}
+const mockGetComputedStyle = (el) => {
+    return {
+        // since we don't load stylesheets in the tests, the style prop is all the css applied, so it's a safe fallback
+        getPropertyValue: (prop) => el.style?.[prop] || defaultStyle[prop]
+    }
+}
+// @ts-ignore
+jest.spyOn(window, 'getComputedStyle').mockImplementation(mockGetComputedStyle)
diff --git a/package.json b/package.json
@@ -13,7 +13,7 @@
     "lint": "eslint .",
     "lint:fix": "npm run lint -- --fix",
     "copy-assets": "node scripts/copy-assets.js",
-    "open-test-extension": "npx web-ext run -t chromium -u https://privacy-test-pages.glitch.me/ -s integration-test/extension",
+    "open-test-extension": "npx web-ext run -t chromium -u https://privacy-test-pages.site/ -s integration-test/extension",
     "schema:generate": "node scripts/api-call-generator.js",
     "test": "npm run test:unit && npm run lint && tsc",
     "test:clean-tree": "npm run build && sh scripts/check-for-changes.sh",

diff --git a/src/DeviceInterface/ExtensionInterface.js b/src/DeviceInterface/ExtensionInterface.js
@@ -57,13 +57,8 @@ class ExtensionInterface extends InterfacePrototype {
         return null
     }
 
-    removeAutofillUIFromPage () {
-        super.removeAutofillUIFromPage()
-        this.activeForm?.removeAllDecorations()
-    }
-
     async resetAutofillUI (callback) {
-        this.removeAutofillUIFromPage()
+        this.removeAutofillUIFromPage('Resetting autofill.')
 
         await this.setupAutofill()
 

diff --git a/src/DeviceInterface/InterfacePrototype.js b/src/DeviceInterface/InterfacePrototype.js
@@ -73,7 +73,7 @@ class InterfacePrototype {
     /** @type {boolean} */
     isInitializationStarted;
 
-    /** @type {(()=>void) | null} */
+    /** @type {((reason, ...rest) => void) | null} */
     _scannerCleanup = null
 
     /**
@@ -102,9 +102,12 @@ class InterfacePrototype {
         return new NativeUIController()
     }
 
-    removeAutofillUIFromPage () {
+    /**
+     * @param {string} reason
+     */
+    removeAutofillUIFromPage (reason) {
         this.uiController?.destroy()
-        this._scannerCleanup?.()
+        this._scannerCleanup?.(reason)
     }
 
     get hasLocalAddresses () {
@@ -332,7 +335,7 @@ class InterfacePrototype {
     postInit () {
         const cleanup = this.scanner.init()
         this.addLogoutListener(() => {
-            cleanup()
+            cleanup('Logged out')
             if (this.globalConfig.isDDGDomain) {
                 notifyWebApp({ deviceSignedIn: {value: false} })
             }

diff --git a/src/Form/Form.js b/src/Form/Form.js
@@ -128,6 +128,9 @@ class Form {
     get isHybrid () {
         return this.formAnalyzer.isHybrid
     }
+    get isCCForm () {
+        return this.formAnalyzer.isCCForm()
+    }
 
     logFormInfo () {
         if (!shouldLog()) return
@@ -352,6 +355,7 @@ class Form {
     destroy () {
         this.removeAllDecorations()
         this.removeTooltip()
+        this.forgetAllInputs()
         this.mutObs.disconnect()
         this.matching.clear()
         this.intObs = null
@@ -451,6 +455,7 @@ class Form {
         const opts = {
             isLogin: this.isLogin,
             isHybrid: this.isHybrid,
+            isCCForm: this.isCCForm,
             hasCredentials: Boolean(this.device.settings.availableInputTypes.credentials?.username),
             supportsIdentitiesAutofill: this.device.settings.featureToggles.inputType_identities
         }

diff --git a/src/Form/FormAnalyzer.js b/src/Form/FormAnalyzer.js
@@ -277,6 +277,45 @@ class FormAnalyzer {
         }
         return this
     }
+
+    /** @type {undefined|boolean} */
+    _isCCForm = undefined
+    /**
+     * Tries to infer if it's a credit card form
+     * @returns {boolean}
+     */
+    isCCForm () {
+        if (this._isCCForm !== undefined) return this._isCCForm
+
+        const formEl = this.form
+        const ccFieldSelector = this.matching.joinCssSelectors('cc')
+        if (!ccFieldSelector) {
+            this._isCCForm = false
+            return this._isCCForm
+        }
+        const hasCCSelectorChild = formEl.matches(ccFieldSelector) || formEl.querySelector(ccFieldSelector)
+        // If the form contains one of the specific selectors, we have high confidence
+        if (hasCCSelectorChild) {
+            this._isCCForm = true
+            return this._isCCForm
+        }
+
+        // Read form attributes to find a signal
+        const hasCCAttribute = [...formEl.attributes].some(({name, value}) =>
+            /(credit|payment).?card/i.test(`${name}=${value}`)
+        )
+        if (hasCCAttribute) {
+            this._isCCForm = true
+            return this._isCCForm
+        }
+
+        // Match form textContent against common cc fields (includes hidden labels)
+        const textMatches = formEl.textContent?.match(/(credit|payment).?card(.?number)?|ccv|security.?code|cvv|cvc|csc/ig)
+
+        // We check for more than one to minimise false positives
+        this._isCCForm = Boolean(textMatches && textMatches.length > 1)
+        return this._isCCForm
+    }
 }
 
 export default FormAnalyzer
diff --git a/src/Form/input-classifiers.test.js b/src/Form/input-classifiers.test.js
@@ -13,6 +13,9 @@ import {createAvailableInputTypes} from '../../integration-test/helpers/utils.js
  * @type {object[]}
  */
 const testCases = JSON.parse(fs.readFileSync(path.join(__dirname, 'test-cases/index.json')).toString('utf-8'))
+testCases.forEach(testCase => {
+    testCase.testContent = fs.readFileSync(path.resolve(__dirname, './test-cases', testCase.html), 'utf-8')
+})
 
 /**
  * @param {HTMLInputElement} el
@@ -147,15 +150,16 @@ describe.each(testCases)('Test $html fields', (testCase) => {
         expectedSubmitFalsePositives = 0,
         expectedSubmitFalseNegatives = 0,
         title = '__test__',
-        hasExtraWrappers = true
+        hasExtraWrappers = true,
+        testContent
     } = testCase
 
     const testTextString = expectedFailures.length > 0
         ? `should contain ${expectedFailures.length} known failure(s): ${JSON.stringify(expectedFailures)}`
         : `should NOT contain failures`
 
-    it(testTextString, () => {
-        const testContent = fs.readFileSync(path.resolve(__dirname, './test-cases', html), 'utf-8')
+    it.concurrent(testTextString, async () => {
+        document.body.innerHTML = ''
 
         let baseWrapper = document.body
 

diff --git a/src/Form/matching.js b/src/Form/matching.js
@@ -223,7 +223,7 @@ class Matching {
 
         // // For CC forms we run aggressive matches, so we want to make sure we only
         // // run them on actual CC forms to avoid false positives and expensive loops
-        if (this.isCCForm(formEl)) {
+        if (opts.isCCForm) {
             const subtype = this.subtypeFromMatchers('cc', input)
             if (subtype && isValidCreditCardSubtype(subtype)) {
                 return `creditCards.${subtype}`
@@ -278,6 +278,7 @@ class Matching {
      * @typedef {{
      *   isLogin?: boolean,
      *   isHybrid?: boolean,
+     *   isCCForm?: boolean,
      *   hasCredentials?: boolean,
      *   supportsIdentitiesAutofill?: boolean
      * }} SetInputTypeOpts
@@ -559,32 +560,6 @@ class Matching {
         this.setActiveElementStrings(input, form)
         return this
     }
-    /**
-     * Tries to infer if it's a credit card form
-     * @param {HTMLElement} formEl
-     * @returns {boolean}
-     */
-    isCCForm (formEl) {
-        const ccFieldSelector = this.joinCssSelectors('cc')
-        if (!ccFieldSelector) {
-            return false
-        }
-        const hasCCSelectorChild = formEl.matches(ccFieldSelector) || formEl.querySelector(ccFieldSelector)
-        // If the form contains one of the specific selectors, we have high confidence
-        if (hasCCSelectorChild) return true
-
-        // Read form attributes to find a signal
-        const hasCCAttribute = [...formEl.attributes].some(({name, value}) =>
-            /(credit|payment).?card/i.test(`${name}=${value}`)
-        )
-        if (hasCCAttribute) return true
-
-        // Match form textContent against common cc fields (includes hidden labels)
-        const textMatches = formEl.textContent?.match(/(credit|payment).?card(.?number)?|ccv|security.?code|cvv|cvc|csc/ig)
-
-        // We check for more than one to minimise false positives
-        return Boolean(textMatches && textMatches.length > 1)
-    }
 
     /**
      * @type {MatchingConfiguration}
@@ -759,7 +734,9 @@ function getInputSubtype (input) {
  * @return {string}
  */
 const removeExcessWhitespace = (string = '') => {
-    return (string || '')
+    if (!string) return ''
+
+    return (string)
         .replace(/\n/g, ' ')
         .replace(/\s{2,}/g, ' ').trim()
 }

diff --git a/src/Form/matching.test.js b/src/Form/matching.test.js
@@ -92,9 +92,21 @@ describe('matching', () => {
         { html: `<input placeholder="captcha-password" />`, subtype: 'unknown' },
         { html: `<input placeholder="username" />`, subtype: 'credentials.username' },
         { html: `<input name="username-search" />`, subtype: 'unknown' },
-        { html: `<input name="cc-name" />`, subtype: 'creditCards.cardName' },
-        { html: `<input name="accountholdername" /><!-- second input is to trigger cc type --><input name="cc-number"/>`, subtype: 'creditCards.cardName' },
-        { html: `<input name="Срок действия карты" /><!-- second input is to trigger cc type --><input name="cc-number"/>`, subtype: 'creditCards.expirationMonth' },
+        {
+            html: `<input name="cc-name" />`,
+            subtype: 'creditCards.cardName',
+            opts: {isCCForm: true}
+        },
+        {
+            html: `<input name="accountholdername" /><!-- second input is to trigger cc type --><input name="cc-number"/>`,
+            subtype: 'creditCards.cardName',
+            opts: {isCCForm: true}
+        },
+        {
+            html: `<input name="Срок действия карты" /><!-- second input is to trigger cc type --><input name="cc-number"/>`,
+            subtype: 'creditCards.expirationMonth',
+            opts: {isCCForm: true}
+        },
         { html: `<input placeholder="ZIP code" autocomplete="shipping postal-code" type="text" name="checkout[shipping_address][zip]"/>`, subtype: 'identities.addressPostalCode' },
         { html: `<input autocomplete="on" id="address_line2" name="address_line2" type="text">`, subtype: 'identities.addressStreet2' },
         { html: `<input name="ADDRESS_LINE_1" type="text" aria-required="true" aria-describedby="ariaId_29" aria-labelledby="ariaId_30" autocomplete="off-street-address" aria-autocomplete="list">`, subtype: 'identities.addressStreet' },

diff --git a/src/InContextSignup.js b/src/InContextSignup.js
@@ -110,7 +110,7 @@ export class InContextSignup {
 
     onIncontextSignupDismissed (options = { shouldHideTooltip: true }) {
         if (options.shouldHideTooltip) {
-            this.device.removeAutofillUIFromPage()
+            this.device.removeAutofillUIFromPage('Email Protection in-context signup dismissed.')
             this.device.deviceApi.notify(new CloseAutofillParentCall(null))
         }
         this.permanentlyDismissedAt = new Date().getTime()