Issue 5464 - Upgrade to new asynchronous version of abp2blocklist

safari/contentBlocking.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 34 matching lines @@
 {
   ext.pages.query({}, pages =>
   {
     for (let page of pages)
       page.browserAction.setBadge();
   });
 }
 
 function setContentBlocker()
 {
   return new Promise((resolve, reject) =>

[Manish Jethani, 2017/08/02 10:55:11]

I made this a promise now because it feels more natural here and also because
errors bubble up naturally, whereas they may be lost with the callback.

   {
     // Reset state and either fulfill or reject this promise.
-    let completePromise = error =>
+    function completePromise(error)
     {
       lastSetContentBlockerError = error;
       contentBlockListDirty = false;
 
-      if (error)
+      if (error instanceof Error)
         reject(error);
       else
         resolve();
-    };
+    }
 
     // When given the same rules as last time setContentBlocker will always
     // resolve with null (success), even when there was actually an
     // error. We cache the last result therefore so that we can provide a
     // consistent result and also to avoid wastefully regenerating an identical
     // blocklist.
     if (!contentBlockListDirty)
     {
       completePromise(lastSetContentBlockerError);
       return;
     }
 
     let contentBlockerList = new ContentBlockerList();
     for (let subscription of FilterStorage.subscriptions)
     {

[Manish Jethani, 2017/08/02 10:55:11]

Added some extra braces as per our style guide.

       if (!subscription.disabled)
       {
         for (let filter of subscription.filters)
           contentBlockerList.addFilter(filter);
       }
     }
 
     contentBlockerList.generateRules().then(rules =>
     {
       safari.extension.setContentBlocker(
@@ skipping 20 matching lines @@
   });
 }
 
 function updateContentBlocker(isStartup, legacyAPISupported)
 {
   // Another update can be requested while one is still in progress (e.g. the
   // user adds filter lists in quick succession). When this happens, save the
   // request and execute it later.
   if (afterContentBlockingFinished)
   {
     pendingContentBlockerUpdate = {

[Manish Jethani, 2017/08/02 10:55:11]

Note that we only need to save the last request. We can discard intermediate
ones. We're only interested in making sure the rule set used by Safari is really
based on the user's last selection.

See also this comment:

https://issues.adblockplus.org/ticket/5464#comment:7

       params: Array.from(arguments),
       // Save the current dirty state so we can set it later before calling
       // this function again.
       setDirty: contentBlockListDirty
     };
     return;
   }
 
-  afterContentBlockingFinished = new Promise(resolve =>
-  {
-    let setContentBlockerHandler = error =>
-    {
-      if (error instanceof Error)
-      {
-        let suppressErrorMessage = false;
-
-        // If the content blocking API fails the first time it's used the
-        // legacy blocking API (if available) won't have been disabled.
-        if (!contentBlockingActive && legacyAPISupported)
-        {
-          Prefs.safariContentBlocker = false;
-          // If content blocking failed on startup and we're switching back to
-          // the legacy API anyway we don't need to show an error message.
-          if (isStartup)
-            suppressErrorMessage = true;
-        }
-
-        if (!suppressErrorMessage)
-          alert(error.message);
-      }
-      else if (!contentBlockingActive)
-      {
-        contentBlockingActive = true;
-        clearBlockCounters();
-      }
-
-      afterContentBlockingFinished = null;

[Manish Jethani, 2017/08/02 10:55:11]

This should be set to null before fulfilling the promise just to be correct.

-      resolve(contentBlockingActive);
-    };
-
-    // There's no need for a catch handler at the end because the handler is
-    // not expected to throw any errors. If this changes, however, then the
-    // error must be handled appropriately.
-    setContentBlocker().then(setContentBlockerHandler,
-                             setContentBlockerHandler);
+  afterContentBlockingFinished = setContentBlocker().then(() =>
+  {
+    if (!contentBlockingActive)
+    {
+      contentBlockingActive = true;
+      clearBlockCounters();
+    }
+  },
+  error =>
+  {
+    let suppressErrorMessage = false;
+
+    // If the content blocking API fails the first time it's used the
+    // legacy blocking API (if available) won't have been disabled.
+    if (!contentBlockingActive && legacyAPISupported)
+    {
+      Prefs.safariContentBlocker = false;
+      // If content blocking failed on startup and we're switching back to
+      // the legacy API anyway we don't need to show an error message.
+      if (isStartup)
+        suppressErrorMessage = true;
+    }
+
+    if (!suppressErrorMessage)
+      alert(error.message);
   })
-  .then(active =>
-  {
+  .then(() =>
+  {
+    afterContentBlockingFinished = null;
+
     // If there's another update pending, execute it now.
     if (pendingContentBlockerUpdate)
     {
       let {params, setDirty} = pendingContentBlockerUpdate;
       pendingContentBlockerUpdate = null;
 
       if (setDirty)
         contentBlockListDirty = true;
 
       updateContentBlocker.apply(null, params);
       return afterContentBlockingFinished;
     }
 
-    return active;
+    return contentBlockingActive;
   });
 }
 
 if (contentBlockingSupported)
 {
   Promise.all([Prefs.untilLoaded,
                FilterNotifier.once("load"),
                legacyAPISupported]).then(resolvedValues =>
   {
     let legacyAPISupported = resolvedValues[2];
@@ skipping 17 matching lines @@
     });
   });
 }
 
 port.on("safari.contentBlockingActive", (msg, sender) =>
 {
   if (!contentBlockingActive && afterContentBlockingFinished)
     return afterContentBlockingFinished;
   return contentBlockingActive;
 });