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

background.js

Index: background.js
===================================================================
--- a/background.js
+++ b/background.js
@@ -224,20 +224,15 @@
   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)
       {
@@ -247,14 +242,13 @@
         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)