Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(363)

Issue 29435650: Issue 5182, 4688 - improve IsSubscriptionDowloadAllowedCallback and corresponding tests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months, 1 week ago by sergei
Modified:
7 months, 1 week ago
Reviewers:
hub
CC:
Felix Dahlke
Base URL:
https://github.com/adblockplus/libadblockplus.git
Visibility:
Public.

Description

- remove LazyWebRequest (issue 4688) and fix tests, the rest is for issue 5182 - make make FilterEngine::IsConnectionAllowedCallback asynchronous (it's also renamed) FilterEngine does not use detached thread for it anymore, now it's a responsibility of a user of libadblockplus. - rename member of FilterEngine::CreationParameters isConnectionAllowedCallback to isSubscriptionDowloadAllowedCallback - rename FilterEngineIsAllowedConnectionTest to FilterEngineIsSubscriptionDownloadAllowedTest and make it less hacky. # it seems too much, let me know if you think it should be split into commits. # based on https://codereview.adblockplus.org/29435645/

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -135 lines) Patch
M include/AdblockPlus/FilterEngine.h View 2 chunks +8 lines, -7 lines 0 comments Download
M src/FilterEngine.cpp View 3 chunks +5 lines, -9 lines 0 comments Download
M test/BaseJsTest.h View 1 chunk +0 lines, -11 lines 0 comments Download
M test/FilterEngine.cpp View 5 chunks +57 lines, -108 lines 3 comments Download

Messages

Total messages: 3
sergei
https://codereview.adblockplus.org/29435650/diff/29435651/test/FilterEngine.cpp File test/FilterEngine.cpp (left): https://codereview.adblockplus.org/29435650/diff/29435651/test/FilterEngine.cpp#oldcode178 test/FilterEngine.cpp:178: class SyncStrings since there are no threads there is ...
7 months, 1 week ago (2017-05-10 17:04:40 UTC) #1
sergei
7 months, 1 week ago (2017-05-10 17:05:00 UTC) #2
hub
7 months, 1 week ago (2017-05-10 18:53:53 UTC) #3
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5