-
-
Couldn't load subscription status.
- Fork 0
FEAT: Identify and resolved major performance issues + Build enhanced performance system and landing site #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Fix: Content script optimizations (throttling, limited DOM scan) Chore: Mutation observer (now ignores extension generated elements) Fix: Background script optimizations (throttling for tab updates, storage optimized) Chore: Improvised manifest (document_idle, http/https, all_frames)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances performance and robustness across content and background scripts by introducing throttling, limiting DOM scans, optimizing observers, and refining manifest settings.
- Introduces throttled and debounced DOM scanning in the content script with early exits and node processing limits.
- Refines the mutation observer to ignore extension-generated elements and pauses/resumes on visibility changes.
- Implements throttled tab update handling and robust storage operations in the background worker.
- Updates manifest permissions to specific URL patterns and adjusts script injection timing.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/content/content.js | Added throttling, limited DOM scanning, early exits, observer optimizations, and cleanup logic. |
| src/background/background.js | Introduced tab update throttling, error handling in storage operations, and guard clauses. |
| src/App.jsx | Wrapped handlers with useCallback, memoized current year, and reduced renders. |
| public/manifest.json | Scoped host_permissions, set run_at to document_idle, and added all_frames. |
src/content/content.js
Outdated
| window.addEventListener("beforeunload", () => { | ||
| this.destroy(); | ||
| }); | ||
|
|
||
| document.addEventListener("visibilitychange", () => { | ||
| if (document.visibilityState === "hidden") { | ||
| this.pauseOperations(); | ||
| } else { | ||
| this.resumeOperations(); | ||
| } | ||
| }); |
Copilot
AI
Jun 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event listeners added in setupCleanup are never removed in destroy, which may cause unexpected behavior after tear-down. Consider storing references to the handlers and removing them during destroy.
| window.addEventListener("beforeunload", () => { | |
| this.destroy(); | |
| }); | |
| document.addEventListener("visibilitychange", () => { | |
| if (document.visibilityState === "hidden") { | |
| this.pauseOperations(); | |
| } else { | |
| this.resumeOperations(); | |
| } | |
| }); | |
| this.beforeUnloadHandler = () => { | |
| this.destroy(); | |
| }; | |
| this.visibilityChangeHandler = () => { | |
| if (document.visibilityState === "hidden") { | |
| this.pauseOperations(); | |
| } else { | |
| this.resumeOperations(); | |
| } | |
| }; | |
| window.addEventListener("beforeunload", this.beforeUnloadHandler); | |
| document.addEventListener("visibilitychange", this.visibilityChangeHandler); |
| chrome.runtime.sendMessage({ type: "TOGGLE_MASKING" }); | ||
| } | ||
|
|
||
| throttledScanPage() { |
Copilot
AI
Jun 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reusing the same scanDebounceTimer for both throttledScanPage and debouncedScanNewContent can lead to scheduling conflicts. It may be clearer to use separate timers for page scanning and new-content scanning.
| return true; | ||
| } | ||
|
|
||
| throttledTabUpdate(tabId, changeInfo, tab) { |
Copilot
AI
Jun 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If tabId is undefined, you end up storing and clearing a key of undefined in the throttle map. Add an early return guard when tabId is falsy to avoid unintended behavior.
| throttledTabUpdate(tabId, changeInfo, tab) { | |
| throttledTabUpdate(tabId, changeInfo, tab) { | |
| if (!tabId) { | |
| console.warn("Safe-Web: Received falsy tabId in throttledTabUpdate"); | |
| return; | |
| } |
src/background/background.js
Outdated
| phone: /(\+1\s?)?(\([0-9]{3}\)|[0-9]{3})[\s\-]?[0-9]{3}[\s\-]?[0-9]{4}/g, | ||
| ssn: /\b\d{3}-?\d{2}-?\d{4}\b/g, | ||
| creditCard: /\b\d{4}[\s\-]?\d{4}[\s\-]?\d{4}[\s\-]?\d{4}\b/g, | ||
| emails: ( |
Copilot
AI
Jun 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keys returned by analyzeContent (emails, phones, ssns, creditCards) don’t match the sensitivePatterns setting keys (email, phone, ssn, creditCard). Consider aligning these names or mapping them consistently.
Fix: extension detection and error handling to performance test Fix: enhanced extension communication and initial masking performance
…lability, and details
…-LD schema, technical SEO, content optimization
…-LD schema, technical SEO, content optimization
FEAT: Extension installation landing site + main website (landing-page)
|
@RianaAzad I think it's ready to merge. Check & merge |
|
@FardinHash two things need to be fixed.
|
Fix: Content script optimizations (throttling, limited DOM scan)
Chore: Mutation observer (now ignores extension generated elements)
Fix: Background script optimizations (throttling for tab updates, storage optimized)
Chore: Improvised manifest (document_idle, http/https, all_frames)