Issue 6559 - Use Map object for known subscriptions

lib/synchronizer.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 182 matching lines @@
     let subscription = Subscription.fromURL(downloadable.redirectURL ||
                                             downloadable.url);
     if (downloadable.redirectURL &&
         downloadable.redirectURL != downloadable.url)
     {
       let oldSubscription = Subscription.fromURL(downloadable.url);
       subscription.title = oldSubscription.title;
       subscription.disabled = oldSubscription.disabled;
       subscription.lastCheck = oldSubscription.lastCheck;
 
-      let listed = (oldSubscription.url in FilterStorage.knownSubscriptions);
-      if (listed)

[kzar, 2018/06/07 13:52:26]

But we already have `listed` here, but now we don't use that and perform the
lookup again in the FilterStorage code? I don't care too much either way, but I
guess I don't understand your reasoning so far.

[Manish Jethani, 2018/06/07 14:29:46]

Yeah I have to admit the last change is not exactly related, let's leave it out
for now (so let's focus on Patch Set 2).

I did it because it made sense that both addSubscription and removeSubscription
would do the same type of check, but let's leave it out. Ideally
removeSubscription would return true if the subscription was actually removed,
which would become the value of "listed" here (so no additional lookup).

-        FilterStorage.removeSubscription(oldSubscription);
+      let listed = FilterStorage.knownSubscriptions.has(oldSubscription.url);
+      FilterStorage.removeSubscription(oldSubscription);
 
-      delete Subscription.knownSubscriptions[oldSubscription.url];
+      Subscription.knownSubscriptions.delete(oldSubscription.url);
 
       if (listed)
         FilterStorage.addSubscription(subscription);
     }
 
     // The download actually succeeded
     subscription.lastSuccess = subscription.lastDownload = Math.round(
       Date.now() / MILLIS_IN_SECOND
     );
     subscription.downloadStatus = "synchronize_ok";
@@ skipping 110 matching lines @@
         request.open("GET", fallbackURL);
         request.overrideMimeType("text/plain");
         request.channel.loadFlags = request.channel.loadFlags |
                                     request.channel.INHIBIT_CACHING |
                                     request.channel.VALIDATE_ALWAYS;
         request.addEventListener("load", ev =>
         {
           if (onShutdown.done)
             return;
 
-          if (!(subscription.url in FilterStorage.knownSubscriptions))
+          if (!FilterStorage.knownSubscriptions.has(subscription.url))
             return;
 
           let match = /^(\d+)(?:\s+(\S+))?$/.exec(request.responseText);
           if (match && match[1] == "301" &&    // Moved permanently
               match[2] && /^https?:\/\//i.test(match[2]))
           {
             redirectCallback(match[2]);
           }
           else if (match && match[1] == "410") // Gone
           {
             let data = "[Adblock]\n" +
               subscription.filters.map(f => f.text).join("\n");
             redirectCallback("data:text/plain," + encodeURIComponent(data));
           }
         }, false);
         request.send(null);
       }
     }
   }
 };
 Synchronizer.init();