Issue 293 - AdBlock button is slow to appear on toolbar after launching Firefox

lib/ui.js

Index: lib/ui.js
===================================================================
--- a/lib/ui.js
+++ b/lib/ui.js
@@ -279,16 +279,19 @@ let UI = exports.UI =
     request.open("GET", "chrome://adblockplus/content/ui/overlay.xul");
     request.addEventListener("load", function(event)
     {
       if (onShutdown.done)
         return;
 
       this.processOverlay(request.responseXML.documentElement);
 
+      // Don't wait for the rest of the startup sequence, add icon already
+      this.addToolbarButton();
+
       overlayLoaded = true;
       if (overlayLoaded && filtersLoaded && sessionRestored)
         this.initDone();
     }.bind(this), false);
     request.send(null);
 
     // Wait for filters to load
     if (FilterStorage._loading)
@@ -391,20 +394,74 @@ let UI = exports.UI =
   },
 
   /**
    * 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)
+    {
+      if (name == "enabled" || name == "defaulttoolbaraction" || name == "defaultstatusbaraction")
+        this.updateState();
+      else if (name == "showinstatusbar")
+      {
+        for (let window in 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));
+
+    notificationTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+    notificationTimer.initWithCallback(this.showNextNotification.bind(this),
+                                       3 * 60 * 1000, Ci.nsITimer.TYPE_ONE_SHOT);
+    onShutdown.add(function() notificationTimer.cancel());
+
+    // Add "anti-adblock messages" notification
+    initAntiAdblockNotification();
+
+    let documentCreationObserver = {
+      observe: function(subject, topic, data)
+      {
+        if (!(subject instanceof Ci.nsIDOMWindow))
+          return;
+
+        this.showNextNotification(subject.location.href);
+      }.bind(UI)
+    };
+    Services.obs.addObserver(documentCreationObserver, "content-document-global-created", false);
+    onShutdown.add(function()
+    {
+      Services.obs.removeObserver(documentCreationObserver, "content-document-global-created", false);
+    });
+
+    if (!this.firstRunDone)
+    {
+      // Execute first-run actions if a window is open already, otherwise it
+      // will happen in applyToWindow() when a window is opened.
+      let window = this.currentWindow;
+      if (window)
+        this.firstRunActions(window);
+    }
+  },
+
+  addToolbarButton: function()
+  {
     let {WindowObserver} = require("windowObserver");
     new WindowObserver(this);
 
-    // Add toolbar icon
     let {defaultToolbarPosition} = require("appSupport");
     if ("abp-toolbarbutton" in this.overlay && defaultToolbarPosition)
     {
       try
       {
         ({CustomizableUI}) = Cu.import("resource:///modules/CustomizableUI.jsm", null);
       }
       catch (e)
@@ -435,56 +492,32 @@ let UI = exports.UI =
         {
           // For emulation only, this callback isn't part of the official
           // CustomizableUI API.
           this.updateIconState(node.ownerDocument.defaultView, node);
         }.bind(this),
       });
       onShutdown.add(CustomizableUI.destroyWidget.bind(CustomizableUI, "abp-toolbarbutton"));
     }
+  },
 
-    // Listen for pref and filters changes
-    Prefs.addListener(function(name)
+  firstRunActions: function(window)

[Thomas Greiner, 2014/05/23 16:10:31]

Is there a case in which firstRunActions are / could be run without
this.firstRunDone being true? If not, why not include it in this function
directly instead of right before calling firstRunActions?

[Wladimir Palant, 2014/05/26 11:16:52]

This was pretty much premature optimization on my end, moved the check inside
the function.

+  {
+    if (FilterStorage._loading)
+      return;
+
+    this.firstRunDone = true;
+
+    let {addonVersion} = require("info");
+    let prevVersion = Prefs.currentVersion;
+    if (prevVersion != addonVersion)
     {
-      if (name == "enabled" || name == "defaulttoolbaraction" || name == "defaultstatusbaraction")
-        this.updateState();
-      else if (name == "showinstatusbar")
-      {
-        for (let window in 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));
-
-    notificationTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    notificationTimer.initWithCallback(this.showNextNotification.bind(this),
-                                       3 * 60 * 1000, Ci.nsITimer.TYPE_ONE_SHOT);
-    onShutdown.add(function() notificationTimer.cancel());
-
-    // Add "anti-adblock messages" notification
-    initAntiAdblockNotification();
-
-    let documentCreationObserver = {
-      observe: function(subject, topic, data)
-      {
-        if (!(subject instanceof Ci.nsIDOMWindow))
-          return;
-
-        this.showNextNotification(subject.location.href);
-      }.bind(UI)
-    };
-    Services.obs.addObserver(documentCreationObserver, "content-document-global-created", false);
-    onShutdown.add(function()
-    {
-      Services.obs.removeObserver(documentCreationObserver, "content-document-global-created", false);
-    });
+      Prefs.currentVersion = addonVersion;
+      this.addSubscription(window, prevVersion);
+    }
   },
 
   /**
    * Will be set to true after the check whether first-run actions should run
    * has been performed.
    * @type Boolean
    */
   firstRunDone: false,
@@ -549,27 +582,17 @@ let UI = exports.UI =
 
     window.document.getElementById("abp-notification-close").addEventListener("command", function(event)
     {
       window.document.getElementById("abp-notification").hidePopup();
     }, false);
 
     // First-run actions?
     if (!this.firstRunDone)
-    {
-      this.firstRunDone = true;
-
-      let {addonVersion} = require("info");
-      let prevVersion = Prefs.currentVersion;
-      if (prevVersion != addonVersion)
-      {
-        Prefs.currentVersion = addonVersion;
-        this.addSubscription(window, prevVersion);
-      }
-    }
+      this.firstRunActions(window);
 
     // Some people actually switch off browser.frames.enabled and are surprised
     // that things stop working...
     window.QueryInterface(Ci.nsIInterfaceRequestor)
           .getInterface(Ci.nsIWebNavigation)
           .QueryInterface(Ci.nsIDocShell)
           .allowSubframes = true;
   },