Issue 1706 - Move first-run page to adblockplusui repository

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(global)
 {
   const Ci = Components.interfaces;
 
-  Object.defineProperty(global, "Page", {

[Sebastian Noack, 2015/01/07 10:27:52]

We only expose Page objects in the background page on Chrome/Opera/Safari. Any
reason you do so in ext/common.js here?

[Wladimir Palant, 2015/01/07 16:36:49]

MessageProxy._handleRequest() below needs to use *something* as sender
parameter. And rather than checking "are we running in background" every time a
message is handled I'd prefer to have a dummy page constructor in content.
However, having the memoization on the global object is indeed misguided - it
should really be a local thing.

[Sebastian Noack, 2015/01/07 17:00:39]

I'm not sure if I correctly understand the problem. But on Chrome/Opera/Safari
|sender.page| is undefined when the message is sent by the background page.
However, I didn't check explicitly whether we are in the background page, but
abstracted that code path in content.js/background.js.

[Wladimir Palant, 2015/01/07 19:01:19]

Yes, the way this is implemented in Safari the sender isn't generated in the
message proxy but rather passed in from outside - that's because the message
proxy doesn't listen to the messages itself, it is rather being triggered by
message listeners defined elsewhere. I consider this a rather awkward solution,
related logic being spread out throughout the code. Here on the other hand the
message proxy registers its own listeners, so having different handling for
background and content isn't quite as straightforward.

-    get: function()
+  if (!global.ext)
+    global.ext = {};
+
+  var holder = {
+    get Page()
     {
       delete this.Page;
       this.Page = (typeof require == "function" ?
           require("ext_background").Page :
           function() {});
       return this.Page;
-    },
-    configurable: true
-  });
+    }
+  };
 
-  function getSender(origin)
+  var getSender = global.ext._getSender = function(origin)
   {
     if (origin instanceof Ci.nsIDOMXULElement)
       return origin.messageManager;
     else if (origin instanceof Ci.nsIMessageSender)
       return origin;
     else
       return null;
   }
 
-  function MessageProxy(messageManager, messageTarget)
+  var MessageProxy = global.ext._MessageProxy = function(messageManager, messageTarget)
   {
     this._messageManager = messageManager;
     this._messageTarget = messageTarget;
     this._callbacks = new Map();
     this._responseCallbackCounter = 0;
 
     this._handleRequest = this._handleRequest.bind(this);
     this._handleResponse = this._handleResponse.bind(this);
     this._messageManager.addMessageListener("AdblockPlus:Message", this._handleRequest);
     this._messageManager.addMessageListener("AdblockPlus:Response", this._handleResponse);
@@ skipping 24 matching lines @@
       var sender = getSender(message.target);
       var request = message.data;
       var sendResponse;
       if (sender && "callbackId" in request)
         sendResponse = this._sendResponse.bind(this, sender, request.callbackId);
       else
         sendResponse = function() {};
 
       this._responseSent = false;
       var result = this._messageTarget._dispatch(request.payload, {
-        page: new Page(sender)
+        page: new holder.Page(sender)
       }, sendResponse);
       if (!result && !this._responseSent)
         sendResponse(undefined);
     },
 
     _handleResponse: function(message)
     {
       var response = message.data;
       var callback = this._callbacks.get(response.callbackId);
       if (callback)
@@ skipping 15 matching lines @@
       if (responseCallback)
       {
         request.callbackId = ++this._responseCallbackCounter;
         this._callbacks.set(request.callbackId, responseCallback);
       }
 
       this._messageManager.sendAsyncMessage("AdblockPlus:Message", request);
     }
   };
 
-  function EventTarget(cancelable)

[Sebastian Noack, 2015/01/07 10:27:52]

The argument is unused.

+  var EventTarget = global.ext._EventTarget = function()
   {
     this._listeners = [];
   };
   EventTarget.prototype = {
     addListener: function(listener)
     {
       if (this._listeners.indexOf(listener) == -1)
         this._listeners.push(listener);
     },
     removeListener: function(listener)
     {
       var idx = this._listeners.indexOf(listener);
       if (idx != -1)
         this._listeners.splice(idx, 1);
     },
     _dispatch: function()
     {
       var result = null;
 
       for (var i = 0; i < this._listeners.length; i++)
         result = this._listeners[i].apply(null, arguments);
 
       return result;
     }
   };
 
   if (typeof exports == "object")
-  {

[Sebastian Noack, 2015/01/07 10:27:52]

What is this branch good for?

[Wladimir Palant, 2015/01/07 16:36:49]

This file is included either as CommonJS module (by the "background page", via
require("ext_common")) or a regular script. In the first scenario it needs to
export variables by added in them to the global exports variable, in the second
it needs to change global.ext.

[Sebastian Noack, 2015/01/07 17:00:39]

Why not |exports = ext|?

[Wladimir Palant, 2015/01/07 19:01:19]

Surprisingly enough, that actually works with our module loader. Let's go with
this for consistency, even though I'd rather have this export MessageProxy
rather than _MessageProxy.

-    exports.MessageProxy = MessageProxy;
-    exports.EventTarget = EventTarget;
-    exports.getSender = getSender;
-  }
-  else
-  {
-    if (!global.ext)
-      global.ext = {};
-    global.ext._MessageProxy = MessageProxy;
-    global.ext._EventTarget = EventTarget;
-  }
+    exports = global.ext;
 })(this);