Don't delay Firefox startup

lib/io.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 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 52 matching lines @@
 
   /**
    * Reads strings from a file asynchronously, calls listener.process() with
    * each line read and with a null parameter once the read operation is done.
    * The callback will be called when the operation is done.
    */
   readFromFile: function(/**nsIFile|nsIURI*/ file, /**Boolean*/ decode, /**Object*/ listener, /**Function*/ callback, /**String*/ timeLineID)
   {
     try
     {
+      let processing = false;
       let buffer = "";
+      let loadEvent = null;
+      let errorEvent = null;
       let uri = file instanceof Ci.nsIFile ? Services.io.newFileURI(file) : file;
       let request = new XMLHttpRequest();
       request.mozBackgroundRequest = true;
       request.open("GET", uri.spec);
       request.responseType = "moz-chunked-text";
       request.overrideMimeType("text/plain" + (decode ? "? charset=utf-8" : ""));
 
-      request.addEventListener("progress", function(event)
+      let onProgress = function(event)
       {
         if (timeLineID)
         {
           TimeLine.asyncStart(timeLineID);
         }
 
         let data = event.target.response;
-        let index = Math.max(data.lastIndexOf("\n"), data.lastIndexOf("\r"));
+        let index = (processing ? -1 : Math.max(data.lastIndexOf("\n"), data.lastIndexOf("\r")));

[Felix Dahlke, 2014/03/14 16:10:45]

This doesn't really seem reentrant to me. What if "processing" is false here,
but actually gets set to true before we try to set it to true below? I think we
need some locking while the variable is tested and set.

[Wladimir Palant, 2014/03/14 18:04:51]

JavaScript is a single-threaded language, reentrance cannot happen at a random
time. A call to Utils.yield() will spin the event loop however, which means that
events waiting to be processed will be processed. That call will return only
once all the events have been removed from the queue.

One such event can be XMLHttpRequest's progress event. So while we are in the
call to listener.process(...) this event handler can be called again (or the
handler for the "load" or "error" events) if the listener calls Utils.yield().

[Felix Dahlke, 2014/03/14 21:32:49]

Ah, thought this was actually proper multithreading. But yeah, currentThread
pretty much implies that it's not. So listener.process() can call Utils.yield()
which can in turn call onProcess() while we're still in the middle of
processing. Got it.

         if (index >= 0)
         {
-          let oldBuffer = buffer;
-          buffer = data.substr(index + 1);
-          data = data.substr(0, index + 1);
-          let lines = data.split(/[\r\n]+/);
-          lines.pop();
-          lines[0] = oldBuffer + lines[0];
-          for (let i = 0; i < lines.length; i++)
-            listener.process(lines[i]);
+          // Protect against reentrance in case the listener processes events.

[Felix Dahlke, 2014/03/14 16:10:45]

"processes multiple events" I suppose?

[Wladimir Palant, 2014/03/14 18:04:51]

No, "processes events" = "spins the event loop"

+          processing = true;
+          try
+          {
+            let oldBuffer = buffer;
+            buffer = data.substr(index + 1);

[Felix Dahlke, 2014/03/14 16:10:45]

Is reference assignment atomic in Gecko? Otherwise we'll probably want some
locking. Same goes for loadEvent and errorEvent.

[Wladimir Palant, 2014/03/14 18:04:51]

As I said, the only place where we might get reentrance is the call to the
listener.

+            data = data.substr(0, index + 1);
+            let lines = data.split(/[\r\n]+/);
+            lines.pop();
+            lines[0] = oldBuffer + lines[0];
+            for (let i = 0; i < lines.length; i++)
+              listener.process(lines[i]);
+          }
+          finally
+          {
+            processing = false;
+            let e = {
+              target: {response: buffer}
+            };
+            buffer = "";
+            onProgress(e);
+
+            if (loadEvent)
+            {
+              loadEvent = null;
+              onLoad(loadEvent);

[Felix Dahlke, 2014/03/14 16:10:45]

onLoad(null) doesn't seem to be what's desired here. Same for onError() below.

[Wladimir Palant, 2014/03/14 18:04:51]

True, we need to save the original event before nulling it out. Nulling out has
to happen before the call however, just in case the load listener calls
Utils.yield() as well and we get another "progress" event in the meantime (a
rather theoretical situation).

+            }
+
+            if (errorEvent)
+            {
+              errorEvent = null;
+              onError(errorEvent);
+            }
+          }
         }
         else
           buffer += data;
 
         if (timeLineID)
         {
           TimeLine.asyncEnd(timeLineID);
         }
-      }, false);
+      };
 
-      request.addEventListener("load", function(event)
+      let onLoad = function(event)
       {
+        if (processing)
+        {
+          // Still processing data, delay processing this event.
+          loadEvent = event;
+          return;
+        }
+
         if (timeLineID)
         {
           TimeLine.asyncStart(timeLineID);
         }
 
         if (buffer !== "")
           listener.process(buffer);
         listener.process(null);
 
         if (timeLineID)
         {
           TimeLine.asyncEnd(timeLineID);
           TimeLine.asyncDone(timeLineID);
         }
 
         callback(null);
-      }, false);
+      };
 
-      request.addEventListener("error", function()
+      let onError = function(event)
       {
+        if (processing)
+        {
+          // Still processing data, delay processing this event.
+          errorEvent = event;
+          return;
+        }
+
         let e = Cc["@mozilla.org/js/xpc/Exception;1"].createInstance(Ci.nsIXPCException);
         e.initialize("File read operation failed", result, null, Components.stack, file, null);
         callback(e);
 
         if (timeLineID)
         {
           TimeLine.asyncDone(timeLineID);
         }
-      }, false);
+      };
+
+      request.addEventListener("progress", onProgress, false);
+      request.addEventListener("load", onLoad, false);
+      request.addEventListener("error", onError, false);
 
       request.send(null);
     }
     catch (e)
     {
       callback(e);
     }
   },
   /**
    * Writes string data to a file asynchronously, optionally encodes it into
@@ skipping 179 matching lines @@
         isFile: exists && file.isFile(),
         lastModified: exists ? file.lastModifiedTime : 0
       });
     }
     catch(e)
     {
       callback(e);
     }
   }
 }