Refactored the abstraction layer to address prerendered pages on Safari caused by leaky abstraction

safari/ext/common.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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-(function() {
-  /* Events */
+(function()
+{
+  /* Message passing */
 
-  WrappedEventTarget = function(target, eventName, capture)
+  var MessageProxy = ext._MessageProxy = function(messageDispatcher)
   {
-    this._listeners = [];
-    this._wrappedListeners = [];
+    this._messageDispatcher = messageDispatcher;
+    this._responseCallbacks = {__proto__: null};
+    this._responseCallbackCounter = 0;
+  };
+  MessageProxy.prototype = {
+    _sendResponse: function(request, message)
+    {
+      var response = {};
+      for (var prop in request)
+        response[prop] = request[prop];
+      response.payload = message;
 
-    this._target = target;
-    this._eventName = eventName;
-    this._capture = capture;
-  };
-  WrappedEventTarget.prototype = {
-    addListener: function(listener)
+      this._messageDispatcher.dispatchMessage("response", response);
+    },
+    handleRequest: function(request, sender)
     {
-      var wrappedListener = this._wrapListener(listener);
+      var sendResponse;
+      if ("callbackId" in request)
+        sendResponse = this._sendResponse.bind(this, request);
+      else
+        sendResponse = function() {};
 
-      this._listeners.push(listener);
-      this._wrappedListeners.push(wrappedListener);
+      ext.onMessage._dispatch(request.payload, sender, sendResponse);
+    },
+    handleResponse: function(response)
+    {
+      var callbackId = response.callbackId;
+      var callback = this._responseCallbacks[callbackId];
+      if (callback)
+      {
+        delete this._responseCallbacks[callbackId];
+        callback(response.payload);
+      }
+    },
+    sendMessage: function(message, responseCallback, extra)
+    {
+      var request = {payload: message};
 
-      this._target.addEventListener(
-        this._eventName,
-        wrappedListener,
-        this._capture
-      );
-    },
-    removeListener: function(listener)
-    {
-      var idx = this._listeners.indexOf(listener);
+      if (responseCallback)
+      {
+        request.callbackId = ++this._responseCallbackCounter;
+        this._responseCallbacks[request.callbackId] = responseCallback;
+      }
 
-      if (idx != -1)
-      {
-        this._target.removeEventListener(
-          this._eventName,
-          this._wrappedListeners[idx],
-          this._capture
-        );
+      for (var prop in extra)
+        request[prop] = extra[prop];
 
-        this._listeners.splice(idx, 1);
-        this._wrappedListeners.splice(idx, 1);
-      }
+      this._messageDispatcher.dispatchMessage("request", request);
     }
   };
 
-  MessageEventTarget = function(target)
-  {
-    WrappedEventTarget.call(this, target, "message", false);
-  };
-  MessageEventTarget.prototype = {
-    __proto__: WrappedEventTarget.prototype,
-    _wrapListener: function(listener)
-    {
-      return function(event)
-      {
-        if (event.name == "request")
-          listener(event.message.payload, this._getSenderDetails(event), function(message)
-          {
-            this._getResponseDispatcher(event).dispatchMessage("response",
-            {
-              requestId: event.message.requestId,
-              payload: message
-            });
-          }.bind(this));
-      }.bind(this);
-    }
-  };
-
-
-  /* Message passing */
-
-  var requestCounter = 0;
-
-  _sendMessage = function(message, responseCallback, messageDispatcher, responseEventTarget, extra)
-  {
-    var requestId = ++requestCounter;
-
-    if (responseCallback)
-    {
-      var responseListener = function(event)
-      {
-        if (event.name == "response" && event.message.requestId == requestId)
-        {
-          responseEventTarget.removeEventListener("message", responseListener, false);
-          responseCallback(event.message.payload);
-        }
-      };
-      responseEventTarget.addEventListener("message", responseListener, false);
-    }
-
-    var rawMessage = {requestId: requestId, payload: message};
-    for (var k in extra)
-      rawMessage[k] = extra[k];
-    messageDispatcher.dispatchMessage("request", rawMessage);
-  };
+  ext.onMessage = new ext._EventTarget();
 
 
   /* I18n */
 
-  var I18n = function()
+  var localeCandidates = null;
+  var uiLocale;
+
+  var getLocaleCandidates = function()
   {
-    this._localeCandidates = this._getLocaleCandidates();
-    this._uiLocale = this._localeCandidates[0];
+    var candidates = [];
+    var defaultLocale = "en_US";
+
+    var bits, i;
+    for (i = (bits = navigator.language.split("-")).length; i > 0; i--)
+    {
+      var locale = bits.slice(0, i).join("_");
+      candidates.push(locale);
+
+      if (locale == defaultLocale)
+        return candidates;
+    }
+
+    candidates.push(defaultLocale);
+    return candidates;
   };
-  I18n.prototype = {
-    _getLocaleCandidates: function()
+
+  var getCatalog = function(locale)
+  {
+    var xhr = new XMLHttpRequest();
+
+    xhr.open("GET", safari.extension.baseURI + "_locales/" + locale + "/messages.json", false);
+
+    try {
+      xhr.send();
+    }
+    catch (e)
     {
-      var candidates = [];
-      var defaultLocale = "en_US";
+      return null;
+    }
 
-      var bits, i;
-      for (i = (bits = navigator.language.split("-")).length; i > 0; i--)
+    if (xhr.status != 200 && xhr.status != 0)
+      return null;
+
+    return JSON.parse(xhr.responseText);
+  };
+
+  ext.i18n = {
+    getMessage: function(msgId, substitutions)
+    {
+      if (!localeCandidates)
       {
-        var locale = bits.slice(0, i).join("_");
-        candidates.push(locale);
-
-        if (locale == defaultLocale)
-          return candidates;
+        localeCandidates = getLocaleCandidates();
+        uiLocale = localeCandidates[0];
       }
 
-      candidates.push(defaultLocale);
-      return candidates;
-    },
-    _getCatalog: function(locale)
-    {
-      var xhr = new XMLHttpRequest();
-      
-      xhr.open("GET", safari.extension.baseURI + "_locales/" + locale + "/messages.json", false);
+      if (msgId == "@@ui_locale")
+        return uiLocale;
 
-      try {
-        xhr.send();
-      }
-      catch (e)
+      for (var i = 0; i < localeCandidates.length; i++)
       {
-        return null;
-      }
-
-      if (xhr.status != 200 && xhr.status != 0)
-        return null;
-
-      return JSON.parse(xhr.responseText);
-    },
-    getMessage: function(msgId, substitutions)
-    {
-      if (msgId == "@@ui_locale")
-        return this._uiLocale;
-
-      for (var i = 0; i < this._localeCandidates.length; i++)
-      {
-        var catalog = this._getCatalog(this._localeCandidates[i]);
+        var catalog = getCatalog(localeCandidates[i]);
         if (!catalog)
         {
           // if there is no catalog for this locale
           // candidate, don't try to load it again
-          this._localeCandidates.splice(i--, 1);
+          localeCandidates.splice(i--, 1);
           continue;
         }
 
         var msg = catalog[msgId];
         if (!msg)
           continue;
 
         var msgstr = msg.message;
         if (!msgstr)
           continue;
 
         for (var placeholder in msg.placeholders)
         {
           var placeholderDetails = msg.placeholders[placeholder];
           if (!placeholderDetails || !placeholderDetails.content)
             continue;
           if (placeholderDetails.content.indexOf("$") != 0)
             continue;
 
           var placeholderIdx = parseInt(placeholderDetails.content.substr(1));
           if (isNaN(placeholderIdx) || placeholderIdx < 1)
             continue;
 
           var placeholderValue;
-          if (Object.prototype.toString.call(substitutions) == "[object Array]")
+          if (substitutions && substitutions.constructor == Array)

[Wladimir Palant, 2014/04/11 13:02:35]

Please remind me, why can't this be simply |substitutions instanceof Array|?

Note that this implementation is different from both Chrome and Firefox. Our
Firefox implementation always expects an array for substitutions, this is what
it should be per chrome.i18n.getMessage() documentation. The Chrome
implementation on the other hand seems to do something like this:

if (typeof substitutions == "string")
  substitutions = [substitutions];

Only one replacement is being done here and other types (e.g. numbers or even
String objects) result in an empty string.

[Sebastian Noack, 2014/04/11 14:47:45]

When I wrote the original code I followed the top answer of a popular
StackOverflow question
(https://stackoverflow.com/questions/4775722/check-if-object-is-array).

However now where I know a lot more about JavaScript that approach seems weird
to me and I don't understand how this answer got over 600 points. So I skipped
the string conversion, and directly checked for the constructor.

The only difference between checking the constructor and using the isinstance
operator, is that the isinstance operator will also return true, if the object
is an instance of a subclass of Array. But that shouldn't be an issue here.
The purpose of this code is to emulate Chrome's behavior, so this should be the
way to go. It might be that, I didn't tested chrome.i18n.getMessage() with
numbers, but were just expecting that they behave like strings, back when I
wrote the original code.

[Wladimir Palant, 2014/04/11 18:31:19]

The only case where instanceof Array is problematic is when dealing with
different JavaScript contexts - each window and frame has its own Array
constructor and an array from another window cannot be checked against your
constructor. Not really something that is a concern here however.

             placeholderValue = substitutions[placeholderIdx - 1];
           else if (placeholderIdx == 1)
             placeholderValue = substitutions;
 
           msgstr = msgstr.replace("$" + placeholder + "$", placeholderValue || "");
         }
 
         return msgstr;
       }
 
       return "";
     }
   };
 
 
-  /* API */
+  /* Utils */
 
-  ext = {
-    getURL: function(path)
-    {
-      return safari.extension.baseURI + path;
-    },
-    i18n: new I18n()
+  ext.getURL = function(path)
+  {
+    return safari.extension.baseURI + path;
   };
 })();