Issue 4131 - Simplify UI initialization logic

lib/ui.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 260 matching lines @@
 let notificationTimer = null;
 
 let UI = exports.UI =
 {
   /**
    * Gets called on startup, initializes UI integration.
    */
   init: function()
   {
     // We have to wait for multiple events before running start-up actions
-    let promises = [];

[Thomas Greiner, 2016/06/08 15:47:32]

Detail: That variable name is not very descriptive because it's like calling an
array of numbers "numbers". For instance, I could imagine naming it
"startupActions" based on your comment.

[Wladimir Palant, 2016/06/08 20:51:30]

Done.

+    let prerequisites = [];
 
     // Start loading overlay
-    promises.push(new Promise((resolve, reject) =>
+    prerequisites.push(new Promise((resolve, reject) =>
     {
       let request = new XMLHttpRequest();
       request.mozBackgroundRequest = true;
       request.open("GET", "chrome://adblockplus/content/ui/overlay.xul");
       request.channel.owner = Utils.systemPrincipal;
       request.addEventListener("load", event =>
       {
         if (onShutdown.done)
           return;

[Thomas Greiner, 2016/06/08 15:47:32]

Instead of returning I'd suggest to resolve or reject the promise instead to
avoid it being in an unresolved state forever. Note that I couldn't find hints
in the standard as to what the browser is supposed to do with promises that
remain unresolved. So seems like it's up to the GC to decide what to do with
those.

Same also applies to the missing handling of the "error" event since the "load"
event is not being dispatched if an error occurs.

[Wladimir Palant, 2016/06/08 20:51:30]

We don't really want to do anything with that promise. If we resolve it, the
follow-up actions will run. If we reject, this will generate an error message.
Neither is desirable at this point, the extension has been shut down. Not doing
anything and allowing the promise to be garbage collected seems to be the best
course of action.
The original code didn't handle the "error" event either, simply because there
is no way this download would error out. In fact, chrome:// is a synchronous
protocol and there are no asynchronous error events. Still, fair enough - let's
fix this and reject the promise on error (had to replace protocol by http:// in
order to test this).

 
         this.processOverlay(request.responseXML.documentElement);
 
         // Don't wait for the rest of the startup sequence, add icon already
         this.addToolbarButton();
 
         resolve();
       }, false);
+
+      request.addEventListener("error", event =>
+      {
+        reject(new Error("Unexpected: Failed to load overlay.xul"));
+      });
+
       request.send(null);
     }));
 
     // Wait for filters to load
     if (FilterStorage._loading)
-      promises.push(FilterNotifier.once("load"));
+      prerequisites.push(FilterNotifier.once("load"));
 
     // Wait for session to be restored
-    promises.push(new Promise((resolve, reject) =>
+    prerequisites.push(new Promise((resolve, reject) =>
     {
       let window = this.currentWindow;
       if (!window && "nsISessionStore" in Ci)
       {
         // No application windows yet, the application must be starting up. Wait
         // for session to be restored before initializing our UI.
         new SessionRestoreObserver(resolve);
       }
       else
         resolve();
     }));
 
-    Promise.all(promises).then(() => this.initDone());
+    Promise.all(prerequisites).then(() => this.initDone())
+        .catch(e => Cu.reportError(e));
   },
 
   /**
    * Provesses overlay document data and initializes overlay property.
    */
   processOverlay: function(/**Element*/ root)
   {
     Utils.splitAllLabels(root);
 
     let specialElements = {"abp-status-popup": true, "abp-status": true, "abp-toolbarbutton": true, "abp-menuitem": true, "abp-bottombar-container": true};
@@ skipping 57 matching lines @@
   /**
    * Gets called once the initialization is finished and Adblock Plus elements
    * can be added to the UI.
    */
   initDone: function()
   {
     // The icon might be added already, make sure its state is correct
     this.updateState();
 
     // Listen for pref and filters changes
-    Prefs.addListener(function(name)
+    Prefs.addListener(name =>
     {
       if (name == "enabled" || name == "defaulttoolbaraction" || name == "defaultstatusbaraction")
         this.updateState();
       else if (name == "showinstatusbar")
       {
         for (let window of this.applicationWindows)
           this.updateStatusbarIcon(window);
       }
-    }.bind(this));
-    FilterNotifier.addListener(function(action)
-    {
-      if (/^(filter|subscription)\.(added|removed|disabled|updated)$/.test(action) || action == "load")
-        this.updateState();
-    }.bind(this));
+    });
+
+    for (let eventName of [
+      "filter.added", "filter.removed", "filter.disabled",
+      "subscription.added", "subscription.removed", "subscription.disabled",
+      "subscription.updated", "load"
+    ])
+    {
+      FilterNotifier.on(eventName, () => this.updateState());
+    }
 
     Notification.addShowListener(notification =>
     {
       let window = this.currentWindow;
       if (!window)
         return;
 
       let button = window.document.getElementById("abp-toolbarbutton")
           || window.document.getElementById("abp-status");
       if (!button)
@@ skipping 1451 matching lines @@
   ["abp-command-contribute-hide", "command", UI.hideContributeButton.bind(UI)],
   ["abp-command-toggleshownotifications", "command", Notification.toggleIgnoreCategory.bind(Notification, "*", null)]
 ];
 
 onShutdown.add(function()
 {
   for (let window of UI.applicationWindows)
     if (UI.isBottombarOpen(window))
       UI.toggleBottombar(window);
 });