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

ext/background.js

Index: ext/background.js
===================================================================
new file mode 100644
--- /dev/null
+++ b/ext/background.js
@@ -0,0 +1,65 @@
+/*
+ * This file is part of Adblock Plus <http://adblockplus.org/>,
+ * Copyright (C) 2006-2013 Eyeo GmbH

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

Note: Please update this when rebasing.

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

Done.

+ *
+ * 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()
+{
+  var nonEmptyPageMaps = {__proto__: null};

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

Note: we will leak every PageMap object ever created so the assumption here is
clearly that PageMap objects are always persistent. That issue was already
present before that change however, and the assumption currently seems to be
true - still non-obvious however and worth a comment.

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

We only keep track of non-empty PageMaps, and pages are automatically removed
from the PageMap, when tabs are closed, reloading, or navigating to a different
URL.

So it is safe to discard PageMap objects without leaking any memory, if you
clear them before. And even if you don't, the leak will be just temporary, for
as long as the contained pages are open.

+  var pageMapCounter = 0;
+
+  var PageMap = ext.PageMap = function()
+  {
+    this._map = {__proto__: null};
+    this._id = ++pageMapCounter;
+  };
+  PageMap.prototype = {
+    _delete: function(id)
+    {
+      delete this._map[id];
+
+      if (Object.keys(this._map).length == 0)
+        delete nonEmptyPageMaps[this._id];

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

This isn't really great from the performance point of view, an empty PageMap
also isn't exactly a common scenario. How about dropping the "non empty"
constraint and simply tracking all maps (in a regular array then)?

+    },
+    get: function(page)
+    {
+      return this._map[page._id];
+    },
+    set: function(page, value)
+    {
+      this._map[page._id] = value;
+      nonEmptyPageMaps[this._id] = this;
+    },
+    has: function(page)
+    {
+      return page._id in this._map;
+    },
+    clear: function()
+    {
+      for (var id in this._map)
+        this._delete(id);
+    },
+    delete: function(page)
+    {
+      this._delete(page._id);
+    }
+  };
+
+  ext._removeFromAllPageMaps = function(pageId)
+  {
+    for (var pageMapId in nonEmptyPageMaps)
+      nonEmptyPageMaps[pageMapId]._delete(pageId);
+  };
+})();