Unified Diff: lib/filterStorage.js
Issue 29800557:
Issue 6559 - Use Map object for known subscriptions (Closed)
Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Move check for known subscription into removeSubscription
Created June 6, 2018, 11 a.m.
Use n/p to move between diff chunks;
N/P to move between comments.
« lib/filterListener.js ('K')
|
« lib/filterListener.js ('k')
|
lib/subscriptionClasses.js » ('j')
|
lib/subscriptionClasses.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
| Index: lib/filterStorage.js |
| =================================================================== |
| --- a/lib/filterStorage.js |
| +++ b/lib/filterStorage.js |
| @@ -81,19 +81,19 @@ |
| /** |
| * List of filter subscriptions containing all filters |
| * @type {Subscription[]} |
| */ |
| subscriptions: [], |
| /** |
| * Map of subscriptions already on the list, by their URL/identifier |
| - * @type {Object} |
| + * @type {Map.<string,Subscription>} |
| */ |
| - knownSubscriptions: Object.create(null), |
| + 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) |
| @@ -119,40 +119,43 @@ |
| }, |
| /** |
| * Adds a filter subscription to the list |
| * @param {Subscription} subscription filter subscription to be added |
| */ |
| addSubscription(subscription) |
| { |
| - if (subscription.url in FilterStorage.knownSubscriptions) |
| + if (FilterStorage.knownSubscriptions.has(subscription.url)) |
| return; |
| FilterStorage.subscriptions.push(subscription); |
| - FilterStorage.knownSubscriptions[subscription.url] = subscription; |
| + FilterStorage.knownSubscriptions.set(subscription.url, subscription); |
| addSubscriptionFilters(subscription); |
| FilterNotifier.triggerListeners("subscription.added", subscription); |
| }, |
| /** |
| * Removes a filter subscription from the list |
| * @param {Subscription} subscription filter subscription to be removed |
| */ |
| removeSubscription(subscription) |
| { |
| + if (!FilterStorage.knownSubscriptions.has(subscription.url)) |
|
Manish Jethani
2018/06/06 11:04:42
IMO it makes more sense to do the check here rathe
|
| + return; |
| + |
| for (let i = 0; i < FilterStorage.subscriptions.length; i++) |
| { |
| if (FilterStorage.subscriptions[i].url == subscription.url) |
| { |
| removeSubscriptionFilters(subscription); |
| FilterStorage.subscriptions.splice(i--, 1); |
| - delete FilterStorage.knownSubscriptions[subscription.url]; |
| + FilterStorage.knownSubscriptions.delete(subscription.url); |
| FilterNotifier.triggerListeners("subscription.removed", subscription); |
| return; |
| } |
| } |
| }, |
| /** |
| * Moves a subscription in the list to a new position. |
| @@ -363,19 +366,19 @@ |
| importData(silent) |
| { |
| let parser = new INIParser(); |
| return line => |
| { |
| parser.process(line); |
| if (line === null) |
| { |
| - let knownSubscriptions = Object.create(null); |
| + let knownSubscriptions = new Map(); |
| for (let subscription of parser.subscriptions) |
| - knownSubscriptions[subscription.url] = subscription; |
| + 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) |
| @@ -653,31 +656,31 @@ |
| /** |
| * Joins subscription's filters to the subscription without any notifications. |
| * @param {Subscription} subscription |
| * filter subscription that should be connected to its filters |
| */ |
| function addSubscriptionFilters(subscription) |
| { |
| - if (!(subscription.url in FilterStorage.knownSubscriptions)) |
| + if (!FilterStorage.knownSubscriptions.has(subscription.url)) |
| return; |
| for (let filter of subscription.filters) |
| filter.subscriptions.push(subscription); |
| } |
| /** |
| * Removes subscription's filters from the subscription without any |
| * notifications. |
| * @param {Subscription} subscription filter subscription to be removed |
| */ |
| function removeSubscriptionFilters(subscription) |
| { |
| - if (!(subscription.url in FilterStorage.knownSubscriptions)) |
| + if (!FilterStorage.knownSubscriptions.has(subscription.url)) |
| return; |
| for (let filter of subscription.filters) |
| { |
| let i = filter.subscriptions.indexOf(subscription); |
| if (i >= 0) |
| filter.subscriptions.splice(i, 1); |
| } |
| @@ -687,17 +690,17 @@ |
| * Listener returned by FilterStorage.importData(), parses filter data. |
| * @constructor |
| */ |
| function INIParser() |
| { |
| this.fileProperties = this.curObj = {}; |
| this.subscriptions = []; |
| this.knownFilters = new Map(); |
| - this.knownSubscriptions = Object.create(null); |
| + this.knownSubscriptions = new Map(); |
| } |
| INIParser.prototype = |
| { |
| linesProcessed: 0, |
| subscriptions: null, |
| knownFilters: null, |
| knownSubscriptions: null, |
| wantObj: true, |
« lib/filterListener.js ('K')
|
« lib/filterListener.js ('k')
|
lib/subscriptionClasses.js » ('j')
|
lib/subscriptionClasses.js » ('J')
