Utils.yield() is a footgun

[trev]

Environment

Adblock Plus 2.7.2.4168
Firefox 46

How to reproduce

1. Install BetterPrivacy 1.69 from ​https://addons.mozilla.org//addon/betterprivacy/
2. Restart Firefox (might need to restart multiple times in order to reproduce the issue).
3. Open Adblock Plus preferences.

Observed behaviour

Adblock Plus filters aren't loaded.

Expected behaviour

Adblock Plus filters should be loaded.

Background

BetterPrivacy 1.69 is broken in Firefox 46, one of their JavaScript files produces a syntax error (outdated syntax, no longer supported). We call Utils.yield() occasionally while reading patterns.ini, this way we prevent hanging Firefox UI. However, this function will currently spin its own event loop. If we are out of luck, an event processed in our event loop will load BetterPrivacy's JavaScript file and error out. The error is then propagated all the way down to our code which assumes that loading patterns.ini produced an error and falls back to the backup.

What to change

IO.readFromFile() is already async, the progress listener is being called in a synchronous fashion however. One option would be evaluating listener result: yield listener(line). This allows the listener to return a promise that will be resolved asynchronously:

return new Promise((resolve, reject) => Utils.runAsync(resolve));

This approach doesn't have any problematic side-effects, our code is no longer on stack when the error happens.

Hints for testers

The dependency update here also imported the changes from #4000 which could potentially impact CSS property filters. However, without any of the adjustments from #3969 the behavior simply stays unchanged.

[trev]

Moving to Adblock Plus for Firefox module, the Core part here is tiny.

[abpbot]

A commit referencing this issue has landed:
​https://hg.adblockplus.org/adblockpluscore/rev/3f1bdb2a4fd4

[abpbot]

A commit referencing this issue has landed:
​https://hg.adblockplus.org/adblockplus/rev/3d6cba98fba5