Issue 6855 - Release all references to Subscription object once removed

test/filterStorage.js

 /*
  * 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
@@ skipping 45 matching lines @@
 function compareFiltersList(test, testMessage, list)
 {
   let result = FilterStorage.subscriptions.map(
     subscription => subscription.filters.map(
       filter => filter.text));
   test.deepEqual(result, list, testMessage);
 }
 
 function compareFilterSubscriptions(test, testMessage, filter, list)
 {
-  let result = filter.subscriptions.map(subscription => subscription.url);
+  let result = [...filter.subscriptions].map(subscription => subscription.url);
   let expected = list.map(subscription => subscription.url);
   test.deepEqual(result, expected, testMessage);
 }
 
 exports.testAddingSubscriptions = function(test)
 {
   let subscription1 = Subscription.fromURL("http://test1/");
   let subscription2 = Subscription.fromURL("http://test2/");
 
   let changes = [];
@@ skipping 30 matching lines @@
   compareSubscriptionList(test, "Re-adding previously removed subscription", [subscription2, subscription1]);
   test.deepEqual(changes, ["subscription.added http://test1/"], "Received changes");
 
   test.done();
 };
 
 exports.testRemovingSubscriptions = function(test)
 {
   let subscription1 = Subscription.fromURL("http://test1/");
   let subscription2 = Subscription.fromURL("http://test2/");
+
+  test.equal(Subscription.fromURL(subscription1.url), subscription1,
+             "Subscription known before addition");
+
   FilterStorage.addSubscription(subscription1);
   FilterStorage.addSubscription(subscription2);
 
   let changes = [];
   function listener(action, subscription)
   {
     if (action.indexOf("subscription.") == 0)
       changes.push(action + " " + subscription.url);
   }
   FilterNotifier.addListener(listener);
 
   compareSubscriptionList(test, "Initial state", [subscription1, subscription2],
                           [subscription1, subscription2]);
   test.deepEqual(changes, [], "Received changes");
+
+  test.equal(Subscription.fromURL(subscription1.url), subscription1,
+             "Subscription known after addition");
 
   changes = [];
   FilterStorage.removeSubscription(subscription1);
   compareSubscriptionList(test, "Removing first subscription", [subscription2],
                           [subscription2]);
   test.deepEqual(changes, ["subscription.removed http://test1/"], "Received changes");
 
+  // Once a subscription has been removed, it is forgotten; a new object is
+  // created for the previously known subscription URL.
+  test.notEqual(Subscription.fromURL(subscription1.url), subscription1,
+                "Subscription forgotten upon removal");
+  Subscription.knownSubscriptions.delete(subscription1.url);
+
   changes = [];
   FilterStorage.removeSubscription(subscription1);
   compareSubscriptionList(test, "Removing already removed subscription", [subscription2],
                           [subscription2]);
   test.deepEqual(changes, [], "Received changes");
 
   changes = [];
   FilterStorage.removeSubscription(subscription2);
   compareSubscriptionList(test, "Removing remaining subscription", [], []);
   test.deepEqual(changes, ["subscription.removed http://test2/"], "Received changes");
 
-  // Compare references to make sure that a new object is created for the same
-  // subscription URL.
-  test.notEqual(Subscription.fromURL(subscription2.url), subscription2,
-                "Subscription forgotten");
-
-  // Adding a removed Subscription object back still works but is not
-  // recommended.
-  FilterStorage.addSubscription(subscription1);
-  compareSubscriptionList(test, "Add", [subscription1]);
+  FilterStorage.addSubscription(subscription1);
+  compareSubscriptionList(test, "Add", [subscription1], []);
 
   changes = [];
   FilterStorage.removeSubscription(subscription1);
-  compareSubscriptionList(test, "Re-removing previously added subscription", []);
+  compareSubscriptionList(test, "Re-removing previously added subscription", [], []);
   test.deepEqual(changes, ["subscription.removed http://test1/"], "Received changes");
 
   test.done();
 };
 
 exports.testMovingSubscriptions = function(test)
 {
   let subscription1 = Subscription.fromURL("http://test1/");
   let subscription2 = Subscription.fromURL("http://test2/");
   let subscription3 = Subscription.fromURL("http://test3/");
@@ skipping 304 matching lines @@
   compareFilterSubscriptions(test, "filter1 subscriptions after adding http://test1/", filter1, [subscription1]);
   compareFilterSubscriptions(test, "filter2 subscriptions after adding http://test1/", filter2, [subscription1]);
   compareFilterSubscriptions(test, "filter3 subscriptions after adding http://test1/", filter3, []);
 
   FilterStorage.addSubscription(subscription2);
 
   compareFilterSubscriptions(test, "filter1 subscriptions after adding http://test2/", filter1, [subscription1]);
   compareFilterSubscriptions(test, "filter2 subscriptions after adding http://test2/", filter2, [subscription1, subscription2]);
   compareFilterSubscriptions(test, "filter3 subscriptions after adding http://test2/", filter3, [subscription2]);
 
-  // Test duplicate subscription entries, which can occur when a subscription
-  // is loaded from disk and contains the same filter more than once.
-  filter1.subscriptions.push(subscription1);
-  compareFilterSubscriptions(test, "filter1 subscriptions after duplicating http://test1/",
-                             filter1, [subscription1, subscription1]);
-
   FilterStorage.removeSubscription(subscription1);
 
   compareFilterSubscriptions(test, "filter1 subscriptions after removing http://test1/", filter1, []);
   compareFilterSubscriptions(test, "filter2 subscriptions after removing http://test1/", filter2, [subscription2]);
   compareFilterSubscriptions(test, "filter3 subscriptions after removing http://test1/", filter3, [subscription2]);
 
   FilterStorage.updateSubscriptionFilters(subscription3, [filter3]);
 
   compareFilterSubscriptions(test, "filter1 subscriptions after updating http://test3/ filters", filter1, []);
   compareFilterSubscriptions(test, "filter2 subscriptions after updating http://test3/ filters", filter2, [subscription2]);
@@ skipping 12 matching lines @@
   compareFilterSubscriptions(test, "filter3 subscriptions after updating http://test3/ filters", filter3, [subscription2]);
 
   FilterStorage.removeSubscription(subscription3);
 
   compareFilterSubscriptions(test, "filter1 subscriptions after removing http://test3/", filter1, []);
   compareFilterSubscriptions(test, "filter2 subscriptions after removing http://test3/", filter2, [subscription2]);
   compareFilterSubscriptions(test, "filter3 subscriptions after removing http://test3/", filter3, [subscription2]);
 
   test.done();
 };