Issue 3515 - Use fetch() API instead XMLHttpRequest (Platform)

background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 206 matching lines @@
       subscription.disabled = true;
       FilterStorage.addSubscription(subscription);
       if (subscription instanceof DownloadableSubscription && !subscription.lastDownload)
         Synchronizer.execute(subscription);
     }
   }
 
   if (!addSubscription && !addAcceptable)
     return;
 
-  function notifyUser()
-  {
-    if (!Prefs.suppress_first_run_page)
-      ext.pages.open(ext.getURL("firstRun.html"));
-  }
-
-  if (addSubscription)
-  {
-    // Load subscriptions data
-    var request = new XMLHttpRequest();
-    request.open("GET", "subscriptions.xml");
-    request.addEventListener("load", function()
+  Promise.resolve(addSubscription && fetch("subscriptions.xml")
+    .then(function(response)
     {
-      var node = Utils.chooseFilterSubscription(request.responseXML.getElementsByTagName("subscription"));
+      return response.text();
+    })
+    .then(function(text)
+    {
+      var doc = new DOMParser().parseFromString(text, "application/xml");
+      var node = Utils.chooseFilterSubscription(doc.getElementsByTagName("subscription"));

[kzar, 2016/01/17 17:14:06]

Nit: While changing mind fixing some of these long lines?

[Sebastian Noack, 2016/01/19 14:58:10]

Done.

       var subscription = (node ? Subscription.fromURL(node.getAttribute("url")) : null);
       if (subscription)
       {
         FilterStorage.addSubscription(subscription);
         subscription.disabled = false;
         subscription.title = node.getAttribute("title");
         subscription.homepage = node.getAttribute("homepage");
         if (subscription instanceof DownloadableSubscription && !subscription.lastDownload)
           Synchronizer.execute(subscription);
-
-          notifyUser();
       }
-    }, false);
-    request.send(null);
-  }
-  else
-    notifyUser();
+    })
+  ).then(function()

[kzar, 2016/01/17 17:14:06]

Nit: The indentation of this `.then(...` call seems inconsistent with the
previous one.

[Sebastian Noack, 2016/01/19 14:58:10]

Intending the code above like that:

  Promise.resolve(addSubscription &&
fetch("subscriptions.xml").then(function(response)
  {
    ..
  }).then(text)
  {
    ..
  });

would make it harder to identify the then() calls. And the first line would be
too long. On the other hand:

  Promise.resolve(
   ..
  )
    .then(function()
    {
      ..
    })

would look quite weird.

[kzar, 2016/01/19 15:39:19]

What about doing this here? That way they'd all be lined up.

  }))
  .then(function()
    ...

[Sebastian Noack, 2016/01/19 16:13:25]

Note sure if I like it any better, but done.

+  {
+    if (!Prefs.suppress_first_run_page)

[kzar, 2016/01/17 17:14:06]

I think previously we always called `notifyUser()` but now we only execute it's
code if `addSubscription` is truthy and everything succeeds. Deliberate?

[Sebastian Noack, 2016/01/19 14:58:10]

That's not correct. Promise.resolve(addSubscription && fetch(..).then(..))
returns a promise that is immediately fulfilled with the value of
addSubscription if it is false, or when the request was sent and its response
was processed successfully.

[kzar, 2016/01/19 15:39:19]

Ah, I see.

+      ext.pages.open(ext.getURL("firstRun.html"));
+  });
 }
 
 Prefs.onChanged.addListener(function(name)
 {
   if (name == "shouldShowBlockElementMenu")
     refreshIconAndContextMenuForAllPages();
 });
 
 // This is a hack to speedup loading of the options page on Safari.
 // Once we replaced the background page proxy with message passing
@@ skipping 157 matching lines @@
   }
 });
 
 // update icon when page changes location
 ext.pages.onLoading.addListener(function(page)
 {
   page.sendMessage({type: "clickhide-deactivate"});
   refreshIconAndContextMenu(page);
   showNextNotificationForUrl(page.url);
 });