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

chrome/ext/background.js

Index: chrome/ext/background.js
===================================================================
--- a/chrome/ext/background.js
+++ b/chrome/ext/background.js
@@ -17,183 +17,140 @@
 
 (function()
 {
-  /* Events */
+  /* Pages */
 
-  var TabEventTarget = function()
+  var sendMessage = chrome.tabs.sendMessage || chrome.tabs.sendRequest;
+
+  var Page = ext.Page = function(tab)
   {
-    WrappedEventTarget.apply(this, arguments);
+    this._id = tab.id;
+    this._url = tab.url;
 
-    this._tabs = {};
+    this.browserAction = new BrowserAction(tab.id);
+  };
+  Page.prototype = {
+    get url()
+    {
+      // usually our Page objects are created from Chrome's Tab objects, which
+      // provide the url. So we can return the url given in the constructor.
+      if (this._url != null)
+        return this._url;
 
-    this._sharedListener = this._sharedListener.bind(this);
-    this._removeTab = this._removeTab.bind(this);
-  };
-  TabEventTarget.prototype = {
-    __proto__: WrappedEventTarget.prototype,
-    _bindToTab: function(tab)
+      // but sometimes we only have the tab id when we create a Page object.
+      // In that case we get the url from top frame of the tab, recorded by
+      // the onBeforeRequest handler.
+      var frames = framesOfTabs[this._id];
+      if (frames)
+      {
+        var frame = frames[0];
+        if (frame)
+          return frame.url;
+      }
+    },
+    activate: function()
     {
-      return {
-        addListener: function(listener)
-        {
-          var listeners = this._tabs[tab._id];
-
-          if (!listeners)
-          {
-            this._tabs[tab._id] = listeners = [];
-
-            if (Object.keys(this._tabs).length == 1)
-              this.addListener(this._sharedListener);
-
-            tab.onRemoved.addListener(this._removeTab);
-          }
-
-          listeners.push(listener);
-        }.bind(this),
-        removeListener: function(listener)
-        {
-          var listeners = this._tabs[tab._id];
-          if (!listeners)
-            return;
-
-          var idx = listeners.indexOf(listener);
-          if (idx == -1)
-            return;
-
-          listeners.splice(idx, 1);
-
-          if (listeners.length == 0)
-            tab.onRemoved.removeListener(this._removeTab);
-          else
-          {
-            if (listeners.length > 1)
-              return;
-            if (listeners[0] != this._removeTab)
-              return;
-          }
-
-          this._removeTab(tab);
-        }.bind(this)
-      };
+      chrome.tabs.update(this._id, {selected: true});
     },
-    _sharedListener: function(tab)
+    sendMessage: function(message, responseCallback)
     {
-      var listeners = this._tabs[tab._id];
-
-      if (!listeners)
-        return;
-
-      // copy listeners before calling them, because they might
-      // add or remove other listeners, which must not be taken
-      // into account before the next occurrence of the event
-      listeners = listeners.slice(0);
-
-      for (var i = 0; i < listeners.length; i++)
-        listeners[i](tab);
-    },
-    _removeTab: function(tab)
-    {
-      delete this._tabs[tab._id];
-
-      if (Object.keys(this._tabs).length == 0)
-        this.removeListener(this._sharedListener);
+      sendMessage(this._id, message, responseCallback);
     }
   };
 
-  var BeforeNavigateTabEventTarget = function()
-  {
-    TabEventTarget.call(this, chrome.webNavigation.onBeforeNavigate);
-  };
-  BeforeNavigateTabEventTarget.prototype = {
-    __proto__: TabEventTarget.prototype,
-    _wrapListener: function(listener)
+  ext.pages = {
+    open: function(url, callback)
     {
-      return function(details)
+      if (callback)
+        chrome.tabs.create({url: url}, function(tab) { callback(new Page(tab)); });
+      else
+        chrome.tabs.create({url: url});
+    },
+    query: function(info, callback)
+    {
+      var rawInfo = {};
+      var visibleWindow = null;
+
+      for (var property in info)
       {
-        if (details.frameId == 0)
-          listener(new Tab({id: details.tabId, url: details.url}));
-      };
-    }
+        switch (property)
+        {
+          case "active":
+          case "lastFocusedWindow":
+            rawInfo[property] = info[property];
+            break;
+          case "visibleWindow":
+            visibleWindow = info[property];
+            break;
+        }
+      }
+
+      chrome.tabs.query(rawInfo, function(tabs)
+      {
+        if (visibleWindow != null && tabs.length > 0)
+        {
+          var windows = {__proto__: null};
+          var pending = 0;
+
+          for (var i = 0; i < tabs.length; i++)
+          {
+            var windowId = tabs[i].windowId;
+            if (!(windowId in windows))
+            {
+              windows[windowId] = null;
+              pending++;
+
+              chrome.windows.get(windowId, null, function(win)
+              {
+                if (visibleWindow != (win.state != "minimized"))

[Wladimir Palant, 2014/04/04 14:00:35]

That's quite a bit of code just to exclude minimized windows. Do we really need
it?

[Sebastian Noack, 2014/04/07 13:15:25]

We need that for the icon animation, in order to animate the icon only for
visible tabs if any. That code isn't actually new, it was just moved from
iconAnimation.js to chrome/ext/background.js.

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

It seems that the original code was only animating the current tab so it didn't
care whether the window is minimized. As discussed on IRC, excluding minimized
windows doesn't seem to be useful - even it helps lower CPU consumptions in some
scenarios (users with lots of minimized windows?), that isn't really worth the
complexity here.

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

Done.

+                {
+                  for (var j = 0; j < tabs.length; j++)
+                  {
+                    if (tabs[j].windowId == win.id)
+                      tabs.splice(j--, 1);
+                  }
+                }
+
+                if (--pending == 0)
+                  callback(tabs.map(function(tab)
+                  {
+                    return new Page(tab);
+                  }));
+              });
+            }
+          }
+        }
+        else
+        {
+          callback(tabs.map(function(tab)
+          {
+            return new Page(tab);
+          }));
+        }
+      });
+    },
+    onLoading: new ext._EventTarget()
   };
 
-  var LoadingTabEventTarget = function()
+  chrome.tabs.onUpdated.addListener(function(tabId, changeInfo, tab)
   {
-    TabEventTarget.call(this, chrome.tabs.onUpdated);
-  };
-  LoadingTabEventTarget.prototype = {
-    __proto__: TabEventTarget.prototype,
-    _wrapListener: function(listener)
-    {
-      return function(id, info, tab)
-      {
-        if (info.status == "loading")
-          listener(new Tab(tab));
-      };
-    }
-  };
+    if (changeInfo.status == "loading")
+      ext.pages.onLoading._dispatch(new Page(tab));
+  });
 
-  var CompletedTabEventTarget = function()
+  chrome.webNavigation.onBeforeNavigate.addListener(function(details)
   {
-    TabEventTarget.call(this, chrome.tabs.onUpdated);
-  };
-  CompletedTabEventTarget.prototype = {
-    __proto__: TabEventTarget.prototype,
-    _wrapListener: function(listener)
-    {
-      return function(id, info, tab)
-      {
-        if (info.status == "complete")
-          listener(new Tab(tab));
-      };
-    }
-  };
+    if (details.frameId == 0)
+      ext._removeFromAllPageMaps(details.tabId);
+  });

[Wladimir Palant, 2014/04/04 14:00:35]

Just wondering: will this also be triggered for navigation to a javascript: URL?

[Sebastian Noack, 2014/04/07 13:15:25]

It isn't triggered for javascript: URLs entered into the address bar.

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

I did some testing and it seems that using onBeforeNavigate as opposed to
onCommitted is sane.

 
-  var ActivatedTabEventTarget = function()
+  chrome.tabs.onRemoved.addListener(function(tabId)
   {
-    TabEventTarget.call(this, chrome.tabs.onActivated);
-  };
-  ActivatedTabEventTarget.prototype = {
-    __proto__: TabEventTarget.prototype,
-    _wrapListener: function(listener)
-    {
-      return function(info)
-      {
-        chrome.tabs.get(info.tabId, function(tab)
-        {
-          listener(new Tab(tab));
-        });
-      };
-    }
-  }
+    ext._removeFromAllPageMaps(tabId);
+    delete framesOfTabs[tabId];
+  });
 
-  var RemovedTabEventTarget = function()
-  {
-    TabEventTarget.call(this, chrome.tabs.onRemoved);
-  };
-  RemovedTabEventTarget.prototype = {
-    __proto__: TabEventTarget.prototype,
-    _wrapListener: function(listener)
-    {
-      return function(id) { listener(new Tab({id: id})); };
-    }
-  };
 
-  var BackgroundMessageEventTarget = function()
-  {
-    MessageEventTarget.call(this);
-  }
-  BackgroundMessageEventTarget.prototype = {
-    __proto__: MessageEventTarget.prototype,
-    _wrapSender: function(sender)
-    {
-      var tab = new Tab(sender.tab);
-      return {tab: tab, frame: new Frame({url: sender.url, tab: tab})};
-    }
-  };
-
-
-  /* Tabs */
-
-  var sendMessage = chrome.tabs.sendMessage || chrome.tabs.sendRequest;
+  /* Browser actions */
 
   var BrowserAction = function(tabId)
   {
@@ -233,148 +190,15 @@
     }
   };
 
-  Tab = function(tab)
-  {
-    this._id = tab.id;
-    this._url = tab.url;
-
-    this.browserAction = new BrowserAction(tab.id);
-
-    this.onLoading = ext.tabs.onLoading._bindToTab(this);
-    this.onCompleted = ext.tabs.onCompleted._bindToTab(this);
-    this.onActivated = ext.tabs.onActivated._bindToTab(this);
-    this.onRemoved = ext.tabs.onRemoved._bindToTab(this);
-
-    // the "beforeNavigate" event in Safari isn't dispatched when a new URL
-    // was entered into the address bar. So we can only use it only on Chrome,
-    // but we have to hide it from the browser-independent high level code.
-    this._onBeforeNavigate = ext.tabs._onBeforeNavigate._bindToTab(this);
-  };
-  Tab.prototype = {
-    get url()
-    {
-      // usually our Tab objects are created from chrome Tab objects, which
-      // provide the url. So we can return the url given in the constructor.
-      if (this._url != null)
-        return this._url;
-
-      // but sometimes we only have the id when we create a Tab object.
-      // In that case we get the url from top frame of the tab, recorded by
-      // the onBeforeRequest handler.
-      var frames = framesOfTabs.get(this);
-      if (frames)
-      {
-        var frame = frames[0];
-        if (frame)
-          return frame.url;
-      }
-    },
-    close: function()
-    {
-      chrome.tabs.remove(this._id);
-    },
-    activate: function()
-    {
-      chrome.tabs.update(this._id, {selected: true});
-    },
-    sendMessage: function(message, responseCallback)
-    {
-      sendMessage(this._id, message, responseCallback);
-    }
-  };
-
-  TabMap = function(deleteOnPageUnload)
-  {
-    this._map = {};
-
-    this._delete = this._delete.bind(this);
-    this._deleteOnPageUnload = deleteOnPageUnload;
-  };
-  TabMap.prototype = {
-    get: function(tab)
-    {
-      return (this._map[tab._id] || {}).value;
-    },
-    set: function(tab, value)
-    {
-      if (!(tab._id in this._map))
-      {
-        tab.onRemoved.addListener(this._delete);
-        if (this._deleteOnPageUnload)
-          tab._onBeforeNavigate.addListener(this._delete);
-      }
-
-      this._map[tab._id] = {tab: tab, value: value};
-    },
-    has: function(tab)
-    {
-      return tab._id in this._map;
-    },
-    clear: function()
-    {
-      for (var id in this._map)
-        this.delete(this._map[id].tab);
-    },
-    _delete: function(tab)
-    {
-      // delay so that other event handlers can still lookup this tab
-      setTimeout(this.delete.bind(this, tab), 0);
-    },
-    delete: function(tab)
-    {
-      delete this._map[tab._id];
-
-      tab.onRemoved.removeListener(this._delete);
-      tab._onBeforeNavigate.removeListener(this._delete);
-    }
-  };
-
-
-  /* Windows */
-
-  Window = function(win)
-  {
-    this._id = win.id;
-    this.visible = win.status != "minimized";
-  };
-  Window.prototype = {
-    getAllTabs: function(callback)
-    {
-      chrome.tabs.query({windowId: this._id}, function(tabs)
-      {
-        callback(tabs.map(function(tab) { return new Tab(tab); }));
-      });
-    },
-    getActiveTab: function(callback)
-    {
-      chrome.tabs.query({windowId: this._id, active: true}, function(tabs)
-      {
-        callback(new Tab(tabs[0]));
-      });
-    },
-    openTab: function(url, callback)
-    {
-      var props = {windowId: this._id, url: url};
-
-      if (!callback)
-        chrome.tabs.create(props);
-      else
-        chrome.tabs.create(props, function(tab)
-        {
-          callback(new Tab(tab));
-        });
-    }
-  };
-
 
   /* Frames */
 
-  var framesOfTabs = new TabMap();
+  var framesOfTabs = {__proto__: null};
 
-  Frame = function(params)
+  var Frame = ext.Frame = function(params)
   {
-    this._tab = params.tab;
-    this._id = params.id;
+    this._frameId = params.frameId;
+    this._tabId = params.tabId;
     this._url = params.url;
   };
   Frame.prototype = {
@@ -383,22 +207,22 @@
       if (this._url != null)
         return this._url;
 
-      var frames = framesOfTabs.get(this._tab);
+      var frames = framesOfTabs[this._tabId];
       if (frames)
       {
-        var frame = frames[this._id];
+        var frame = frames[this._frameId];
         if (frame)
           return frame.url;
       }
     },
     get parent()
     {
-      var frames = framesOfTabs.get(this._tab);
+      var frames = framesOfTabs[this._tabId];
       if (frames)
       {
         var frame;
-        if (this._id != null)
-          frame = frames[this._id];
+        if (this._frameId != null)
+          frame = frames[this._frameId];
         else
         {
           // the frame ID wasn't available when we created
@@ -417,13 +241,18 @@
         if (!frame || frame.parent == -1)
           return null;
 
-        return new Frame({id: frame.parent, tab: this._tab});
+        return new Frame({frameId: frame.parent, tabId: this._tabId});
       }
     }
   };
 
 
-  /* Web request blocking */
+  /* Web requests */
+
+  ext.webRequest = {
+    onBeforeRequest: new ext._EventTarget(true),
+    handlerBehaviorChanged: chrome.webRequest.handlerBehaviorChanged
+  };
 
   chrome.webRequest.onBeforeRequest.addListener(function(details)
   {
@@ -433,13 +262,12 @@
     if (details.tabId == -1)
       return;
 
-    var tab = new Tab({id: details.tabId});
-    var frames = framesOfTabs.get(tab);
+    var page = new Page({id: details.tabId});
+    var frames = framesOfTabs[details.tabId];
 
     if (!frames)
     {
-      frames = [];
-      framesOfTabs.set(tab, frames);
+      frames = framesOfTabs[details.tabId] = [];
 
       // assume that the first request belongs to the top frame. Chrome
       // may give the top frame the type "object" instead of "main_frame".
@@ -480,57 +308,18 @@
         frames[details.frameId].parent = frameId;
     }
 
-    var frame = new Frame({id: frameId, tab: tab});
+    var frame = new Frame({frameId: frameId, tabId: details.tabId});
 
-    for (var i = 0; i < ext.webRequest.onBeforeRequest._listeners.length; i++)
-    {
-      if (ext.webRequest.onBeforeRequest._listeners[i](details.url, details.type, tab, frame) === false)
-        return {cancel: true};
-    }
+    if (!ext.webRequest.onBeforeRequest._dispatch(details.url, details.type, page, frame))
+      return {cancel: true};
   }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 
-  /* API */
-
-  ext.windows = {
-    getAll: function(callback)
-    {
-      chrome.windows.getAll(function(windows)
-      {
-        callback(windows.map(function(win)
-        {
-          return new Window(win);
-        }));
-      });
-    },
-    getLastFocused: function(callback)
-    {
-      chrome.windows.getLastFocused(function(win)
-      {
-        callback(new Window(win));
-      });
-    }
-  };
-
-  ext.tabs = {
-    onLoading: new LoadingTabEventTarget(),
-    onCompleted: new CompletedTabEventTarget(),
-    onActivated: new ActivatedTabEventTarget(),
-    onRemoved: new RemovedTabEventTarget(),
-
-    // the "beforeNavigate" event in Safari isn't dispatched when a new URL
-    // was entered into the address bar. So we can only use it only on Chrome,
-    // but we have to hide it from the browser-independent high level code.
-    _onBeforeNavigate: new BeforeNavigateTabEventTarget()
-  };
-
-  ext.webRequest = {
-    onBeforeRequest: new SimpleEventTarget(),
-    handlerBehaviorChanged: chrome.webRequest.handlerBehaviorChanged
-  };
+  /* Context menus */
 
   var contextMenuItems = [];
   var isContextMenuHidden = true;
+
   ext.contextMenus = {
     addMenuItem: function(title, contexts, onclick)
     {
@@ -539,7 +328,7 @@
         contexts: contexts,
         onclick: function(info, tab)
         {
-          onclick(info.srcUrl, new Tab(tab));
+          onclick(info.srcUrl, new Page(tab));
         }
       });
       this.showMenuItems();
@@ -578,5 +367,14 @@
     }
   };
 
-  ext.onMessage = new BackgroundMessageEventTarget();
+
+  /* Message passing */
+
+  ext._setupMessageListener(function(sender)
+  {
+    return {
+      page: new Page(sender.tab),
+      frame: new Frame({url: sender.url, tabId: sender.tab.id})
+    };
+  });
 })();