Rob--W / dont-track-me-google

Firefox and Chrome extensions to prevent Google from making links ugly.
MIT License
459 stars 26 forks source link

Degraded performances on console.cloud.google.com #52

Closed Delgan closed 1 year ago

Delgan commented 1 year ago

Hi.

I'm using this extension since a few weeks now and it's really good, I wonder why I didn't install it sooner. Thanks for publishing and maintaining it!

However, I observed degraded performances on https://console.cloud.google.com with Firefox displaying a warning "this extension is slowing down your browser".

See the difference when the extension is disabled:

https://github.com/Rob--W/dont-track-me-google/assets/4193924/cab8d292-ac21-4729-86e6-10b25fceb056

Compared to when it's enabled:

https://github.com/Rob--W/dont-track-me-google/assets/4193924/48f21bb6-ae42-4016-834c-944ba1160b0b

It seems that this doesn't happen systematically, but it's still quite common.

Do you think it's possible to add an option to disable the extension on some sites, based on a regex for example?

I'm using Firefox v114.0 with v4.27 of the "Don't track me Google" add-on.

Rob--W commented 1 year ago

The extension is not supposed to be slowing down anything. Adding an option to disable it on some sites is a work-around, but I would prefer to resolve the root cause.

I have yet to investigate in more detail, but it seems that the referrer hiding logic is triggered very often in the given test case.

I'll look into this more and work towards a fix. Thanks for your report!

Rob--W commented 1 year ago

I cannot conclusively attribute slowness to the extension. Could you try to capture a profile with and without the extension, using the tool from https://profiler.firefox.com?

When I capture a profile myself, I see that the site is experiencing a performance issue regardless of whether the extension is enabled. Interacting with the sidebar triggers many mousemove events (an event that is quite chatty on its own) along with relatively expensive operations in the sidebar.

Delgan commented 1 year ago

Hi @Rob--W, thanks for looking into it.

It took me about 5 minutes of browsing and refreshing the page for the problem to appear. It's not just the sidebar actually, the whole site is slow, starting with the initial page load.

I managed to capture the problem in a Firefox profiling session, as you suggested. However, it potentially contains data that I do not wish to share publicly. Can I contact you via the e-mail address in your Github profile?

It seems that in some cases, infinite recursion occurs.

Screenshot 2023-06-18 at 12-59-29 Firefox 114 – Linux – 6_18_2023 10 02 12 AM UTC – Firefox Profiler console(1) slowdown

Rob--W commented 1 year ago

I managed to capture the problem in a Firefox profiling session, as you suggested. However, it potentially contains data that I do not wish to share publicly. Can I contact you via the e-mail address in your Github profile?

Yes.

It seems that in some cases, infinite recursion occurs.

I'm able to extract the following stack trace from your screenshot

As the stack trace is from a sampling profiler, the call stack may be incomplete. If you are able to reproduce consistently and get a meaningful error, can you share the stack trace from the console? E.g. when you observe InternalError: too much recursion, can you expand the error in the console and share the full stack trace of the original error (i.e. the cause of InternalError, not th code that logged the error ultimately).

Note: the pretty-printed version of n6a.prototype.attachTemplatePortal referenced above is this:

    n6a.prototype.attachTemplatePortal = function(a) {
        var b = this,
            c = a.viewContainerRef,
            d = c.createEmbeddedView(a.templateRef, a.context, {
                injector: a.injector
            });    
        d.rootNodes.forEach(function(e) {
            return b.outletElement.appendChild(e)
        });    
        d.detectChanges();
        this.setDisposeFn(function() {
            var e = c.indexOf(d); - 1 !== e && c.remove(e)
        });    
        this._attachedPortal = a;
        return d
    };
Delgan commented 1 year ago

I sent you an e-mail. :+1:

Rob--W commented 1 year ago

I have found the cause of the issue.

The extension tries to intercept events on links by assigning proto.dispatchEvent, where proto is HTMLAnchorElement.prototype, at https://github.com/Rob--W/dont-track-me-google/blob/5c2593b40768340c1f4ed4844a2a2fe6a15bfea2/contentscript.js#L274-L278

Ordinarily, proto.dispatchEvent is a value inherit from EventTarget.prototype.dispatchEvent. The intent of assigning to proto.dispatchEvent is to replace the dispatchEvent method of <a> elements.

However, a script on console.cloud.google.com changes the contract of the API: Element.prototype.dispatchEvent is replaced with a getter and setter. Consequently, upon assigning to proto.dispatchEvent, the page redirects that assignment to EventTarget.prototype, and consequently the <a>-specific logic applies to any subclass of EventTarget, not just HTMLAnchorElement.

This triggers recursion because the updateReferrerPolicy method also relies on dispatchEvent, at: https://github.com/Rob--W/dont-track-me-google/blob/5c2593b40768340c1f4ed4844a2a2fe6a15bfea2/contentscript.js#L286-L293

I'm going to add a commit to account for this behavior.

Rob--W commented 1 year ago

I have pushed the fixes to the repository, but haven't published the extension yet.

Do you want to test the changes before I publish an update?

Delgan commented 1 year ago

Great, glad you figured it out and solved the problem so quickly. Thanks!

I've tested the master branch locally and so far the bug hasn't reappeared.

Rob--W commented 1 year ago

Thanks for your confirmation (and also the initial report and details!). I have published version 4.28 that includes the fix.