Index: lib/filterListener.js =================================================================== --- a/lib/filterListener.js +++ b/lib/filterListener.js @@ -103,17 +103,16 @@ filterNotifier.on("filter.removed", onFilterRemoved); filterNotifier.on("filter.disabled", onFilterDisabled); filterNotifier.on("filter.moved", onGenericChange); filterNotifier.on("subscription.added", onSubscriptionAdded); filterNotifier.on("subscription.removed", onSubscriptionRemoved); filterNotifier.on("subscription.disabled", onSubscriptionDisabled); filterNotifier.on("subscription.updated", onSubscriptionUpdated); - filterNotifier.on("subscription.moved", onGenericChange); filterNotifier.on("subscription.title", onGenericChange); filterNotifier.on("subscription.fixedTitle", onGenericChange); filterNotifier.on("subscription.homepage", onGenericChange); filterNotifier.on("subscription.downloadStatus", onGenericChange); filterNotifier.on("subscription.lastCheck", onGenericChange); filterNotifier.on("subscription.errors", onGenericChange); filterNotifier.on("load", onLoad); @@ -332,17 +331,17 @@ { isDirty = 0; defaultMatcher.clear(); ElemHide.clear(); ElemHideEmulation.clear(); ElemHideExceptions.clear(); Snippets.clear(); - for (let subscription of FilterStorage.subscriptions) + for (let subscription of FilterStorage.subscriptions()) { if (!subscription.disabled) addFilters(subscription.filters); } } function onSave() { Index: lib/filterStorage.js =================================================================== --- a/lib/filterStorage.js +++ b/lib/filterStorage.js @@ -75,37 +75,49 @@ /** * Map of properties listed in the filter storage file before the sections * start. Right now this should be only the format version. */ fileProperties: Object.create(null), /** - * List of filter subscriptions containing all filters - * @type {Subscription[]} + * Yields subscriptions containing all filters + * @yields {Subscription} */ - subscriptions: [], + *subscriptions() + { + yield* this.knownSubscriptions.values(); + }, + + /** + * Number of known subscriptions. + * @type {number} + */ + get subscriptionCount() + { + return this.knownSubscriptions.size; + }, /** * Map of subscriptions already on the list, by their URL/identifier * @type {Map.} */ knownSubscriptions: new Map(), /** * Finds the filter group that a filter should be added to by default. Will * return null if this group doesn't exist yet. * @param {Filter} filter * @return {?SpecialSubscription} */ getGroupForFilter(filter) { let generalSubscription = null; - for (let subscription of FilterStorage.subscriptions) + for (let subscription of FilterStorage.knownSubscriptions.values()) { if (subscription instanceof SpecialSubscription && !subscription.disabled) { // Always prefer specialized subscriptions if (subscription.isDefaultFor(filter)) return subscription; // If this is a general subscription - store it as fallback @@ -123,75 +135,40 @@ * Adds a filter subscription to the list * @param {Subscription} subscription filter subscription to be added */ addSubscription(subscription) { if (FilterStorage.knownSubscriptions.has(subscription.url)) return; - FilterStorage.subscriptions.push(subscription); FilterStorage.knownSubscriptions.set(subscription.url, subscription); addSubscriptionFilters(subscription); filterNotifier.emit("subscription.added", subscription); }, /** * Removes a filter subscription from the list * @param {Subscription} subscription filter subscription to be removed */ removeSubscription(subscription) { - for (let i = 0; i < FilterStorage.subscriptions.length; i++) - { - if (FilterStorage.subscriptions[i].url == subscription.url) - { - removeSubscriptionFilters(subscription); - - FilterStorage.subscriptions.splice(i--, 1); - FilterStorage.knownSubscriptions.delete(subscription.url); - - // This should be the last remaining reference to the Subscription - // object. - Subscription.knownSubscriptions.delete(subscription.url); - - filterNotifier.emit("subscription.removed", subscription); - return; - } - } - }, - - /** - * Moves a subscription in the list to a new position. - * @param {Subscription} subscription filter subscription to be moved - * @param {Subscription} [insertBefore] filter subscription to insert before - * (if omitted the subscription will be put at the end of the list) - */ - moveSubscription(subscription, insertBefore) - { - let currentPos = FilterStorage.subscriptions.indexOf(subscription); - if (currentPos < 0) + if (!FilterStorage.knownSubscriptions.has(subscription.url)) return; - let newPos = -1; - if (insertBefore) - newPos = FilterStorage.subscriptions.indexOf(insertBefore); + removeSubscriptionFilters(subscription); - if (newPos < 0) - newPos = FilterStorage.subscriptions.length; + FilterStorage.knownSubscriptions.delete(subscription.url); - if (currentPos < newPos) - newPos--; - if (currentPos == newPos) - return; + // This should be the last remaining reference to the Subscription + // object. + Subscription.knownSubscriptions.delete(subscription.url); - FilterStorage.subscriptions.splice(currentPos, 1); - FilterStorage.subscriptions.splice(newPos, 0, subscription); - filterNotifier.emit("subscription.moved", subscription); + filterNotifier.emit("subscription.removed", subscription); }, /** * Replaces the list of filters in a subscription by a new list * @param {Subscription} subscription filter subscription to be updated * @param {Filter[]} filters new filter list */ updateSubscriptionFilters(subscription, filters) @@ -368,17 +345,16 @@ parser.process(line); if (line === null) { let knownSubscriptions = new Map(); for (let subscription of parser.subscriptions) knownSubscriptions.set(subscription.url, subscription); this.fileProperties = parser.fileProperties; - this.subscriptions = parser.subscriptions; this.knownSubscriptions = knownSubscriptions; Filter.knownFilters = parser.knownFilters; Subscription.knownSubscriptions = parser.knownSubscriptions; if (!silent) filterNotifier.emit("load"); } }; @@ -389,17 +365,17 @@ * @return {Promise} promise resolved or rejected when loading is complete */ loadFromDisk() { let tryBackup = backupIndex => { return this.restoreBackup(backupIndex, true).then(() => { - if (this.subscriptions.length == 0) + if (this.knownSubscriptions.size == 0) return tryBackup(backupIndex + 1); }).catch(error => { // Give up }); }; return IO.statFile(this.sourceFile).then(statData => @@ -409,17 +385,17 @@ this.firstRun = true; return; } let parser = this.importData(true); return IO.readFromFile(this.sourceFile, parser).then(() => { parser(null); - if (this.subscriptions.length == 0) + if (this.knownSubscriptions.size == 0) { // No filter subscriptions in the file, this isn't right. throw new Error("No data in the file"); } }); }).catch(error => { Cu.reportError(error); @@ -463,19 +439,22 @@ }, /** * Generator serializing filter data and yielding it line by line. */ *exportData() { // Do not persist external subscriptions - let subscriptions = this.subscriptions.filter( - s => !(s instanceof ExternalSubscription) - ); + let subscriptions = []; + for (let subscription of this.subscriptions()) + { + if (!(subscription instanceof ExternalSubscription)) + subscriptions.push(subscription); + } yield "# Adblock Plus preferences"; yield "version=" + formatVersion; let saved = new Set(); let buf = []; // Save subscriptions Index: lib/synchronizer.js =================================================================== --- a/lib/synchronizer.js +++ b/lib/synchronizer.js @@ -90,17 +90,17 @@ * downloaded. * @yields {Downloadable} */ *_getDownloadables() { if (!Prefs.subscriptions_autoupdate) return; - for (let subscription of FilterStorage.subscriptions) + for (let subscription of FilterStorage.subscriptions()) { if (subscription instanceof DownloadableSubscription) yield this._getDownloadable(subscription, false); } } /** * Creates a {@link Downloadable} instance for a subscription. Index: test/filterStorage.js =================================================================== --- a/test/filterStorage.js +++ b/test/filterStorage.js @@ -41,43 +41,42 @@ function addListener(listener) { let makeWrapper = name => (...args) => listener(name, ...args); filterNotifier.on("subscription.added", makeWrapper("subscription.added")); filterNotifier.on("subscription.removed", makeWrapper("subscription.removed")); - filterNotifier.on("subscription.moved", makeWrapper("subscription.moved")); filterNotifier.on("filter.added", makeWrapper("filter.added")); filterNotifier.on("filter.removed", makeWrapper("filter.removed")); filterNotifier.on("filter.moved", makeWrapper("filter.moved")); filterNotifier.on("filter.hitCount", makeWrapper("filter.hitCount")); filterNotifier.on("filter.lastHit", makeWrapper("filter.lastHit")); } function compareSubscriptionList(test, testMessage, list, knownSubscriptions = null) { - let result = FilterStorage.subscriptions.map(subscription => subscription.url); + let result = [...FilterStorage.knownSubscriptions.keys()]; 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( + 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); @@ -203,48 +202,18 @@ if (action.indexOf("subscription.") == 0) changes.push(action + " " + subscription.url); } addListener(listener); compareSubscriptionList(test, "Initial state", [subscription1, subscription2, subscription3]); test.deepEqual(changes, [], "Received changes"); - changes = []; - FilterStorage.moveSubscription(subscription1); - compareSubscriptionList(test, "Move without explicit position", [subscription2, subscription3, subscription1]); - test.deepEqual(changes, ["subscription.moved http://test1/"], "Received changes"); - - changes = []; - FilterStorage.moveSubscription(subscription1); - compareSubscriptionList(test, "Move without explicit position (subscription already last)", [subscription2, subscription3, subscription1]); - test.deepEqual(changes, [], "Received changes"); - - changes = []; - FilterStorage.moveSubscription(subscription2, subscription1); - compareSubscriptionList(test, "Move with explicit position", [subscription3, subscription2, subscription1]); - test.deepEqual(changes, ["subscription.moved http://test2/"], "Received changes"); - - changes = []; - FilterStorage.moveSubscription(subscription3, subscription2); - compareSubscriptionList(test, "Move without explicit position (subscription already at position)", [subscription3, subscription2, subscription1]); - test.deepEqual(changes, [], "Received changes"); - FilterStorage.removeSubscription(subscription2); - compareSubscriptionList(test, "Remove", [subscription3, subscription1]); - - changes = []; - FilterStorage.moveSubscription(subscription3, subscription2); - compareSubscriptionList(test, "Move before removed subscription", [subscription1, subscription3]); - test.deepEqual(changes, ["subscription.moved http://test3/"], "Received changes"); - - changes = []; - FilterStorage.moveSubscription(subscription2); - compareSubscriptionList(test, "Move of removed subscription", [subscription1, subscription3]); - test.deepEqual(changes, [], "Received changes"); + compareSubscriptionList(test, "Remove", [subscription1, subscription3]); test.done(); }; exports.testAddingFilters = function(test) { let subscription1 = Subscription.fromURL("~blocking"); subscription1.defaults = ["blocking"]; Index: test/filterStorage_readwrite.js =================================================================== --- a/test/filterStorage_readwrite.js +++ b/test/filterStorage_readwrite.js @@ -106,17 +106,17 @@ if (withExternal) { { let subscription = new ExternalSubscription("~external~external subscription ID", "External subscription"); subscription.filters = [Filter.fromText("foo"), Filter.fromText("bar")]; FilterStorage.addSubscription(subscription); } - let externalSubscriptions = FilterStorage.subscriptions.filter(subscription => subscription instanceof ExternalSubscription); + let externalSubscriptions = [...FilterStorage.subscriptions()].filter(subscription => subscription instanceof ExternalSubscription); test.equal(externalSubscriptions.length, 1, "Number of external subscriptions after updateExternalSubscription"); test.equal(externalSubscriptions[0].url, "~external~external subscription ID", "ID of external subscription"); test.equal(externalSubscriptions[0].filters.length, 2, "Number of filters in external subscription"); } return FilterStorage.saveToDisk(); }).then(() => testData).then(expected => @@ -225,28 +225,28 @@ exports.testRestoringBackup = function(test) { Prefs.patternsbackups = 2; Prefs.patternsbackupinterval = 24; FilterStorage.saveToDisk().then(() => { - test.equal(FilterStorage.subscriptions[0].filters.length, 1, "Initial filter count"); + test.equal([...FilterStorage.subscriptions()][0].filters.length, 1, "Initial filter count"); FilterStorage.addFilter(Filter.fromText("barfoo")); - test.equal(FilterStorage.subscriptions[0].filters.length, 2, "Filter count after adding a filter"); + test.equal([...FilterStorage.subscriptions()][0].filters.length, 2, "Filter count after adding a filter"); return FilterStorage.saveToDisk(); }).then(() => { return FilterStorage.loadFromDisk(); }).then(() => { - test.equal(FilterStorage.subscriptions[0].filters.length, 2, "Filter count after adding filter and reloading"); + test.equal([...FilterStorage.subscriptions()][0].filters.length, 2, "Filter count after adding filter and reloading"); return FilterStorage.restoreBackup(1); }).then(() => { - test.equal(FilterStorage.subscriptions[0].filters.length, 1, "Filter count after restoring backup"); + test.equal([...FilterStorage.subscriptions()][0].filters.length, 1, "Filter count after restoring backup"); return FilterStorage.loadFromDisk(); }).then(() => { - test.equal(FilterStorage.subscriptions[0].filters.length, 1, "Filter count after reloading"); + test.equal([...FilterStorage.subscriptions()][0].filters.length, 1, "Filter count after reloading"); }).catch(unexpectedError.bind(test)).then(() => test.done()); }; Index: test/synchronizer.js =================================================================== --- a/test/synchronizer.js +++ b/test/synchronizer.js @@ -321,49 +321,49 @@ { return [Cr.NS_OK, 200, "[Adblock]\n!Redirect: http://example.com/redirected\nbar"]; }); let requests; this.runScheduledTasks(30).then(() => { - test.equal(FilterStorage.subscriptions[0], subscription, "Invalid redirect ignored"); + test.equal([...FilterStorage.subscriptions()][0], subscription, "Invalid redirect ignored"); test.equal(subscription.downloadStatus, "synchronize_connection_error", "Connection error recorded"); test.equal(subscription.errors, 2, "Number of download errors"); requests = []; this.registerHandler("/redirected", metadata => { requests.push(this.getTimeOffset()); return [Cr.NS_OK, 200, "[Adblock]\n! Expires: 8 hours\nbar"]; }); resetSubscription(subscription); return this.runScheduledTasks(15); }).then(() => { - test.equal(FilterStorage.subscriptions[0].url, "http://example.com/redirected", "Redirect followed"); + test.equal([...FilterStorage.subscriptions()][0].url, "http://example.com/redirected", "Redirect followed"); test.deepEqual(requests, [0 + initialDelay, 8 + initialDelay], "Resulting requests"); this.registerHandler("/redirected", metadata => { return [Cr.NS_OK, 200, "[Adblock]\n!Redirect: http://example.com/subscription\nbar"]; }); subscription = Subscription.fromURL("http://example.com/subscription"); resetSubscription(subscription); - FilterStorage.removeSubscription(FilterStorage.subscriptions[0]); + FilterStorage.removeSubscription([...FilterStorage.subscriptions()][0]); FilterStorage.addSubscription(subscription); return this.runScheduledTasks(2); }).then(() => { - test.equal(FilterStorage.subscriptions[0], subscription, "Redirect not followed on redirect loop"); + test.equal([...FilterStorage.subscriptions()][0], subscription, "Redirect not followed on redirect loop"); test.equal(subscription.downloadStatus, "synchronize_connection_error", "Download status after redirect loop"); }).catch(unexpectedError.bind(test)).then(() => test.done()); }; exports.testFallback = function(test) { Prefs.subscriptions_fallbackerrors = 3; Prefs.subscriptions_fallbackurl = "http://example.com/fallback?%SUBSCRIPTION%&%CHANNELSTATUS%&%RESPONSESTATUS%"; @@ -402,68 +402,68 @@ { test.deepEqual(requests, [0 + initialDelay, 24 + initialDelay, 48 + initialDelay], "Stop trying if the fallback responds with Gone"); test.equal(fallbackParams, "http://example.com/subscription&0&404", "Fallback arguments"); // Fallback redirecting to a missing file subscription = Subscription.fromURL("http://example.com/subscription"); resetSubscription(subscription); - FilterStorage.removeSubscription(FilterStorage.subscriptions[0]); + FilterStorage.removeSubscription([...FilterStorage.subscriptions()][0]); FilterStorage.addSubscription(subscription); requests = []; this.registerHandler("/fallback", metadata => { return [Cr.NS_OK, 200, "301 http://example.com/redirected"]; }); return this.runScheduledTasks(100); }).then(() => { - test.equal(FilterStorage.subscriptions[0].url, "http://example.com/subscription", "Ignore invalid redirect from fallback"); + test.equal([...FilterStorage.subscriptions()][0].url, "http://example.com/subscription", "Ignore invalid redirect from fallback"); test.deepEqual(requests, [0 + initialDelay, 24 + initialDelay, 48 + initialDelay, 72 + initialDelay, 96 + initialDelay], "Requests not affected by invalid redirect"); // Fallback redirecting to an existing file resetSubscription(subscription); requests = []; redirectedRequests = []; this.registerHandler("/redirected", metadata => { redirectedRequests.push(this.getTimeOffset()); return [Cr.NS_OK, 200, "[Adblock]\n!Expires: 1day\nfoo\nbar"]; }); return this.runScheduledTasks(100); }).then(() => { - test.equal(FilterStorage.subscriptions[0].url, "http://example.com/redirected", "Valid redirect from fallback is followed"); + test.equal([...FilterStorage.subscriptions()][0].url, "http://example.com/redirected", "Valid redirect from fallback is followed"); test.deepEqual(requests, [0 + initialDelay, 24 + initialDelay, 48 + initialDelay], "Stop polling original URL after a valid redirect from fallback"); test.deepEqual(redirectedRequests, [48 + initialDelay, 72 + initialDelay, 96 + initialDelay], "Request new URL after a valid redirect from fallback"); // Redirect loop this.registerHandler("/subscription", metadata => { return [Cr.NS_OK, 200, "[Adblock]\n! Redirect: http://example.com/subscription2"]; }); this.registerHandler("/subscription2", metadata => { return [Cr.NS_OK, 200, "[Adblock]\n! Redirect: http://example.com/subscription"]; }); subscription = Subscription.fromURL("http://example.com/subscription"); resetSubscription(subscription); - FilterStorage.removeSubscription(FilterStorage.subscriptions[0]); + FilterStorage.removeSubscription([...FilterStorage.subscriptions()][0]); FilterStorage.addSubscription(subscription); return this.runScheduledTasks(100); }).then(() => { - test.equal(FilterStorage.subscriptions[0].url, "http://example.com/redirected", "Fallback can still redirect even after a redirect loop"); + test.equal([...FilterStorage.subscriptions()][0].url, "http://example.com/redirected", "Fallback can still redirect even after a redirect loop"); }).catch(unexpectedError.bind(test)).then(() => test.done()); }; exports.testStateFields = function(test) { let subscription = Subscription.fromURL("http://example.com/subscription"); FilterStorage.addSubscription(subscription);