Issue 5264 - Return page objects instead of page IDs

ext/background.js

Index: ext/background.js
===================================================================
--- a/ext/background.js
+++ b/ext/background.js
@@ -30,17 +30,17 @@
     {
       this._map.delete(id);
 
       if (this._map.size == 0)
         nonEmptyPageMaps.delete(this);
     },
     keys()
     {
-      return this._map.keys();
+      return Array.from(this._map.keys()).map(ext.getPage);

[kzar, 2017/05/23 11:47:59]

I wonder if you could do `return Array.prototype.map.call(this._map.keys(),
ext.getPage)`?

[Sebastian Noack, 2017/05/23 12:16:54]

Please don't. Array generics are non-standard and deprecated:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

[Manish Jethani, 2017/05/23 12:32:27]

I think Dave meant Array.prototype.map rather than Array.map. The former isn't
going anywhere, though for better or worse it won't work with a non-array-like
object.

To map an iterable regardless of its type you have to use a function like this: 

https://codereview.adblockplus.org/29417597/patch/29417598/29417600

If you have generators support, then this: 

function* mapIterable(iterable, callback)
{
  for (let entry of iterable)
    yield callback(entry);
}

Here we don't have too many keys though, it's OK to create an intermediate
array.

[Sebastian Noack, 2017/05/23 15:09:18]

Sorry, you are right, I mixed things up, calling array method's through the
prototype isn't generally an issue, but as you pointed out it only works with
array-like objects.

I wouldn't be opposed to make keys() a generator function. All browsers this
code is running on support generators, and it doesn't seem to make the code any
more complicated. However, this would imply that there is no usage of keys()
which access the result like an array, I didn't check this.

Alternatively, we could create an array like below:

  let pages = [];
  for (let key of this._map.keys())
    pages.push(ext.getPage(key));
  return pages;

In the end it doesn't matter much, so as far as I'm concerned, just pick the
approach that seems best to you.

[kzar, 2017/05/23 16:15:04]

Acknowledged.

     },
     get(page)
     {
       return this._map.get(page.id);
     },
     set(page, value)
     {
       this._map.set(page.id, value);