squgeim
commented
5 years ago I don't think the current implementation has any perceivable performance impact. It can be made slightly better by using a setTimeout to schedule the next check, instead of setInterval; but going all the way to MutationObservers and monitoring XHR requests will be overkill.
To make sure we notice when the skip ad button is shown, we'd need to set up the mutation observable to observe a node that might be quite high in the hierarchy from the button (it needs to exist whether there's an ad or not). This means our callback will be called each time anything changes below that DOM node (even attribute change in a deep element). Having to handle all the false positives might be more work (both in writing the code and performance-wise) than the simple poling we have right now.
The current implementation also has the added benefit of being very simple to reason through.
“Premature optimization is the root of all evil” - Donald Knuth
It sure sounds like a fun exercise though. Thanks!
I am not sure if either of those ideas is feasible but those came to mind:
Using a MutationObserver to check when the button is added (this probably depends on how many other UI updates are happening on Youtube at a time; if there aren't that many, this might work out).
Manipulating XHR to see where requests are going. Then if one goes to where the video ads come from, check for the button 5 or 6 seconds after it completes, if it is still not there, continue looking for it in 500ms intervals. When the button is found, clear the interval so it's not trying to find buttons for 10 minutes every 2 seconds.
Then again, it's probably debatable whether that is required at all or if the 2 second polling is working out well. I could imagine though that with things like whitelisting or other checks going on in the function, it might be worth a try to see if one can reduce function calls.