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 214 matching lines @@
     return;
 
   Promise.resolve(addSubscription && fetch("subscriptions.xml")
     .then(function(response)
     {
       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);
+      var nodes = doc.getElementsByTagName("subscription");
+      var node = Utils.chooseFilterSubscription(nodes);
+      var subscription = node && Subscription.fromURL(node.getAttribute("url"));
+
       if (subscription)
       {
         FilterStorage.addSubscription(subscription);
+
         subscription.disabled = false;
         subscription.title = node.getAttribute("title");
         subscription.homepage = node.getAttribute("homepage");
-        if (subscription instanceof DownloadableSubscription && !subscription.lastDownload)
+
+        if (subscription instanceof DownloadableSubscription &&
+            !subscription.lastDownload)
           Synchronizer.execute(subscription);
       }
     })
-  ).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.

+  )
+  .then(function()
   {
     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();
 });
 
@@ skipping 110 matching lines @@
       {
         sendResponse({status: "invalid", error: result.errors.join("\n")});
         break;
       }
 
       for (var i = 0; i < result.filters.length; i++)
         FilterStorage.addFilter(result.filters[i]);
 
       sendResponse({status: "ok"});
       break;
-    case "add-subscription":
-      ext.showOptions(function(page)
-      {
-        page.sendMessage(msg);
-      });
-      break;
     case "add-sitekey":
       processKey(msg.token, sender.page, sender.frame);
       break;
     case "report-html-page":
       htmlPages.set(sender.page, null);
       refreshIconAndContextMenu(sender.page);
       break;
     case "compose-filters":
       sendResponse(composeFilters({
         tagName: msg.tagName,
@@ skipping 23 matching lines @@
   }
 });
 
 // update icon when page changes location
 ext.pages.onLoading.addListener(function(page)
 {
   page.sendMessage({type: "clickhide-deactivate"});
   refreshIconAndContextMenu(page);
   showNextNotificationForUrl(page.url);
 });