Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: test/filterStorage.js

Issue 29853574: Issue 6855 - Release all references to Subscription object once removed (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Remove Subscription object before emitting "subscription.removed" event Created Aug. 12, 2018, 7:12 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/filterStorage.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: test/filterStorage.js
===================================================================
--- a/test/filterStorage.js
+++ b/test/filterStorage.js
@@ -34,21 +34,28 @@
{FilterNotifier} = sandboxedRequire("../lib/filterNotifier"),
{FilterStorage} = sandboxedRequire("../lib/filterStorage"),
{Subscription} = sandboxedRequire("../lib/subscriptionClasses")
);
callback();
};
-function compareSubscriptionList(test, testMessage, list)
+function compareSubscriptionList(test, testMessage, list,
+ knownSubscriptions = null)
{
let result = FilterStorage.subscriptions.map(subscription => subscription.url);
let expected = list.map(subscription => subscription.url);
test.deepEqual(result, expected, testMessage);
+
+ if (knownSubscriptions)
+ {
+ test.deepEqual([...Subscription.knownSubscriptions.values()],
+ knownSubscriptions, testMessage);
+ }
}
function compareFiltersList(test, testMessage, list)
{
let result = FilterStorage.subscriptions.map(
subscription => subscription.filters.map(
filter => filter.text));
test.deepEqual(result, list, testMessage);
@@ -113,34 +120,44 @@
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]);
+ compareSubscriptionList(test, "Initial state", [subscription1, subscription2],
+ [subscription1, subscription2]);
test.deepEqual(changes, [], "Received changes");
changes = [];
FilterStorage.removeSubscription(subscription1);
- compareSubscriptionList(test, "Removing first subscription", [subscription2]);
+ compareSubscriptionList(test, "Removing first subscription", [subscription2],
+ [subscription2]);
test.deepEqual(changes, ["subscription.removed http://test1/"], "Received changes");
changes = [];
FilterStorage.removeSubscription(subscription1);
- compareSubscriptionList(test, "Removing already removed subscription", [subscription2]);
+ compareSubscriptionList(test, "Removing already removed subscription", [subscription2],
+ [subscription2]);
test.deepEqual(changes, [], "Received changes");
changes = [];
FilterStorage.removeSubscription(subscription2);
- compareSubscriptionList(test, "Removing remaining subscription", []);
+ 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]);
changes = [];
FilterStorage.removeSubscription(subscription1);
compareSubscriptionList(test, "Re-removing previously added subscription", []);
test.deepEqual(changes, ["subscription.removed http://test1/"], "Received changes");
@@ -461,16 +478,22 @@
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]);
« no previous file with comments | « lib/filterStorage.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld