First run page is not displayed in Edge after all filter lists have been removed

[Ross]

Environment

ABP 0.9.11.2206 (Devbuild)
Edge 42.17134.1.0 / EdgeHTML 17.17134

IS a regression compared to 0.9.11 (Release)

How to reproduce

1. Select [ABP Icon] > [Cog] > [Advanced].
2. Delete/remove all filter lists.
3. Restart the browser.

Observed behaviour

The user is resubscribed to an EasyList subscription as expected, however the first run page is not displayed with the message explaining what has occured (as happens in Chrome/Firefox).

Expected behaviour

The first run page to be displayed on restart when the user is resubscribed to EasyList because all subsriptions were missing/removed.

Notes

• This is a regression since the 0.9.11 release.

[Ross]

After double checking, it turns out this is a regression because the previous/current release (0.9.11) does redisplay the first run page with the warning.

Not sure if it's worth delaying the release for though?

[kzar]

Not sure if it's worth delaying the release for though?

I'm not sure, let's see what Sebastian thinks. But in the mean time I'll take a look and see if I can figure out what's going on here.

[kzar]

I'm still waiting for my VM to update (yay Windows 10...), but I wonder if this could be a side effect of the change for #6775?

[sebastian]

Replying to Ross:

Not sure if it's worth delaying the release for though?

If this is a regression, we should address it before the release.

Replying to kzar:

but I wonder if this could be a side effect of the change for #6775?

I first thought the same, but we only restore filter data from the backup when the file doesn't exist in IndexedDB anymore, but after removing all filter lists the file should still exist at least.

[kzar]

I finally got my Windows 10 VM to cooperate. I can reproduce the problem, investigating now.

[kzar]

I can reproduce this problem perhaps half the time.

Like Sebastian says, the getBackupData() function shouldn't be called at all, since patterns.ini exists. But it seems that after realising that patterns.ini is empty, the filterStorage code attempts to read patterns-backup1.ini, and that in turn triggers the getBackupData() to be called, I guess since that file doesn't exist.

When the first run page is displayed correctly, the getBackupData() function is returning {type: "NoSuchFile"}, but otherwise it's returning the file's contents - which contains the subscriptions, even though I removed them.

Order of events is something like this:

statFile patterns.ini
getFile patterns.ini Result [object Object]
statFile returning...
getFile patterns.ini Result [object Object]
Error: No data in the file
getFile patterns-backup1.ini Result undefined
getBackupData Result {object containing the subscriptions}
First run? false

I'm not sure why the getBackupData() function sometimes returns the subscriptions and sometimes returns "No such file". Perhaps it's when there was a backup made before I removed the subscriptions, but not afterwards? I suppose that timing aspect might explain why I can't reproduce this every time.

I'll continue investigating tomorrow, any suggestions welcome.

[sebastian]

The backup is deferred by 60 seconds. So if Microsoft Edge is closed within 60 seconds after the subscriptions have been removed, the backup available on next browser start is from before removing the subscriptions. But if you wait for the backup to complete before terminating Microsoft Edge, the backup will be empty as well. Furthermore, there is only one backup in browser.storage.local that is returned for any file that failed to load from IndexedDB.

[kzar]

Gotya thanks, that makes sense.

So if I understand this all right, we can simply skip restoring from our IndexedDB backup if the failed read is for one of these patterns-backupX.ini files. I've opened a review to do that now.

[abpbot]

A commit referencing this issue has landed:
​Issue 7182 - Avoid restoring IndexedDB backup when patterns.ini is empty

[kzar]

(I don't see a Adblock-Plus-For-Edge-next milestone, so hopefully this milestone is OK.)

[kzar]

Seems my last commit broke the IndexedDB unit tests, looking it that now.

[abpbot]

A commit referencing this issue has landed:
​Issue 7182 - Fix the Edge IndexedDB unit tests

[Ross]

In the build below, if I removed the lists and close immediately close Edge. On restart the first run page is displayed with the message. If I wait a while (guessing more than 60 seconds) the first run page is not displayed and no lists are added. Not sure if I need to wait for 2221?

ABP 0.9.11.2216 (Devbuild)
Edge 44.17763.1.0 / EdgeHTML 18.17763

[geo]

I've checked the current build, this fix is not included,so we have to wait for the new build to be published.

[kzar]

So I installed the devbuild this morning, the store gave me 0.9.11.2223. I tested removing all the subscriptions, and then restarting Edge both immediately and after a few minutes. Everything worked as expected, the first run page always showed up with the message about data corruption.

[Ross]

Fixed. Works for me too both immediately and after a few minutes.

ABP 0.9.11.2223 (Devbuild)
Edge 44.17763.1.0 / EdgeHTML 18.17763