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

chrome/ext/background.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
- * Copyright (C) 2006-2013 Eyeo GmbH
+ * 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.
  *
@@ skipping 40 matching lines @@
     sendMessage: function(message, responseCallback)
     {
       sendMessage(this._id, message, responseCallback);
     }
   };
 
   ext.pages = {
     open: function(url, callback)
     {
       if (callback)
-        chrome.tabs.create({url: url}, function(tab) { callback(new Page(tab)); });
+      {
+        chrome.tabs.create({url: url}, function(openedTab)
+        {
+          var onUpdated = function(tabId, changeInfo, tab)
+          {
+            if (tabId == openedTab.id && changeInfo.status == "complete")
+            {
+              chrome.tabs.onUpdated.removeListener(onUpdated);
+              callback(new Page(tab));
+            }
+          };
+          chrome.tabs.onUpdated.addListener(onUpdated);
+        });
+      }
       else
         chrome.tabs.create({url: url});
     },
     query: function(info, callback)
     {
       var rawInfo = {};
-      var visibleWindow = null;
-
       for (var property in info)
       {
         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);
-          }));
-        }
+        callback(tabs.map(function(tab)
+        {
+          return new Page(tab);
+        }));
       });
     },
     onLoading: new ext._EventTarget()
   };
 
   chrome.tabs.onUpdated.addListener(function(tabId, changeInfo, tab)
   {
     if (changeInfo.status == "loading")
       ext.pages.onLoading._dispatch(new Page(tab));
   });
 
   chrome.webNavigation.onBeforeNavigate.addListener(function(details)
   {
     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.

 
   chrome.tabs.onRemoved.addListener(function(tabId)
   {
     ext._removeFromAllPageMaps(tabId);
     delete framesOfTabs[tabId];
   });
 
 
   /* Browser actions */
 
   var BrowserAction = function(tabId)
   {
     this._tabId = tabId;
   };
   BrowserAction.prototype = {
     setIcon: function(path)
     {
-      chrome.browserAction.setIcon({tabId: this._tabId, path: path});
+      var paths = {};
+      for (var i = 1; i <= 2; i++)
+      {
+        var size = i * 19;
+        paths[size] = path.replace("$size", size);
+      }
+
+      chrome.browserAction.setIcon({tabId: this._tabId, path: paths});
     },
     setBadge: function(badge)
     {
       if (!badge)
       {
         chrome.browserAction.setBadgeText({
           tabId: this._tabId,
           text: ""
         });
         return;
@@ skipping 76 matching lines @@
 
   /* Web requests */
 
   ext.webRequest = {
     onBeforeRequest: new ext._EventTarget(true),
     handlerBehaviorChanged: chrome.webRequest.handlerBehaviorChanged
   };
 
   chrome.webRequest.onBeforeRequest.addListener(function(details)
   {
-    // the high-level code isn't interested in requests that aren't related
-    // to a tab and since those can only be handled in Chrome, we ignore
-    // them here instead of in the browser independent high-level code.
-    if (details.tabId == -1)
-      return;
-
-    var page = new Page({id: details.tabId});
-    var frames = framesOfTabs[details.tabId];
-
-    if (!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".
-      // https://code.google.com/p/chromium/issues/detail?id=281711
-      if (frameId == 0)
-        details.type = "main_frame";
-    }
-
-    var frameId;
-    if (details.type == "main_frame" || details.type == "sub_frame")
-    {
-      frameId = details.parentFrameId;
-      frames[details.frameId] = {url: details.url, parent: frameId};
-
-      // the high-level code isn't interested in top frame requests and
-      // since those can only be handled in Chrome, we ignore them here
-      // instead of in the browser independent high-level code.
-      if (details.type == "main_frame")
+    try
+    {
+      // the high-level code isn't interested in requests that aren't related
+      // to a tab and since those can only be handled in Chrome, we ignore
+      // them here instead of in the browser independent high-level code.
+      if (details.tabId == -1)
         return;
-    }
-    else
-      frameId = details.frameId;
-
-    if (!(frameId in frames))
-    {
-      // the high-level code relies on the frame. So ignore the request if we
-      // don't even know the top-level frame. That can happen for example when
-      // the extension was just (re)loaded.
-      if (!(0 in frames))
-        return;
-
-      // however when the src of the frame is a javascript: or data: URL, we
-      // don't know the frame either. But since we know the top-level frame we
-      // can just pretend that we are in the top-level frame, in order to have
-      // at least most domain-based filter rules working.
-      frameId = 0;
-      if (details.type == "sub_frame")
-        frames[details.frameId].parent = frameId;
-    }
-
-    var frame = new Frame({frameId: frameId, tabId: details.tabId});
-
-    if (!ext.webRequest.onBeforeRequest._dispatch(details.url, details.type, page, frame))
-      return {cancel: true};
+
+      var page = new Tab({id: details.tabId});
+      var frames = framesOfTabs[details.tabId];
+
+      if (!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".
+        // https://code.google.com/p/chromium/issues/detail?id=281711
+        if (frameId == 0)
+          details.type = "main_frame";
+      }
+
+      var frameId;
+      if (details.type == "main_frame" || details.type == "sub_frame")
+      {
+        frameId = details.parentFrameId;
+        frames[details.frameId] = {url: details.url, parent: frameId};
+
+        // the high-level code isn't interested in top frame requests and
+        // since those can only be handled in Chrome, we ignore them here
+        // instead of in the browser independent high-level code.
+        if (details.type == "main_frame")
+          return;
+      }
+      else
+        frameId = details.frameId;
+
+      if (!(frameId in frames))
+      {
+        // the high-level code relies on the frame. So ignore the request if we
+        // don't even know the top-level frame. That can happen for example when
+        // the extension was just (re)loaded.
+        if (!(0 in frames))
+          return;
+
+        // however when the src of the frame is a javascript: or data: URL, we
+        // don't know the frame either. But since we know the top-level frame we
+        // can just pretend that we are in the top-level frame, in order to have
+        // at least most domain-based filter rules working.
+        frameId = 0;
+        if (details.type == "sub_frame")
+          frames[details.frameId].parent = frameId;
+      }
+
+      var frame = new Frame({id: frameId, tabId: details.tabId});
+
+      if (!ext.webRequest.onBeforeRequest._dispatch(details.url, details.type, page, frame))
+        return {cancel: true};
+    }
+    catch (e)
+    {
+      // recent versions of Chrome cancel the request when an error occurs in
+      // the onBeforeRequest listener. However in our case it is preferred, to
+      // let potentially some ads through, rather than blocking legit requests.
+      console.error(e);
+    }
   }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 
   /* Context menus */
 
   var contextMenuItems = [];
   var isContextMenuHidden = true;
 
   ext.contextMenus = {
     addMenuItem: function(title, contexts, onclick)
@@ skipping 45 matching lines @@
 
   /* Message passing */
 
   ext._setupMessageListener(function(sender)
   {
     return {
       page: new Page(sender.tab),
       frame: new Frame({url: sender.url, tabId: sender.tab.id})
     };
   });
+
+
+  /* Storage */
+
+  ext.storage = localStorage;
 })();