Issue 1319,5450,1285 - FilterEngine and Updater JS api separation

Moved Updater class to dedicated header and source files. Created apiUpdater.js with a separate API for Updater component. Updated unit tests. Signed-off-by: Krystian Zlomek [email protected]
adblockplus · Oct 9, 2018 · 0a3f088 · 0a3f088
1 parent 9fb9625
commit 0a3f088
Show file tree

Hide file tree

Showing 14 changed files with 539 additions and 109 deletions.
diff --git a/include/AdblockPlus.h b/include/AdblockPlus.h
@@ -32,6 +32,7 @@
 
 #include <AdblockPlus/AppInfo.h>
 #include <AdblockPlus/FilterEngine.h>
+#include <AdblockPlus/Updater.h>
 #include <AdblockPlus/LogSystem.h>
 #include <AdblockPlus/JsEngine.h>
 #include <AdblockPlus/JsValue.h>

diff --git a/include/AdblockPlus/FilterEngine.h b/include/AdblockPlus/FilterEngine.h
@@ -187,7 +187,6 @@ namespace AdblockPlus
    * It handles:
    * - Filter management and matching.
    * - Subscription management and synchronization.
-   * - Update checks for the application.
    */
   class FilterEngine
   {
@@ -225,18 +224,6 @@ namespace AdblockPlus
      */
     typedef int32_t ContentTypeMask;
 
-    /**
-     * 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;
-
     /**
      * Callback type invoked when the filters change.
      * The first parameter is the action event code (see
@@ -287,6 +274,12 @@ namespace AdblockPlus
      */
     typedef std::function<void(const FilterEnginePtr&)> OnCreatedCallback;
 
+    /**
+     * Callback type for evaluating JS expression.
+     * The parameter is the JS file name containing the expression.
+     */
+    typedef std::function<void(const std::string&)> EvaluateCallback;
+
     /**
      * Asynchronously constructs FilterEngine.
      * @param jsEngine `JsEngine` instance used to run JavaScript code
@@ -296,6 +289,7 @@ namespace AdblockPlus
      * @param parameters optional creation parameters.
      */
     static void CreateAsync(const JsEnginePtr& jsEngine,
+      const EvaluateCallback& evaluateCallback,
       const OnCreatedCallback& onCreated,
       const CreationParameters& parameters = CreationParameters());
 
@@ -476,32 +470,6 @@ namespace AdblockPlus
      */
     std::string GetHostFromURL(const std::string& url) const;
 
-    /**
-     * 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.
-     * `FilterEngine` 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
-     *        `FilterEngine::SetUpdateAvailableCallback()`.
-     */
-    void ForceUpdateCheck(const UpdateCheckDoneCallback& callback = UpdateCheckDoneCallback());
-
     /**
      * Sets the callback invoked when the filters change.
      * @param callback Callback to invoke.
@@ -558,7 +526,6 @@ namespace AdblockPlus
   private:
     JsEnginePtr jsEngine;
     bool firstRun;
-    int updateCheckId;
     static const std::map<ContentType, std::string> contentTypes;
 
     explicit FilterEngine(const JsEnginePtr& jsEngine);

diff --git a/include/AdblockPlus/Platform.h b/include/AdblockPlus/Platform.h
@@ -25,8 +25,12 @@
 #include "AppInfo.h"
 #include "Scheduler.h"
 #include "FilterEngine.h"
+#include "Updater.h"
 #include <mutex>
 #include <future>
+#include <set>
+#include <string>
+#include <functional>
 
 namespace AdblockPlus
 {
@@ -108,6 +112,11 @@ namespace AdblockPlus
      */
     FilterEngine& GetFilterEngine();
 
+    /**
+     * Retrieves the Updater component instance.
+     */
+    Updater& GetUpdater();
+
     typedef std::function<void(ITimer&)> WithTimerCallback;
     virtual void WithTimer(const WithTimerCallback&);
 
@@ -130,6 +139,10 @@ namespace AdblockPlus
     std::mutex modulesMutex;
     std::shared_ptr<JsEngine> jsEngine;
     std::shared_future<FilterEnginePtr> filterEngine;
+    std::shared_ptr<Updater> updater;
+    std::set<std::string> evaluatedJsSources;
+    std::mutex evaluatedJsSourcesMutex;
+    std::function<void(const std::string&)> GetEvaluateCallback();
   };
 
   /**

diff --git a/include/AdblockPlus/Updater.h b/include/AdblockPlus/Updater.h
@@ -0,0 +1,100 @@
+/*
+ * 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:
+    /**
+     * 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();
+
+    /**
+     * Callback type for evaluating JS expression.
+     * The parameter is the JS file name containing the expression.
+     */
+    typedef std::function<void(const std::string&)> EvaluateCallback;
+
+    /**
+     * 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);
+
+    explicit Updater(const JsEnginePtr& jsEngine, const EvaluateCallback& callback);
+
+  private:
+    JsEnginePtr jsEngine;
+    int updateCheckId;
+  };
+}
+
+#endif
diff --git a/lib/api.js b/lib/api.js
@@ -28,7 +28,6 @@ let API = (() =>
   const {ElemHide} = require("elemHide");
   const {Synchronizer} = require("synchronizer");
   const {Prefs} = require("prefs");
-  const {checkForUpdates} = require("updater");
   const {Notification} = require("notification");
 
   return {
@@ -186,6 +185,7 @@ let API = (() =>
     {
       Notification.markAsShown(id);
     },
+
     checkFilterMatch(url, contentTypeMask, documentUrl)
     {
       let requestHost = extractHostFromURL(url);
@@ -211,11 +211,6 @@ let API = (() =>
       Prefs[pref] = value;
     },
 
-    forceUpdateCheck(eventName)
-    {
-      checkForUpdates(eventName ? _triggerEvent.bind(null, eventName) : null);
-    },
-
     getHostFromUrl(url)
     {
       return extractHostFromURL(url);

diff --git a/lib/apiUpdater.js b/lib/apiUpdater.js
@@ -0,0 +1,41 @@
+/*
+ * 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 {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);
+    }
+  };
+})();
diff --git a/lib/compat.js b/lib/compat.js
@@ -337,6 +337,16 @@ XMLHttpRequest.prototype =
       for (let i = 0; i < list.length; i++)
         list[i].call(this, event);
     };
+
+    // #1319: Now FilterEngine and Updater are separated, so we want to
+    // allow update requests no matter if subscriptions download requests
+    // are allowed or not.
+    if (this._url.includes("update.json"))
+    {
+      window._webRequest.GET(this._url, this._requestHeaders, onGetDone);
+      return;
+    }
+
     // HACK (#5066): the code checking whether the connection is
     // allowed is temporary, the actual check should be in the core
     // when we make a decision whether to update a subscription with

diff --git a/lib/prefs.js b/lib/prefs.js
@@ -146,10 +146,11 @@ let Prefs = exports.Prefs = {
   }
 };
 
-// Update the default prefs with what was preconfigured
-for (let key in _preconfiguredPrefs)
-  if (preconfigurable.indexOf(key) != -1)
-    defaults[key] = _preconfiguredPrefs[key];
+if (typeof _preconfiguredPrefs !== "undefined")
+  // Update the default prefs with what was preconfigured
+  for (let key in _preconfiguredPrefs)
+    if (preconfigurable.indexOf(key) != -1)
+      defaults[key] = _preconfiguredPrefs[key];
 
 // Define defaults
 for (let key in defaults)

diff --git a/libadblockplus.gyp b/libadblockplus.gyp
@@ -44,6 +44,7 @@
       'src/DefaultWebRequest.cpp',
       'src/FileSystemJsObject.cpp',
       'src/FilterEngine.cpp',
+      'src/Updater.cpp',
       'src/GlobalJsObject.cpp',
       'src/JsContext.cpp',
       'src/JsEngine.cpp',
@@ -160,6 +161,7 @@
         ],
         'load_after_files': [
           'lib/api.js',
+          'lib/apiUpdater.js',
           'lib/publicSuffixList.js',
           'lib/punycode.js',
           'lib/basedomain.js',