Skip to content
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

Network filtering not working in latest Firefox Nightly (webext) #2684

Closed
HopefullyNotTaken opened this issue Jun 6, 2017 · 37 comments
Closed

Comments

@HopefullyNotTaken
Copy link

Describe the issue

With the latest Firefox Nightly, network filters are no longer being applied. Cosmetic filtering, however, appears to be working just fine.

Steps for anyone to reproduce the issue

  1. Download the latest version of Firefox Nightly.

  2. Create a new profile.

  3. Install any version of uBlock Origin. Make sure it's the webext.

  4. Go to any site and create a rule to block a resource using the network filter ( || ).

  5. Open the Network Monitor on said site and reload. Notice how the resource still shows up in the Network Monitor and visually despite there being a rule in uBlock Origin to block it.

Your settings

Tried on a clean profile so all settings are at default.

  • OS/version: Arch Linux
  • Browser/version: 55.0a1 (2017-06-05) (64-bit)
  • uBlock Origin version: 1.12.5rc1 but tried older versions as well ( 1.12.5rc0, 1.12.4, 1.12.2, 1.12.1, 1.12.0, 1.11.4, 1.11.2, 1.11.0, 1.10.6)
Your filter lists

Default.

Your custom filters (if any)

None.

@gorhill
Copy link
Owner

gorhill commented Jun 6, 2017

latest Firefox Nightly

Why report the issue here? If using latest Nightly causes uBO to stop working, then in all likelihood isn't the issue with Nightly?

@gorhill
Copy link
Owner

gorhill commented Jun 6, 2017

If I comment out the code adding ws://*/* and wss://*/* to onBeforeRequest.addListener, it seems to "fix" the issue.

This use to cause a warning at the console at most without causing uBO to stop functioning. So something changed in Nightly to cause uBO's onBeforeRequest listener to be completely ignored if declaring ws;/wss: URL patterns.

Related: https://bugzilla.mozilla.org/show_bug.cgi?id=1367478.

@gorhill
Copy link
Owner

gorhill commented Jun 6, 2017

For the time being, I'm going to stop building the webext version of uBO, there are too many serious issues now with it. I will resume when things stabilize, it's unclear whether it is expected things will have to change in uBO or whether things will be fixed in webext.

@shellye5
Copy link

shellye5 commented Jun 6, 2017

Thank you for the explanation and quick workaround :) @gorhill

@gorhill
Copy link
Owner

gorhill commented Jun 6, 2017

I just recevied a Nightly update which appears to solve the issue reported here. However the issue with uBO's auxiliary pages is not solved, and this quite cripples the webext version of uBO, so for now the webext version won't be released anymore until things improve, as I just do not have the time to deal with all reports and there is no quick fix. Whoever really want the webext version badly despite the problems can just git clone and build it themselves.

@andymckay
Copy link

Is there an issue for the auxiliary pages? Sorry, not sure what that is.

@gorhill
Copy link
Owner

gorhill commented Jun 6, 2017

Problem which appeared lately, discussed here: https://bugzilla.mozilla.org/show_bug.cgi?id=1368152#c15.

I use "auxiliary pages" to refer to the dashboard, logger principally, but also others such as the warning when a document is blocked, advanced settings, asset viewer, or even the popup panel-in-a-tab. With legacy version, these would be like:

  • chrome://ublock0/content/dashboard.html
  • chrome://ublock0/content/logger-ui.html
  • chrome://ublock0/content/advanced-settings.html
  • chrome://ublock0/content/asset-viewer.html?url=easyprivacy

Etc.

@andymckay
Copy link

Thanks.

@gorhill
Copy link
Owner

gorhill commented Jun 24, 2017

I brought back the hybrid-webext build, since it works as expected with Firefox 54.

@shellye5
Copy link

shellye5 commented Jun 25, 2017

webext version still uses high disk , and slows down startup
seems like this bug is affecting it too.
@gorhill can you look into it? because the performance is low

