perf: Streamlining scans and messaging; Debugging info #428
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
matatk
commented
Jun 26, 2021
matatk
commented
- Add debugging output to debug builds.
- Add rollup-plugin-strip to remove debugging info from production builds.
- Add a script to compare different builds (browser-debug and cross-browser).
- Clarify and update a load of comments.
- Count mutations not caused by mutations separately. (Scans scheduled after a pause caused by a mutation count as having been caused by mutations.)
- Factor out doing stuff with the active tab, or all tabs.
- Work around errors when sending messages to non-scriptable pages in Firefox and Opera (where the sidebar may be open) by checking for the error.
- Clean up background script code.
- Rename several functions in the content script to clear up what they do.
- Don't scan for landmarks on content script startup; wait until the devtools-state message is received.
- Don't scan for landmarks if the page is not visible when that message is received.
- Do scan for landmarks if we are asked and haven't scanned yet (otherwise don't).
- Fix detecting whether landmarks are out-of-date.
- Ensure that counters are correctly incremented when scanning.
- Harmonise the way errors are thrown/reported.
Debug the extension by tracking messages and logging them in the background script.
This will need a bit of monitoring, to be sure...
Also include a workaround for the bug in Chrome 91 regarding tabs.
* Fix errors on Firefox caused by sending a message on a non-scriptable page (the badge doesn"t need setting to nothing in Firefox) * Fix errors in Firefox and Chrome caused when a tab is closed when the DevTools are open.
* Find landmarks on startup (but don't send; will be asked for them). * Fix a nesting error in GUI HTML * Count mutation-triggered scans separately to non-mutation-triggered scans. * Refactor content script so LandmarksFinder.find() is only called in one place, so that the counting can be correct.
* Count scheduled scans as caused by mutations. * Explain that in the UI. * Implement outdated landmark check properly by asking the PauseHandler if we are currently in a pause or not.
* Clarify some comments. * Rename the function that enables/disables the browserAction. * Add some FIXME notes.
* Differentiate between debugging messages and general messages that the background script is capturing and logging centrally. * Correctly classify DevTools debug messages for a given tab. * Tweak content script visibility messages. * Recognise messages coming from in-built extension pages. * Add some more FIXMEs to check. * Trap error in Firefox and Opera when there's no GUI to listen to the message that's sent when on a non-scriptable page to nullify the GUI (most likely sidebar, but popup could be open too).
* Do not scan on boot of content script; scan when starting to observe. * Remove the observation grace period (it was a nice idea but, if we believe that it is a waste to scan all tabs on startup, we have to immediately scan tabs when the user switches to them). * Tidy up the debug logging code (especially in background). * Check and update comments in background about SPAs. * Reset the PauseHandler as well as other bits when refreshing due to SPA action.
* Clarify background script logging messages.
* Content script:
- Reinstate grace period before scanning and connecting to observer.
- Increase the grace period.
- Don't set a scanner by default—wait until devtools-state message
received.
- Consider whether we are waiting to reconnect to the observer when
deciding if landmarks are out-of-date.
- If they are out of date, cancel the timer and scan now (FIXME this
is a logic bug!)
- Only find landmarks, on reflecting DevTools' state, if document is
visible.
* Rename stuff in content script for clarity. * Improve logic of detecting if out-of-date via an else-if clause. * Break up scanning from (re-)observing
Also clarify wording of debugging message.
We have to consider if we have _ever_ scanned (cannot think of a way around this).
Have tested for a while and not come across a situation in which the scanner was null when asked for a scan, but also there is no guarantee that webNavigation.onCompleted-resulting message will come after the devtools-state one (though content scripts do seem to start up before the background script notices, which seems nice). Anyway this guards against that issue by defaulting to the standard scanner (again-ish) and also removes checks for trying to change to the same scanner, as they do not seem to be needed.
matatk
added a commit
that referenced
this pull request
Nov 30, 2021
…ges (#460) PR #428 introduced a bug whereby the MutationObserver wouldn't be wired up until the page became visible (i.e. either the user moves to it for the first time, if it was loaded in a background tab, or the user moves away from and back to it, if it was loaded in the foreground tab). This was discovered and reported by @cbou in #458. Fixes #458
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.