Add functionality enabling AA subscription into libadblockplus

[sergz]

Background

This issue arose during discussion of #4604.

Why in libadblockplus and not in core

I think that adblockpluscore is a generic core without any presumable subscriptions, that's why subscription URLs and the adding logic are duplicated in each extension and in libadblockplus. Next, libadblockplus chooses and adds the default language subscription and it has URL of acceptable ads subscription in its pref.js, so it seems legit to add AA subscription in libadblockplus on the first run when a default subscription corresponding to the current locale is being added.

Why

It should prevent the users of libadblockplus from implementing the same functionality each time and solves some known issues, like possible race conditions for instance.

How

However, I would like to have it optional, disabled by default and enabled through a constructor argument of FilterEngine to avoid affecting of tests. Currently they are already at certain degree affected by default language subscription, though.

What to change

• add adding of AA subscriptions in init.js where we choose and add default subscription
• add conditions which enable mentioned above functionality in accordance with corresponding settings
• define what settings should be passed through preconfiguredPrefs argument of FilterEngine constructor and define default values in prefs.js
• check which tests can be simplified and make required adjustments of all affected tests.

[fhd]

Yeah, makes sense to have this in libadblockplus rather than duplicate the behaviour in all clients.

But while at it, I actually think we should deal with the automatic language subscription selection, too, in the same way. But I think the default should be automatic subscription selection, if people want different behaviour they can change it, and we can change it in the tests where we need to.

And also, while at it: It would be handy to have API for enabling/disabling AA, too.

[sergz]

Sorry, it was unclear. That's actually what I meant, beside AA stuff, to have a property which disables both functionalities by default to avoid any side-effects in tests. But maybe you are right, for users it would be better to have everything enabled by default.

Adding API to manage AA seems to be a bit more complicated, maybe another issue?

• FilterEngine::SetAAEnabled(bool)
• FilterEngine::IsAAEnabled()
• Subscription::IsAA() it should allow to find AA subscription among items returned by FilterEngine::GetListedSubscriptions
• FilterEngine::GetAAURL() it seems not an ideal way but already better than to ask user of libadblockplus to read hard-coded property subscriptions_exceptionsurl.

[fhd]

Yeah, can be a separate issue. But this is one of the things we keep duplicating that bothers me :P (I doubt we have a use case for GetAAURL, I think all we use that URL for is to check if AA is enabled etc, which would be covered by the new API functions.)

[sergz]

If AA is not in the list of subscriptions then there is no way to show to user the URL and make a link to AA on an options page.

[fhd]

Hm, true I guess. Then we might not need IsAA though, but both wouldn't hurt.

[abpbot]

A commit referencing this issue has landed:
​Issue 4612 - enable AA on first run and make automatic adding of any subscription optional