-
Notifications
You must be signed in to change notification settings - Fork 54
Refactoring task #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kzlomek
wants to merge
8
commits into
adblockplus:master
Choose a base branch
from
kzlomek:refactoring_task
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
46488a5
FilterEngine refactoring
kzlomek 540ca04
WIP: FilterEngine and Updater JS api separation
kzlomek 5d21081
Updates in Updater class
kzlomek 7d6bdda
Code cleanup
kzlomek 49f0171
Addressed review comments
kzlomek 511c949
Fixed thread safety and added more tests
kzlomek 1ae9556
Reworked last commit applying review comments
kzlomek 956d620
Updated UpdaterAndFilterEngineCreationTest to match coding convention
kzlomek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,11 @@ | |
| #include "AppInfo.h" | ||
| #include "Scheduler.h" | ||
| #include "FilterEngine.h" | ||
| #include "Updater.h" | ||
| #include <mutex> | ||
| #include <future> | ||
| #include <set> | ||
| #include <string> | ||
|
|
||
| namespace AdblockPlus | ||
| { | ||
|
|
@@ -108,6 +111,11 @@ namespace AdblockPlus | |
| */ | ||
| FilterEngine& GetFilterEngine(); | ||
|
|
||
| /** | ||
| * Retrieves the Updater component instance. | ||
| */ | ||
| Updater& GetUpdater(); | ||
|
|
||
| typedef std::function<void(ITimer&)> WithTimerCallback; | ||
| virtual void WithTimer(const WithTimerCallback&); | ||
|
|
||
|
|
@@ -125,11 +133,15 @@ namespace AdblockPlus | |
| TimerPtr timer; | ||
| FileSystemPtr fileSystem; | ||
| WebRequestPtr webRequest; | ||
| JsEngine::EvaluateCallback evaluateCallback; | ||
|
||
| private: | ||
| // used for creation and deletion of modules. | ||
| std::mutex modulesMutex; | ||
| std::shared_ptr<JsEngine> jsEngine; | ||
| std::shared_future<FilterEnginePtr> filterEngine; | ||
| std::shared_ptr<Updater> updater; | ||
| std::set<std::string> evaluatedJsSources; | ||
| JsEngine::EvaluateCallback GetEvaluateCallback(); | ||
| }; | ||
|
|
||
| /** | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /* | ||
| * This file is part of Adblock Plus <https://adblockplus.org/>, | ||
| * Copyright (C) 2006-present eyeo GmbH | ||
| * | ||
| * Adblock Plus is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU General Public License version 3 as | ||
| * published by the Free Software Foundation. | ||
| * | ||
| * Adblock Plus is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License | ||
| * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| #ifndef ADBLOCK_PLUS_UPDATER_H | ||
| #define ADBLOCK_PLUS_UPDATER_H | ||
|
|
||
| #include <functional> | ||
| #include <string> | ||
| #include <AdblockPlus/JsEngine.h> | ||
| #include <AdblockPlus/JsValue.h> | ||
|
|
||
| namespace AdblockPlus | ||
| { | ||
| /** | ||
| * Component of libadblockplus responsible for Update checks for the application. | ||
| */ | ||
| class Updater | ||
| { | ||
| public: | ||
| explicit Updater(const JsEnginePtr& jsEngine, const JsEngine::EvaluateCallback& callback); | ||
|
|
||
| /** | ||
| * Callback type invoked when an update becomes available. | ||
| * The parameter is the download URL of the update. | ||
| */ | ||
| typedef std::function<void(const std::string&)> UpdateAvailableCallback; | ||
|
|
||
| /** | ||
| * Callback type invoked when a manually triggered update check finishes. | ||
| * The parameter is an optional error message. | ||
| */ | ||
| typedef std::function<void(const std::string&)> UpdateCheckDoneCallback; | ||
|
|
||
| /** | ||
| * Sets the callback invoked when an application update becomes available. | ||
| * @param callback Callback to invoke. | ||
| */ | ||
| void SetUpdateAvailableCallback(const UpdateAvailableCallback& callback); | ||
|
|
||
| /** | ||
| * Removes the callback invoked when an application update becomes | ||
| * available. | ||
| */ | ||
| void RemoveUpdateAvailableCallback(); | ||
|
|
||
| /** | ||
| * Forces an immediate update check. | ||
| * `Updater` will automatically check for updates in regular intervals, | ||
| * so applications should only call this when the user triggers an update | ||
| * check manually. | ||
| * @param callback Optional callback to invoke when the update check is | ||
| * finished. The string parameter will be empty when the update check | ||
| * succeeded, or contain an error message if it failed. | ||
| * Note that the callback will be invoked whether updates are | ||
| * available or not - to react to updates being available, use | ||
| * `Updater::SetUpdateAvailableCallback()`. | ||
| */ | ||
| void ForceUpdateCheck(const UpdateCheckDoneCallback& callback = UpdateCheckDoneCallback()); | ||
|
|
||
| /** | ||
| * Retrieves a preference value. | ||
| * @param pref Preference name. | ||
| * @return Preference value, or `null` if it doesn't exist. | ||
| */ | ||
| JsValue GetPref(const std::string& pref) const; | ||
|
|
||
| /** | ||
| * Sets a preference value. | ||
| * @param pref Preference name. | ||
| * @param value New value of the preference. | ||
| */ | ||
| void SetPref(const std::string& pref, const JsValue& value); | ||
|
|
||
| private: | ||
| JsEnginePtr jsEngine; | ||
| int updateCheckId; | ||
| }; | ||
| } | ||
|
|
||
| #endif |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * This file is part of Adblock Plus <https://adblockplus.org/>, | ||
| * Copyright (C) 2006-present eyeo GmbH | ||
| * | ||
| * Adblock Plus is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU General Public License version 3 as | ||
| * published by the Free Software Foundation. | ||
| * | ||
| * Adblock Plus is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License | ||
| * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| "use strict"; | ||
|
|
||
| let API_UPDATER = (() => | ||
| { | ||
| const {Services} = Cu.import("resource://gre/modules/Services.jsm", {}); | ||
|
||
| const {Prefs} = require("prefs"); | ||
| const {checkForUpdates} = require("updater"); | ||
|
|
||
| return { | ||
| getPref(pref) | ||
| { | ||
| return Prefs[pref]; | ||
| }, | ||
|
|
||
| setPref(pref, value) | ||
| { | ||
| Prefs[pref] = value; | ||
| }, | ||
|
|
||
| forceUpdateCheck(eventName) | ||
| { | ||
| checkForUpdates(eventName ? _triggerEvent.bind(null, eventName) : null); | ||
| } | ||
| }; | ||
| })(); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually unrelated to
JsEngine, it's rather an interface ofFilterEngineandUpdater, therefore IMO it would be better to define it there, thus two separatetypedefs in bothFilterEngineandUpdater.