Issue 5182 - fix IsConnectionAllowed

test/FilterEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 178 matching lines @@
         strings.clear();
       }
     private:
       mutable std::mutex mutex;
       std::vector<std::pair<bool, std::string>> strings;
     };
   protected:
     MockWebRequest* webRequest;
     std::string subscriptionUrlPrefix;
     FilterEngine::CreationParameters createParams;
-    SyncStrings capturedConnectionTypes;
-    bool isConnectionAllowed;
+    // HACK: it's a shared pointer to keep it available in
+    // isConnectionAllowedCallback after destroying of the test.
+    struct SharedData
+    {
+      SyncStrings capturedConnectionTypes;
+      bool isConnectionAllowed;
+      struct
+      {
+        std::string url;
+        std::mutex mutex;
+        std::condition_variable cv;
+      } downloadStatusChanged;
+    };
+    std::shared_ptr<SharedData> data;
     FilterEnginePtr filterEngine;
 
-    struct
-    {
-      std::string url;
-      std::mutex mutex;
-      std::condition_variable cv;
-    } downloadStatusChanged;
-
     void SetUp()
     {
+      data = std::make_shared<SharedData>();
       BaseJsTest::SetUp();
       jsEngine->SetFileSystem(AdblockPlus::FileSystemPtr(new LazyFileSystem()));
       jsEngine->SetWebRequest(AdblockPlus::WebRequestPtr(webRequest = new MockWebRequest()));
       jsEngine->SetLogSystem(AdblockPlus::LogSystemPtr(new LazyLogSystem()));
 
       subscriptionUrlPrefix = "http://example";
       ServerResponse exampleSubscriptionResponse;
       exampleSubscriptionResponse.responseStatus = 200;
       exampleSubscriptionResponse.status = WebRequest::NS_OK;
       exampleSubscriptionResponse.responseText = "[Adblock Plus 2.0]\n||example.com";
       webRequest->responses.emplace(subscriptionUrlPrefix, exampleSubscriptionResponse);
       createParams.preconfiguredPrefs.emplace("first_run_subscription_auto_select", jsEngine->NewValue(false));
-      isConnectionAllowed = true;
-      createParams.isConnectionAllowedCallback = [this](const std::string* allowedConnectionType)->bool{
-        capturedConnectionTypes.Add(allowedConnectionType);
-        return isConnectionAllowed;
+      data->isConnectionAllowed = true;
+      auto closure = data;
+      createParams.isConnectionAllowedCallback = [closure](const std::string* allowedConnectionType)->bool{
+        closure->capturedConnectionTypes.Add(allowedConnectionType);
+        return closure->isConnectionAllowed;
       };
-      jsEngine->SetEventCallback("filterChange", [this](JsValueList&& params/*action, item*/)
-      {
-        ASSERT_EQ(2u, params.size());
-        if (params[0].AsString() == "subscription.downloadStatus")
-        {
-          {
-            std::lock_guard<std::mutex> lock(downloadStatusChanged.mutex);
-            downloadStatusChanged.url = params[1].GetProperty("url").AsString();
-          }
-          downloadStatusChanged.cv.notify_one();
-        }
-      });
     }
 
     Subscription EnsureExampleSubscriptionAndForceUpdate(const std::string& apppendToUrl = std::string())
     {
       if (!filterEngine)
+      {
         filterEngine = FilterEngine::Create(jsEngine, createParams);
+        auto closure = data;
+        filterEngine->SetFilterChangeCallback([closure](const std::string& action, JsValue&& item)
+        {
+          if (action == "subscription.downloadStatus")
+          {
+            {
+              std::lock_guard<std::mutex> lock(closure->downloadStatusChanged.mutex);
+              closure->downloadStatusChanged.url = item.GetProperty("url").AsString();
+            }
+            closure->downloadStatusChanged.cv.notify_one();
+          }
+        });
+      }
       auto subscriptionUrl = subscriptionUrlPrefix + apppendToUrl;
       auto subscription = filterEngine->GetSubscription(subscriptionUrl);
       EXPECT_EQ(0u, subscription.GetProperty("filters").AsList().size()) << subscriptionUrl;
       EXPECT_TRUE(subscription.GetProperty("downloadStatus").IsNull()) << subscriptionUrl;
       subscription.UpdateFilters();
       {
-        std::unique_lock<std::mutex> lock(downloadStatusChanged.mutex);
-        downloadStatusChanged.cv.wait_for(lock,
-          /*don't block tests forever*/std::chrono::seconds(5),
+        std::unique_lock<std::mutex> lock(data->downloadStatusChanged.mutex);
+        data->downloadStatusChanged.cv.wait_for(lock,
+          /*don't block tests forever*/std::chrono::seconds(300),
           [this, subscriptionUrl]()->bool
         {
-          return subscriptionUrl == downloadStatusChanged.url;
+          return subscriptionUrl == data->downloadStatusChanged.url;
         });
         // Basically it's enough to wait only for downloadStatus although there
         // is still some JS code being executed. Any following attempt to work
         // with subscription object will result in execution of JS, which will
         // be blocked until finishing of currently running code.
       }
       return subscription;
     }
   };
 }
