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

Delta Between Two Patch Sets: lib/filterStorage.js

Issue 29853574: Issue 6855 - Release all references to Subscription object once removed (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Left Patch Set: Remove duplicate subscription entries from Filter object Created Aug. 12, 2018, 5:41 a.m.
Right Patch Set: Improve tests Created Aug. 16, 2018, 5:33 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | test/filterStorage.js » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 130 matching lines...) Expand 10 before | Expand all | Expand 10 after
141 removeSubscription(subscription) 141 removeSubscription(subscription)
142 { 142 {
143 for (let i = 0; i < FilterStorage.subscriptions.length; i++) 143 for (let i = 0; i < FilterStorage.subscriptions.length; i++)
144 { 144 {
145 if (FilterStorage.subscriptions[i].url == subscription.url) 145 if (FilterStorage.subscriptions[i].url == subscription.url)
146 { 146 {
147 removeSubscriptionFilters(subscription); 147 removeSubscriptionFilters(subscription);
148 148
149 FilterStorage.subscriptions.splice(i--, 1); 149 FilterStorage.subscriptions.splice(i--, 1);
150 FilterStorage.knownSubscriptions.delete(subscription.url); 150 FilterStorage.knownSubscriptions.delete(subscription.url);
151 FilterNotifier.triggerListeners("subscription.removed", subscription);
152 151
153 // This should be the last remaining reference to the Subscription 152 // This should be the last remaining reference to the Subscription
154 // object. 153 // object.
155 Subscription.knownSubscriptions.delete(subscription.url); 154 Subscription.knownSubscriptions.delete(subscription.url);
155
156 FilterNotifier.triggerListeners("subscription.removed", subscription);
156 return; 157 return;
157 } 158 }
158 } 159 }
159 }, 160 },
160 161
161 /** 162 /**
162 * Moves a subscription in the list to a new position. 163 * Moves a subscription in the list to a new position.
163 * @param {Subscription} subscription filter subscription to be moved 164 * @param {Subscription} subscription filter subscription to be moved
164 * @param {Subscription} [insertBefore] filter subscription to insert before 165 * @param {Subscription} [insertBefore] filter subscription to insert before
165 * (if omitted the subscription will be put at the end of the list) 166 * (if omitted the subscription will be put at the end of the list)
(...skipping 41 matching lines...) Expand 10 before | Expand all | Expand 10 after
207 * @param {Filter} filter 208 * @param {Filter} filter
208 * @param {SpecialSubscription} [subscription] 209 * @param {SpecialSubscription} [subscription]
209 * particular group that the filter should be added to 210 * particular group that the filter should be added to
210 * @param {number} [position] 211 * @param {number} [position]
211 * position within the subscription at which the filter should be added 212 * position within the subscription at which the filter should be added
212 */ 213 */
213 addFilter(filter, subscription, position) 214 addFilter(filter, subscription, position)
214 { 215 {
215 if (!subscription) 216 if (!subscription)
216 { 217 {
217 if (filter.subscriptions.some(s => s instanceof SpecialSubscription && 218 for (let currentSubscription of filter.subscriptions)
218 !s.disabled)) 219 {
219 { 220 if (currentSubscription instanceof SpecialSubscription &&
220 return; // No need to add 221 !currentSubscription.disabled)
222 {
223 return; // No need to add
224 }
221 } 225 }
222 subscription = FilterStorage.getGroupForFilter(filter); 226 subscription = FilterStorage.getGroupForFilter(filter);
223 } 227 }
224 if (!subscription) 228 if (!subscription)
225 { 229 {
226 // No group for this filter exists, create one 230 // No group for this filter exists, create one
227 subscription = SpecialSubscription.createForFilter(filter); 231 subscription = SpecialSubscription.createForFilter(filter);
228 this.addSubscription(subscription); 232 this.addSubscription(subscription);
229 return; 233 return;
230 } 234 }
231 235
232 if (typeof position == "undefined") 236 if (typeof position == "undefined")
233 position = subscription.filters.length; 237 position = subscription.filters.length;
234 238
235 if (filter.subscriptions.indexOf(subscription) < 0) 239 filter.subscriptions.add(subscription);
236 filter.subscriptions.push(subscription);
237 subscription.filters.splice(position, 0, filter); 240 subscription.filters.splice(position, 0, filter);
238 FilterNotifier.triggerListeners("filter.added", filter, subscription, 241 FilterNotifier.triggerListeners("filter.added", filter, subscription,
239 position); 242 position);
240 }, 243 },
241 244
242 /** 245 /**
243 * Removes a user-defined filter from the list 246 * Removes a user-defined filter from the list
244 * @param {Filter} filter 247 * @param {Filter} filter
245 * @param {SpecialSubscription} [subscription] a particular filter group that 248 * @param {SpecialSubscription} [subscription] a particular filter group that
246 * the filter should be removed from (if ommited will be removed from all 249 * the filter should be removed from (if ommited will be removed from all
247 * subscriptions) 250 * subscriptions)
248 * @param {number} [position] position inside the filter group at which the 251 * @param {number} [position] position inside the filter group at which the
249 * filter should be removed (if ommited all instances will be removed) 252 * filter should be removed (if ommited all instances will be removed)
250 */ 253 */
251 removeFilter(filter, subscription, position) 254 removeFilter(filter, subscription, position)
252 { 255 {
253 let subscriptions = ( 256 let subscriptions = (
254 subscription ? [subscription] : filter.subscriptions.slice() 257 subscription ? [subscription] : filter.subscriptions
255 ); 258 );
256 for (let i = 0; i < subscriptions.length; i++) 259 for (let currentSubscription of subscriptions)
257 { 260 {
258 let currentSubscription = subscriptions[i];
259 if (currentSubscription instanceof SpecialSubscription) 261 if (currentSubscription instanceof SpecialSubscription)
260 { 262 {
261 let positions = []; 263 let positions = [];
262 if (typeof position == "undefined") 264 if (typeof position == "undefined")
263 { 265 {
264 let index = -1; 266 let index = -1;
265 do 267 do
266 { 268 {
267 index = currentSubscription.filters.indexOf(filter, index + 1); 269 index = currentSubscription.filters.indexOf(filter, index + 1);
268 if (index >= 0) 270 if (index >= 0)
269 positions.push(index); 271 positions.push(index);
270 } while (index >= 0); 272 } while (index >= 0);
271 } 273 }
272 else 274 else
273 positions.push(position); 275 positions.push(position);
274 276
275 for (let j = positions.length - 1; j >= 0; j--) 277 for (let j = positions.length - 1; j >= 0; j--)
276 { 278 {
277 let currentPosition = positions[j]; 279 let currentPosition = positions[j];
278 if (currentSubscription.filters[currentPosition] == filter) 280 if (currentSubscription.filters[currentPosition] == filter)
279 { 281 {
280 currentSubscription.filters.splice(currentPosition, 1); 282 currentSubscription.filters.splice(currentPosition, 1);
281 if (currentSubscription.filters.indexOf(filter) < 0) 283 if (currentSubscription.filters.indexOf(filter) < 0)
282 { 284 filter.subscriptions.delete(currentSubscription);
283 let index = filter.subscriptions.indexOf(currentSubscription);
284 if (index >= 0)
285 filter.subscriptions.splice(index, 1);
286 }
287 FilterNotifier.triggerListeners( 285 FilterNotifier.triggerListeners(
288 "filter.removed", filter, currentSubscription, currentPosition 286 "filter.removed", filter, currentSubscription, currentPosition
289 ); 287 );
290 } 288 }
291 } 289 }
292 } 290 }
293 } 291 }
294 }, 292 },
295 293
296 /** 294 /**
(...skipping 362 matching lines...) Expand 10 before | Expand all | Expand 10 after
659 * Joins subscription's filters to the subscription without any notifications. 657 * Joins subscription's filters to the subscription without any notifications.
660 * @param {Subscription} subscription 658 * @param {Subscription} subscription
661 * filter subscription that should be connected to its filters 659 * filter subscription that should be connected to its filters
662 */ 660 */
663 function addSubscriptionFilters(subscription) 661 function addSubscriptionFilters(subscription)
664 { 662 {
665 if (!FilterStorage.knownSubscriptions.has(subscription.url)) 663 if (!FilterStorage.knownSubscriptions.has(subscription.url))
666 return; 664 return;
667 665
668 for (let filter of subscription.filters) 666 for (let filter of subscription.filters)
669 filter.subscriptions.push(subscription); 667 filter.subscriptions.add(subscription);
670 } 668 }
671 669
672 /** 670 /**
673 * Removes subscription's filters from the subscription without any 671 * Removes subscription's filters from the subscription without any
674 * notifications. 672 * notifications.
675 * @param {Subscription} subscription filter subscription to be removed 673 * @param {Subscription} subscription filter subscription to be removed
676 */ 674 */
677 function removeSubscriptionFilters(subscription) 675 function removeSubscriptionFilters(subscription)
678 { 676 {
679 if (!FilterStorage.knownSubscriptions.has(subscription.url)) 677 if (!FilterStorage.knownSubscriptions.has(subscription.url))
680 return; 678 return;
681 679
682 for (let filter of subscription.filters) 680 for (let filter of subscription.filters)
683 { 681 filter.subscriptions.delete(subscription);
684 let index = 0;
685
686 // The same filter can occur more than once in a subscription. We could
687 // avoid making duplicate subscription entries in a Filter object in
688 // INIParser, but we don't do this in order to avoid slowing down the
689 // loading of the initial subscriptions from disk.
690 while ((index = filter.subscriptions.indexOf(subscription), index) != -1)
691 filter.subscriptions.splice(index, 1);
692 }
693 } 682 }
694 683
695 /** 684 /**
696 * Listener returned by FilterStorage.importData(), parses filter data. 685 * Listener returned by FilterStorage.importData(), parses filter data.
697 * @constructor 686 * @constructor
698 */ 687 */
699 function INIParser() 688 function INIParser()
700 { 689 {
701 this.fileProperties = this.curObj = {}; 690 this.fileProperties = this.curObj = {};
702 this.subscriptions = []; 691 this.subscriptions = [];
(...skipping 42 matching lines...) Expand 10 before | Expand all | Expand 10 after
745 case "subscription filters": 734 case "subscription filters":
746 if (this.subscriptions.length) 735 if (this.subscriptions.length)
747 { 736 {
748 let subscription = this.subscriptions[ 737 let subscription = this.subscriptions[
749 this.subscriptions.length - 1 738 this.subscriptions.length - 1
750 ]; 739 ];
751 for (let text of this.curObj) 740 for (let text of this.curObj)
752 { 741 {
753 let filter = Filter.fromText(text); 742 let filter = Filter.fromText(text);
754 subscription.filters.push(filter); 743 subscription.filters.push(filter);
755 filter.subscriptions.push(subscription); 744 filter.subscriptions.add(subscription);
756 } 745 }
757 } 746 }
758 break; 747 break;
759 } 748 }
760 } 749 }
761 750
762 if (val === null) 751 if (val === null)
763 return; 752 return;
764 753
765 this.curSection = match[1].toLowerCase(); 754 this.curSection = match[1].toLowerCase();
(...skipping 16 matching lines...) Expand all
782 else if (this.wantObj === false && val) 771 else if (this.wantObj === false && val)
783 this.curObj.push(val.replace(/\\\[/g, "[")); 772 this.curObj.push(val.replace(/\\\[/g, "["));
784 } 773 }
785 finally 774 finally
786 { 775 {
787 Filter.knownFilters = origKnownFilters; 776 Filter.knownFilters = origKnownFilters;
788 Subscription.knownSubscriptions = origKnownSubscriptions; 777 Subscription.knownSubscriptions = origKnownSubscriptions;
789 } 778 }
790 } 779 }
791 }; 780 };
LEFTRIGHT

Powered by Google App Engine
This is Rietveld