Opened 3 years ago

Last modified 2 months ago

#5182 new change

Fix IsAllowedConnection — at Version 2

Reported by: sergz Assignee:
Priority: P2 Milestone:
Module: Libadblockplus Keywords:
Cc: fhd Blocked By: #5066, #5179, #5198
Blocking: #3595, #5180 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29424786/
https://codereview.adblockplus.org/29435650/

Description

Background

IsAllowedConnection had been implemented in a hurry and we had cut some corners, now it's time to fix it, especially because it affects a threading model.

What to change

The issue is pretty comprehensive

  • rename it
    This method is applicable only for subscriptions download, so it should not be called in a so generic manner.
  • remove it from JsEngine and from WebRequestThread
    Since it's only about subscriptions, it has no relation to JS engine, it should be on the level of FilterEngine, while initializing of FilterEngine we should inject a corresponding callback into JS global object and call it from JS, later it will be called by adblockpluscore #5066.
  • make it asynchronous
    In order to avoid blocking of execution of JS code (see above, it's called from JS and not from a worker thread) the asynchronicity should be either implemented in FilterEngine or supported by a provider of the callback, in the latter case the callback should be async. It seems so far, in order to simplify the life of users of libadblockplus it would be better to have it in FilterEngine.
    Until asynchronous executor is not ready one may just run it a detached thread.

Change History (2)

comment:1 Changed 3 years ago by sergz

  • Blocked By 5198 added

I have removed it from JsEngine and from WebRequestThread and made the execution of it asynchronous in FilterEngine, though it is still in a detached thread (added blocked by #5198).

The codereview will be after landing of https://codereview.adblockplus.org/29424663/, because otherwise tests don't work anymore, and after https://codereview.adblockplus.org/29424659/ because it uses that code.

Last edited 3 years ago by sergz (previous) (diff)

comment:2 Changed 3 years ago by sergz

  • Review URL(s) modified (diff)
Note: See TracTickets for help on using tickets.