https://bugzilla.mozilla.org/show_bug.cgi?id=1371255

extensions.webextensions.remote
layers.popups.compositing.enabled

Breaks webext sometimes(does not block ad's and restarting fixes it)

@gorhill
Copy link
Owner

gorhill commented Jul 8, 2017

@andymckay Any thoughts on whether I should just go ahead and push the webext-hybrid version of uBO to the dev channel on AMO?

@andymckay
Copy link

Sorry, I'm not sure what your requirements are for that and I'm a bit biased on this point, so you probably wouldn't want to take my advice.

@shellye5
Copy link

shellye5 commented Aug 27, 2017

Just a PSA to stop pushing beta update 1.13.11rc0.webext to users as it might break their settings

[test setup=32bit lin/win10 Asus X552EA-SX009D Laptop (APU Dual Core- 2GB ram) with FF57]

the webext in beta 1.13.11rc0 on AMO
breaks filters/settings if user has a custom list added

ERRORS:

Failed to serialize browser.storage key "cache/https://www.kiboke-studio.hr/i-dont-care-about-cookies/abp/": TypeError: JSON.serialize is not a function ExtensionStorage.jsm:60

Failed to serialize browser.storage key "cache/disconnect-malware": TypeError: JSON.serialize is not a function ExtensionStorage.jsm:60

settings and storage files

https://1drv.ms/f/s!AuafaQl91miScMPu2qlVOpMJ8yc

Prefer storage file as it is easily reproducible

Import the attached settings, update lists and check storage size(will be in MBs)
now restart and
just add any filter list and restart -purge-update and storage size is 1kb and settings/lists broken,
delete the added filter apply and works again but with restart before deleting the added filter causes
the same problem again(Basically cannot use other filters )

try adding

https://www.kiboke-studio.hr/i-dont-care-about-cookies/abp/

or any other

https://github.com/collinbarrett/FilterLists

The NON-WEBEXT works fine in Palemoon/ESR

@gorhill can you please add ADGuard's Filter-lists(english/privacy/annoyance/etc) as many of US prefer theirs over more popular ones please.

@gorhill
Copy link
Owner

gorhill commented Aug 27, 2017

That error is not uBO's code, report to bugzilla please.

@shellye5
Copy link

@gorhill against what should the bug be filed?

So till it's fired the webext uBo will be broken?

@gorhill
Copy link
Owner

gorhill commented Aug 27, 2017

It's not broken for me. The error:

TypeError: JSON.serialize is not a function ExtensionStorage.jsm:60

Is Firefox's code. I don't get this error, using Nightly 57.0a1 (2017-08-26) (64-bit)/Linux.

Edit: Nightly updated to 57.0a1 (2017-08-27), and all still work fine.

@shellye5
Copy link

shellye5 commented Aug 27, 2017

@gorhill I mentioned that using 32bit builds will test in today's build but can only do in windows10 machine today,

@gorhill
Copy link
Owner

gorhill commented Aug 27, 2017

What is easier to test is for you to un-check all the following lists:

  • EasyList
  • EasyList without element hiding rules
  • EasyPrivacy
  • Fanboy’s Enhanced Tracking List
  • Fanboy’s Annoyance List
  • Fanboy’s Social Blocking List

These are all parts of Fanboy Ultimate, no point enabling them when you have Fanboy Ultimate enabled. Even if uBO discards duplicates, it still has to parse/load to find out what is duplicate, and loading all these redundants lists is not cheap CPU- and memory-wise. See if this makes your issue go away.

@shellye5
Copy link

@gorhill okay doing a clean install again and just setting up with a few, what else can be helpful to you for debug?

@gorhill
Copy link
Owner

gorhill commented Aug 27, 2017

No need to clean install, I just wanted to see if un-checking the above lists solved your issue. If yes, this would have confirmed your issue is related to storage size.

@shellye5
Copy link

shellye5 commented Aug 27, 2017

@gorhill
Sorry missed you comment .

DID a clean install and found out 3 more things in windows 10 (linux 32bit a bit later).

1> This happens only on 32builds of Firefox even with default filter, just when the total size of filters or storage reaches around 30-35 mb,
uBo or Firefox reverts storage size to 1kb(on () is present in it)
RAM/Disk/CPU usage goes as high as 700-1100mb during this process(have 4gb as page file too) so memory limitation should not be a problem
but it looks that way? no other 32 bit apps have memory issues if page file is high enough but here looks like Firefox/uBo api is the problem ? That's why no issues in 64bit version
& this issue should not be happening!
please file a bug as IDK about this.

2> compared to legacy version using with Palemoon & Firefox ESR52 this issue never happens,
just for testing went with storage size upto 60-65 mb and no issues.

3> In WebEXT version there is high disk/cpu/ram usage too especially when updating filters

(SQlite in legacy was using chunks/append ? )

because data is added in KBs eg total size 5m->new update list->add 500KB update to DB->new size=5.5mb,

but in WEBEXT stores all in ram then writes to disk every time so for eg:

30mb->uncompressed to ram->write full uncompressed data to disk->
new size 40mb->uncompressed to ram->write full uncompressed data to disk->
new size 50mb.

This is very inefficient don't you think?
Updating filters in background hanks the UI and you can see that in ram/disk/usage under task manager, it goes as high as 1.8gb(increased page file to25gb just to test and no other apps have have memory issues but here it is)
& this is with the default lists + ignore cookies list.

Edit: Tested with chrome 60 32bit with huge filter-list, even with 70mb in size and no issues with it.

@gorhill
Copy link
Owner

gorhill commented Aug 27, 2017

I reported your observations here: https://bugzilla.mozilla.org/show_bug.cgi?id=1371255.

@shellye5
Copy link

shellye5 commented Aug 27, 2017

Thank you ! @gorhill Hoping it gets fixed before FF57 lands any idea about priority @andymckay .

@gorhill
Copy link
Owner

gorhill commented Aug 28, 2017

I added new comments to https://bugzilla.mozilla.org/show_bug.cgi?id=1371255.

Bottom line is that it's quite a serious issue with Firefox 55, and I don't see any trivial workaround for uBO.

@shellye5
Copy link

shellye5 commented Aug 28, 2017

@gorhill

https://bugzilla.mozilla.org/show_bug.cgi?id=1371255#c11

If you want resource efficiency, use IndexedDB. storage.local is not designed for storing huge amounts of data.

like you said

The issue here is concerning for uBlock Origin, since there is a webext (embedded) version (1.13.10) on the verge of approval on AMO, which means the reported issue here is likely to start to affect a lot of users once it becomes available on AMO.

@gorhill
Copy link
Owner

gorhill commented Aug 28, 2017

He is not recommending to use sqlite API, that is a legacy API which use will not be allowed in FF57.

He is recommending to use indexedDB. This will have to be the fix, but as said it's not trivial, and it's the 1st time I ever used this API, so I still have to learn how it works.

It quite bothers me to learn just now from bugzilla that one shouldn't use browser.storage.local for storing "huge amounts of data" -- nowhere was this ever mentioned, and there is no such issue with Chromium -- there was no reason for me to believe this was going to be an issue. I suggested that the limitation should be officially documented.

I will be working to migrate the cache storage to indexedDB. Just keep in mind I am not a paid dev working full time on this project, so I don't know when I will have something ready.

@shellye5
Copy link

shellye5 commented Aug 28, 2017

@gorhill thank you

@gorhill
Copy link
Owner

gorhill commented Aug 28, 2017

Looking at the files generated by Firefox when using indexedDB, I can confirm that the API uses sqlite behind the scene.

@gorhill
Copy link
Owner

gorhill commented Aug 28, 2017

I just recall @nikrolls using indexedDB to address a storage issue with Edge (size limitation). If I can reuse this that should help a lot to get something working sooner: https://github.com/nikrolls/uBlock-Edge/blob/master/platform/edge/vapi-background.js#L70.

@andymckay
Copy link

We want to get the API in Firefox faster and we'll probably do so by using an indexedDB backend at some point. The amount of data and frequency of data storage is something that surprised us. For example the Chrome docs do say how its "not a big truck". But the simple fact is people are storing a large amount of data there and quite regularly, so we've got to get it faster.

I don't think we'll be able to do that in time for Firefox 57, there's a lot happening and its a risky patch for Firefox.

I'm curious what % of your users you think this might affect. For AdBlock Plus we found it was an edge case where people had a large amount (say 60MB) of filters set up. Wondering if its similar.

@shellye5
Copy link

shellye5 commented Aug 28, 2017

@gorhill So It's ublock0.sqlite that will be used in dev builds?
so basically it's the legacy version but using NEW API ?

Mozilla should have documented it.

So the performance should be same as legacy version?

@andymckay

I'm curious what % of your users you think this might affect. For AdBlock Plus we found it was an edge case where people had a large amount (say 60MB) of filters set up. Wondering if its similar.

For ABP that was not an exception the size increased drastically when it shifted to storage.js,
and it still was not 60mb just single file containing 34 MB (compressed SQliteDB & uncompressed .js it's 80mb) of data with just 2 filters(which is an edge case as users have more) which is not a lot by any means.

as long as the data merely stays in memory the memory usage stays below 400 MB (that's the total memory usage of the Firefox process, not merely Adblock Plus)

This part is important too as the filters were not causing a lot of overheads even with the limited resources but reading(loading) & writing(storing) were causing the issues,

here was using a way smaller file 15mb but could see the hit in performance as soon as webextention version was installed , plus when migrated ublock0.sqlite to storage.js it was like 30mb((~12mb in SQlite.db & that's how hit the bug) so guessing that the number will be in most cases not an EDGE case and will keep on increasing as filter-lists add more and more.

IMHO the size is not the matter but the performance, let the user have 100mb of data,
It should not affect the performance so drastically just like legacy version handles 60-100mb SQlite DB file without any problems when was testing it earlier to see how it performs & currently testing legacy version with ~100mb SQlite DB without any type of performance hit, whereas 33mb(can go higher as it errors out and data is lost & it reverts to 1kb) of storage.js(~12mb in SQlite.db) causes hangs and janks so it's not the size but performance.

And why does 32bit builds fails to write the data of ABP and uBO ?
OOM should not occur if page file is present and no other apps exhibit that behavior

I don't think we'll be able to do that in time for Firefox 57, there's a lot happening and its a risky patch for Firefox.

wait so this

Looking at the files generated by Firefox when using indexedDB, I can confirm that the API uses sqlite behind the scene.

is not possible ?

by using an indexedDB backend

sqlite DB is used in Legacy so why not implement it in the back-end?
It performs well even with huge sizes just like places.sqlite .

Does indexedDB provide a better performance than sqlite DB or are both the same thing?
i.e. the API uses sqlite behind the scene.

EDIT:

we'll probably do so by using an indexedDB backend at some point.

@andymckay why wait as it is a severe improvement(if too risky put it behind a preference so we can test), afaik Firefox uses it too so should not be a problem theoretically
& like @gorhill said IndexDB uses sqlite behind the scene so what did you mean by that?

Sorry to bug you all so much and not trying to be a witch just interested in how things tick but also this is my way of contributing and making things better

@gorhill
Copy link
Owner

gorhill commented Aug 29, 2017

@shellye5 You may want to give a try to 1.13.11rc1, see if this fixes your issue.

@shellye5
Copy link

shellye5 commented Aug 30, 2017

@gorhill thanks and yes going to test it now!

Can you elaborated on other things asked just before your last comments? please

https://bugzilla.mozilla.org/show_bug.cgi?id=1313401 will affect this 1.13.11rc1?

@shellye5
Copy link

shellye5 commented Aug 30, 2017

@gorhill Did a quick test and found a few things which you need to know about.**

**1.13.11rc1 takes a while for migration,which is understandable **

@andymckay can you shed light on this?

but the filesize is double of storage.js!! _let alone the size of legacy version which was fairly small.

eg say 12mb in legacy- 30mb in storage.js- 85mb in new indexedDB
(isn't it supposed to be the same as legacy SQ3.db? so why not 12mb & 85mb instead?),

extension-data is used as storage in legacy & has only one file(which is compressed)
webextensions used browser-extension-data\uBlock Storage.js uncompressed file.

Now settings are stored in -browser-extension-data\uBlock Storage.js

& filters in storage\default\moz-extension+++6a4e2ef8-e974-4cda-8cb9-e703d0900d68
which is temporary location ?
instead of more permanent \storage\permanent\moz-extension+++6a4e2ef8-e974-4cda-8cb9-e703d0900d68 but both of which are which are not as good as
\extension-data\ or
\browser\extension-data\uBo\

But it's not ideal so can they be a single file just like legacy version in extension-data? It's better and simpler, easier to backup(local backup & sharing to multiple pc w/o downloading again).

When starting FF uBo takes a bit more time to initialize and open dashboard is slow and uses high CPU.
can you look into these_

Rest looks good in the short testing, will test more thoroughly and let you know.

@shellye5
Copy link

shellye5 commented Aug 30, 2017

@gorhill there seems to be a huge regression in page loading times in webext 1.13.11rc1,

Just open 10-12 bookmarks look at the loading times and now switch tabs..
Seems 1.13.11rc1 is delaying or causing a delay in performance, will test it in FF56 so verify.

EDIT: CPU usage skyrockets with even one filter ,
RAM/Disk usage has gone down significantly but still not as good as legacy.
Maybe moving to single file in \browser-storage-data\ublock.DB will improve
because legacy uses that and has to read/write to a single file so cpu usage is low?

@shellye5
Copy link

shellye5 commented Aug 30, 2017

@gorhill Cpu 100% and Disk usage at startup halts the browser for good 30-60 seconds,
Filters start getting updated at random startups even if you update them manually.

Page loading times have gone up significantly even with just loading one page and opening multiple pages at once makes it painfully slow with again 100% CPU usage and high disk usage.

Just to see open multiple reddit sub reddits at once and start typing in the comment box here,
the pages take a long time to load completely and typing in comments box here lags and once the page loading is complete ever thing is speedy.

eg reproduced in these

https://www.reddit.com/r/aww/
https://www.reddit.com/r/pictures/
https://www.reddit.com/r/lolcats/

or just open 10 bookmarks in background scroll this page, keep looking at the loading time of pages while you scroll up & edit any one of the above comments to see the issue.

Now open these with the 1.13.11rc0 version to see the speed difference

This with just 1 filter(easylist)
It's even worse when have more than 1 filters.

Is it because you used the EDGE code here to fix this?
Which might not be optimized for Firefox use?
beb7933

Seems like even worse than 1.13.11rc0 in term of CPU at startup and loading pages &
DISK usage while loading pages but it fixed filters not being saved which is fixed but the Filters size is huge and scattered instead of just one ublock0.sqlite file like in legacy
(which is significantly smaller in size so even better performance).

Please consider reverting this change so beta users keep getting FEATURES+FIXES,
Consider providing debug builds here so we can test it and when the performance matches that of legacy then switch to the
webext version storage back-end(preferably \extension-data\ublock0.sqlite single file for all data).
Don't want to be selfish and make other beta users suffer :(

@gorhill
Copy link
Owner

gorhill commented Aug 30, 2017

Please, bring your issue to Mozilla, there is nothing I can do, uBO is merely using indexedDB, just as I was told to do by Firefox dev.

Locking this issue because it's better to bring your specific issue to Firefox devs -- I've done what I could here.

Repository owner locked and limited conversation to collaborators Aug 30, 2017
Repository owner unlocked this conversation Aug 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants