Issue 4931 - add possibility to not send data depending on connection properties

src/FilterEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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
@@ skipping 156 matching lines @@
   };
 }
 
 FilterEngine::FilterEngine(const JsEnginePtr& jsEngine)
   : jsEngine(jsEngine), firstRun(false), updateCheckId(0)
 {
 }
 
 void FilterEngine::CreateAsync(const JsEnginePtr& jsEngine,
   const FilterEngine::OnCreatedCallback& onCreated,
-  const FilterEngine::CreateParameters& params)
+  const FilterEngine::CreationParameters& params)
 {
   FilterEnginePtr filterEngine(new FilterEngine(jsEngine));
   auto sync = std::make_shared<Sync>();
-  auto isConnectionAllowed = params.isConnectionAllowed;

[Oleksandr, 2017/03/16 14:19:53]

The naming here is highly confusing, IMO. How about isConnectionAllowedCallback?

[sergei, 2017/03/16 16:25:22]

No objections, fixed.

-  if (isConnectionAllowed)
+  auto isConnectionAllowedCallback = params.isConnectionAllowedCallback;
+  if (isConnectionAllowedCallback)
     jsEngine->SetIsConnectionAllowedCallback([sync, jsEngine]()->bool
     {
       sync->Wait();
       return jsEngine->IsConnectionAllowed();
     });
-  jsEngine->SetEventCallback("_init", [jsEngine, filterEngine, onCreated, sync, isConnectionAllowed](JsValueList& params)
+  jsEngine->SetEventCallback("_init", [jsEngine, filterEngine, onCreated, sync, isConnectionAllowedCallback](JsValueList& params)
   {
     filterEngine->firstRun = params.size() && params[0]->AsBool();
-    if (isConnectionAllowed)
+    if (isConnectionAllowedCallback)
     {
       std::weak_ptr<FilterEngine> weakFilterEngine = filterEngine;
-      jsEngine->SetIsConnectionAllowedCallback([weakFilterEngine, isConnectionAllowed]()->bool
+      jsEngine->SetIsConnectionAllowedCallback([weakFilterEngine, isConnectionAllowedCallback]()->bool
       {
         auto filterEngine = weakFilterEngine.lock();
         if (!filterEngine)
           return false;
-        return isConnectionAllowed(filterEngine->GetAllowedConnectionType().get());
+        return isConnectionAllowedCallback(filterEngine->GetAllowedConnectionType().get());
       });
     }
     sync->Set();
     onCreated(filterEngine);
     jsEngine->RemoveEventCallback("_init");
   });
 
   // Lock the JS engine while we are loading scripts, no timeouts should fire
   // until we are done.
   const JsContext context(jsEngine);
 
   // Set the preconfigured prefs
   JsValuePtr preconfiguredPrefsObject = jsEngine->NewObject();
   for (FilterEngine::Prefs::const_iterator it = params.preconfiguredPrefs.begin();
     it != params.preconfiguredPrefs.end(); it++)
   {
     preconfiguredPrefsObject->SetProperty(it->first, it->second);
   }
   jsEngine->SetGlobalProperty("_preconfiguredPrefs", preconfiguredPrefsObject);
   // Load adblockplus scripts
   for (int i = 0; !jsSources[i].empty(); i += 2)
     jsEngine->Evaluate(jsSources[i + 1], jsSources[i]);
 }
 
 FilterEnginePtr FilterEngine::Create(const JsEnginePtr& jsEngine,
-  const FilterEngine::CreateParameters& params)
+  const FilterEngine::CreationParameters& params)
 {
   FilterEnginePtr retValue;
   Sync sync;
   CreateAsync(jsEngine, [&retValue, &sync](const FilterEnginePtr& filterEngine)
   {
     retValue = filterEngine;
     sync.Set();
   }, params);
   sync.Wait();
   return retValue;
@@ skipping 202 matching lines @@
   JsValueList params;
   params.push_back(jsEngine->NewValue(pref));
   return func->Call(params);
 }
 
 void FilterEngine::SetPref(const std::string& pref, JsValuePtr value)
 {
   JsValuePtr func = jsEngine->Evaluate("API.setPref");
   JsValueList params;
   params.push_back(jsEngine->NewValue(pref));
   if (value)

[Oleksandr, 2017/03/16 14:19:53]

Nit: this doesn't look related, is it?

[sergei, 2017/03/16 16:25:22]

It's related, now we can call SetPref(nullptr) and if value is nullptr then it
crashes afterwards. nullptr here means there is no value, what is undefined in
terms of JavaScript, so we either should pass undefined or not pass the
argument. I have chosen the latter.

     params.push_back(value);
   func->Call(params);
 }
 
 std::string FilterEngine::GetHostFromURL(const std::string& url)
 {
   JsValuePtr func = jsEngine->Evaluate("API.getHostFromUrl");
   JsValueList params;
   params.push_back(jsEngine->NewValue(url));
   return func->Call(params)->AsString();
@@ skipping 125 matching lines @@
     FilterPtr filter = GetWhitelistingFilter(currentUrl, contentTypeMask, parentUrl);
     if (filter)
     {
       return filter;
     }
     currentUrl = parentUrl;
   }
   while (urlIterator != documentUrls.end());
   return FilterPtr();
 }