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

Issue 29853574: Issue 6855 - Release all references to Subscription object once removed (Closed)

Created:
Aug. 11, 2018, 3:21 p.m. by Manish Jethani
Modified:
Aug. 16, 2018, 8:06 p.m.
Reviewers:
hub
CC:
kzar
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

To test this: 1. Load the extension with the default subscriptions 2. Take a heap snapshot in DevTools 3. Subscribe to EasyList China at https://adblockplus.org/subscriptions 4. Take a second heap snapshot 5. Delete EasyList China 6. Take a third heap snapshot When you search for "Subscription" in the heap snapshot, you'll notice that only the second snapshot has one more Subscription object for EasyList China. It is gone again in the third snapshot. Be sure to run garbage collection manually before taking each heap snapshot.

Patch Set 1 #

Total comments: 1

Patch Set 2 : Simplify #

Patch Set 3 : Remove duplicate subscription entries from Filter object #

Patch Set 4 : Add tests #

Patch Set 5 : Add test for duplicate subscriptions #

Patch Set 6 : Remove Subscription object before emitting "subscription.removed" event #

Patch Set 7 : Rebase #

Patch Set 8 : Improve tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -7 lines) Patch
M lib/filterStorage.js View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M test/filterStorage.js View 1 2 3 4 5 6 7 2 chunks +30 lines, -7 lines 0 comments Download

Messages

Total messages: 6
Manish Jethani
Aug. 11, 2018, 3:21 p.m. (2018-08-11 15:21:08 UTC) #1
Manish Jethani
Patch Set 1 https://codereview.adblockplus.org/29853574/diff/29853575/lib/filterStorage.js File lib/filterStorage.js (right): https://codereview.adblockplus.org/29853574/diff/29853575/lib/filterStorage.js#newcode153 lib/filterStorage.js:153: subscription.free(); At this point all the ...
Aug. 11, 2018, 3:28 p.m. (2018-08-11 15:28:17 UTC) #2
Manish Jethani
Patch Set 2: Simplify Let's do the free method in a separate patch that encapsulated ...
Aug. 12, 2018, 6:51 a.m. (2018-08-12 06:51:29 UTC) #3
Manish Jethani
Patch Set 6: Remove Subscription object before emitting "subscription.removed" event
Aug. 12, 2018, 7:22 a.m. (2018-08-12 07:22:48 UTC) #4
Manish Jethani
Patch Set 7: Rebase Patch Set 8: Improve tests
Aug. 16, 2018, 5:34 a.m. (2018-08-16 05:34:29 UTC) #5
hub
Aug. 16, 2018, 5:36 p.m. (2018-08-16 17:36:25 UTC) #6
LGTM

Powered by Google App Engine
This is Rietveld