@@ skipping 780 matching lines @@
   EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
   EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, AllowingCallbackAllowsUpdating)
 {
   // no stored allowed_connection_type preference
   auto subscription = EnsureExampleSubscriptionAndForceUpdate();
   EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
   EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
-  auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
+  auto capturedConnectionTypes = data->capturedConnectionTypes.GetStrings();
   ASSERT_EQ(1u, capturedConnectionTypes.size());
   EXPECT_FALSE(capturedConnectionTypes[0].first);
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, NotAllowingCallbackDoesNotAllowUpdating)
 {
-  isConnectionAllowed = false;
+  data->isConnectionAllowed = false;
   // no stored allowed_connection_type preference
   auto subscription = EnsureExampleSubscriptionAndForceUpdate();
   EXPECT_EQ("synchronize_connection_error", subscription.GetProperty("downloadStatus").AsString());
   EXPECT_EQ(0u, subscription.GetProperty("filters").AsList().size());
-  auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
+  auto capturedConnectionTypes = data->capturedConnectionTypes.GetStrings();
   EXPECT_EQ(1u, capturedConnectionTypes.size());
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, PredefinedAllowedConnectionTypeIsPassedToCallback)
 {
   std::string predefinedAllowedConnectionType = "non-metered";
   createParams.preconfiguredPrefs.insert(std::make_pair("allowed_connection_type",
     jsEngine->NewValue(predefinedAllowedConnectionType)));
   auto subscription = EnsureExampleSubscriptionAndForceUpdate();
   EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
   EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
-  auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
+  auto capturedConnectionTypes = data->capturedConnectionTypes.GetStrings();
   ASSERT_EQ(1u, capturedConnectionTypes.size());
   EXPECT_TRUE(capturedConnectionTypes[0].first);
   EXPECT_EQ(predefinedAllowedConnectionType, capturedConnectionTypes[0].second);
 }
 
 TEST_F(FilterEngineIsAllowedConnectionTest, ConfiguredConnectionTypeIsPassedToCallback)
 {
   // FilterEngine->RemoveSubscription is not usable here because subscriptions
   // are cached internally by URL. So, different URLs are used in diffirent
   // checks.
   {
     std::string predefinedAllowedConnectionType = "non-metered";
     createParams.preconfiguredPrefs.insert(std::make_pair(
       "allowed_connection_type", jsEngine->NewValue(predefinedAllowedConnectionType)));
     auto subscription = EnsureExampleSubscriptionAndForceUpdate();
     EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
     EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
-    auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
+    auto capturedConnectionTypes = data->capturedConnectionTypes.GetStrings();
     ASSERT_EQ(1u, capturedConnectionTypes.size());
     EXPECT_TRUE(capturedConnectionTypes[0].first);
     EXPECT_EQ(predefinedAllowedConnectionType, capturedConnectionTypes[0].second);
   }
-  capturedConnectionTypes.Clear();
+  data->capturedConnectionTypes.Clear();
   {
     // set no value
     filterEngine->SetAllowedConnectionType(nullptr);
     auto subscription = EnsureExampleSubscriptionAndForceUpdate("subA");
     EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
     EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
-    auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
+    auto capturedConnectionTypes = data->capturedConnectionTypes.GetStrings();
     ASSERT_EQ(1u, capturedConnectionTypes.size());
     EXPECT_FALSE(capturedConnectionTypes[0].first);
     subscription.RemoveFromList();
-    this->capturedConnectionTypes.Clear();
+    data->capturedConnectionTypes.Clear();
   }
-  capturedConnectionTypes.Clear();
+  data->capturedConnectionTypes.Clear();
   {
     // set some value
     std::string testConnection = "test connection";
     filterEngine->SetAllowedConnectionType(&testConnection);
     auto subscription = EnsureExampleSubscriptionAndForceUpdate("subB");
     EXPECT_EQ("synchronize_ok", subscription.GetProperty("downloadStatus").AsString());
     EXPECT_EQ(1u, subscription.GetProperty("filters").AsList().size());
-    auto capturedConnectionTypes = this->capturedConnectionTypes.GetStrings();
+    auto capturedConnectionTypes = data->capturedConnectionTypes.GetStrings();
     ASSERT_EQ(1u, capturedConnectionTypes.size());
     EXPECT_TRUE(capturedConnectionTypes[0].first);
     EXPECT_EQ(testConnection, capturedConnectionTypes[0].second);
   }
 }