Move pseudo-user stylesheets out of contentscript.js

[gorhill]

Firefox webext and legacy supports real user stylesheets, so these versions should not suffer whatever overhead incurred by any of the code necessary for pseudo-user stylesheets required for browsers which do not support real user stylesheets (Chromium, Edge, Safari).

[gorhill]

As of writing, with default filter lists:

• Low generic filters (class- or id-based): 18,175
• High generic filters (everything else): 427
• Ratio of high generic filters / all generic filters = 2.3%. This is typical.

For instance, even with Fanboy Annoyance:

• Low generic filters (class- or id-based): 29,382
• High generic filters (everything else): 885
• Ratio of high generic filters / all generic filters = 2.9%.

Given this, I will no longer make uBO distinguish between flavors of high generic filters, they will be all treated by the same code path. This will simplify the fix required here, which I want to get out before Firefox 57 stable is released.

[gwarser]

www.wp.pl###home-page-button, #newsfeed-player-column

from https://raw.githubusercontent.com/azet12/PopupBlocker/master/PPB.txt

is reported as www.wp.pl##home-page-button, #newsfeed-player-column - (missing #)
in logger. Can be tested here https://www.wp.pl/ (for the time being)

I mean - in details dialog. On list this is ###home-page-button, #newsfeed-player-column

[gwarser]

Here https://www.wp.pl/ when scrolling on 1.14.16 I see ads collapsing. On 6112a68 I see them loading.

Default filters + POL

No change in 4f7aab6

[gorhill]

Do you know which specific filter is supposed to be at work there?

[gwarser]

I think:

www.wp.pl##SECTION#section_celebrities > DIV[class]:first-child
www.wp.pl##SECTION#section_mototech > DIV[class]:first-child
www.wp.pl##SECTION#section_sport > DIV[class]:first-child
www.wp.pl##SECTION#section_finances > DIV[class]:first-child
www.wp.pl##SECTION#section_lifestyle > DIV[class]:first-child

I see them in logger, but they are not applied.

[kasper93]

I can reproduce, with some of the ads on this page. For me ads flickers, basically they are hidden, after some delay they are shown again and hidden again after delay. Easily viable when scrolling.

EDIT: I need to test with older release tho

[gorhill]

www.wp.pl##SECTION#section_celebrities > DIV[class]:first-child

Those filters assume the ads will be in the first div child. I see differently on my side. I suspect the site has javascript which shuffles the ads to a different position if they are not visible. If I remove :first-child, the specific ad in your pic is properly hidden, no flickering (because the site attempt to shuffle them to a different position does not work -- all the child div are hidden).

[gorhill]

Ok I think I understand what is going on. The POL maintainer(s) created this filter:

www.wp.pl##SECTION[class^="_3"] > DIV[hidden] + DIV[class]

Of course, the web site does not create the hidden attribute on their element, this was something added by uBO 1.14.16 and less. uBO post 1.14.16 no longer uses the hidden attribute internally, as there is no point to do so with real user stylesheet. So the filter depends completely on a specific uBO implementation which was never designed to be used by filter list maintainers. They need to fix their filters to stop depending on the hidden attribute unless it is added by the site itself.

Edit the filter to www.wp.pl##SECTION[class^="_3"] > DIV + DIV[class] and it works, without the flickering (uBO was adding hidden after a delay).

[gwarser]

They need to fix their filters to stop depending on the hidden attribute

Reported MajkiIT/polish-ads-filter#4080

[kasper93]

@gorhill Filter issue is fixed in upstream. wp.pl is one of the biggest polish website (top 7 according to Alexa). Would you please take a look at performance on this site? They update DOM heavily on scroll. The side bar is updated. I captured two profiles:

https://perfht.ml/2xXzWdZ 1.14.16
forEachNode takes 240-300ms

https://perfht.ml/2xYevsZ noroundtrip (e1acf6d)
domFilterer.prototype.commitNow takes around 1,103ms, I got 2.5 seconds per call once, I think without noroundtrip patch.

To make this profile:

1. Go to the site, wait till everything loads.
2. Scroll the page, not so fast to let the right sidebar keep up with loading.

My configuration is the same as we discussed in the other topic, screen for reference.

Let me know if you need more info. I think this is great page to benchmark, because it has a lot of advertisements, a lot of dynamic content and is one of the top websites in Poland. So a lot of people (uBO users) hit this page everyday.

EDIT:
According to gemius.pl (top analytic platform in Poland) wp.pl is 4-th in Poland with 20 238 250 users and 3 242 651 514 hits in September 2017.

[gorhill]

https://perfht.ml/2xYevsZ noroundtrip (e1acf6d)
domFilterer.prototype.commitNow takes around 1,103ms, I got 2.5 seconds per call once, I think without noroundtrip patch.

It's the matches-css filter. I didn't touch procedural selectors, so you should get same performance issue with 1.14.16. To implement matches-css, uBO needs to call getComputedStyle, and it seems this is what is causing the issue. I see servo in the profile data, maybe worth to try without servo.

[gorhill]

Just to add, specifically I see a matches-css with the width property. It is known that querying for the computed width will cause the browser to force a recalculation of the layout of the page. This is expensive.

[kasper93]

Indeed it spends time in PSelectorMatchesCSSTask.prototype.exec in both profiles. Could it be that injecting user css causes matches-css to be more expensive? You can see profile with 1.14.16 it is a lot faster.

I will ask list maintainers to try remove matches-css filter if possible. Or make them more specific... Currently there are two filters, that are just evil.

www.wp.pl##div[class]:matches-css(width: 309px):matches-css(height: 182px)
www.wp.pl##div[class]:matches-css(max-width: 300px):matches-css(max-height: 250px)

[gorhill]

I don't see the profile in 1.14.16 to be a lot faster. Processing matches-css requires almost 20s in 1.14.16, and 30s in 1.14.17. They are both very expensive, I suspect the difference is merely down to luck: force-recalculation of the layout maybe fall at the best or worst time for the rendering engine, I suspect you would get quite varying results from one profile to another. On my side I got 6s and 10s with/without servo (can't remember which one is which).

There is no way uBO's injected styles are involved, the site's own CSS is quite certainly magnitude larger than what uBO adds. The site itself contributes to its demise by shuffling around DOM elements while the page loads, this probably contributes to the whole issue of matches-css being expensive.

[kasper93]

Thanks for your time. I will resolve this with list maintainers as there is clearly an improvement to be done in filters.

[gorhill]

I tried to create an exception filter for that matches-css filter to see the difference but it's not working. I still have much code review to do following the refactor.

[gorhill]

So I fixed my exception filter issue, and yes, removing the matches-css(width: ...) filter makes a huge difference. The other matches-css(max-width: ...) is not nearly as bad -- I don't think max-width force a layout recalculation. There is doc somewhere on the net about which style properties causes layout recalculation.

[gorhill]

As part of the refactoring I will add code to stop processing the procedural filters which are too expensive.

[gorhill]

@gwarser I see the site wp.pl is using a web worker, I am wondering if they are using this to bypass blockers (see behind-the-scene requests in logger). A filter such as ||wp.pl^$csp=worker-src 'none' stops the web worker, but I can't tell if this breaks anything. This won't work on Firefox though.

[gwarser]

I don't think they need workers. They "encrypt" all links server-side, so all looks the same (base64 encoded random(?) bits).
They use worker for web notifications.