Key equality of ES6 Map behaves oddly in Chrome content script

Question

In my content script, I use a Map to keep track of all the opened popup windows. The key-value pairs in the Map are constructed as follows:

• key - The reference returned by Window.open()
• value - Some associated data

The problem is, Map.prototype.has() and Map.prototype.get() sometimes return unexpected results.

// content.js

let map = new Map();
let popup = window.open('https://www.google.com');
let data = {};

map.set(popup, data);

// retrieve data later
window.setTimeout(() => {

  // should return true, but sometimes return false
  console.log(map.has(popup));

  // should return {}, but sometimes return undefined
  console.log(map.get(popup));

}, 3000);

It seems that the added key and the reference popup are not always considered "equal" for some reason. And this ambiguous situation appears to exist in the content script only. If the above code is executed in the browser's console instead, then map.has() and map.get() will always return the correct values.

So my questions are: Why this happened? Was it caused by some content script's underlying mechanism I wasn't aware of?

Answer 1

I can reproduce this (Chrome 60.0.3112.78, Windows 10, 64-bit), and reported it as a bug. Well done for discovering this!

The bug was unwittingly fixed in Chrome 62.0.3175.2 (canary), but I constructed a new demo using WeakMap that still fails (https://jsfiddle.net/769a0qmu):

var map = new WeakMap();
var popup = window.open("https://google.com"); // requires popups to be enabled
map.set(popup, "data");
window.setTimeout(function() {
  console.log(map.get(popup)); // expect: "data"
  map.set(popup, "modified");
  console.log(map.get(popup)); // expect: "modified"
}, 3000);

In Chrome 62.0.3175.2 (canary) the output is

data
data

Explanation from bug report

Comment 10 by adamk:

The issue in V8 seems to be that globals created for a remote context return an empty string for %_ClassOf(). This causes us to go down the wrong path when looking for hash codes for such objects.

Comment 12 by adamk:

Here's what's going on inside V8. First, note from the original stack overflow report that this used to be broken in Map, but isn't anymore. Map and WeakMap used to use exactly the same logic to get the hash code for an object. That code looks like this (it's written in internal v8 JS):

function GetExistingHash(key) {
  if (IS_RECEIVER(key) && !IS_PROXY(key) && !IS_GLOBAL(key)) {
    var hash = GET_PRIVATE(key, hashCodeSymbol);
    return hash;
  }
  return %GenericHash(key);
}

Note the IS_GLOBAL(key). That's a macro which expands to %_ClassOf(key) == 'global'. Since the RemoteContext-created global object returns empty string, it fails this check and so we incorrectly try to use the normal object mechanism for dealing with hashcodes, rather than the Global-specific logic (which is handled in the C++ runtime function %GenericHash().

Over the last couple of months, v8 has moved its implementation of the Map methods out of JS, which means they're no longer sharing the above logic with WeakMap. In particular, the decision about how to find the hash code always goes directly to %GenericHash, which uses a different mechanism (an instance type check) on the key to see how to handle the hash code. This method gets the right answer for the RemoteContext case, and thus uses the special hash field on JSGlobalProxy (source), as intended.

All that said, there's the question of what to do to fix the WeakMap case. There's ongoing work inside v8 that will eventually make this go away (analogous to how Map got fixed), but that's probably on the order of weeks until it's fixed. If we'd like a fix sooner, we could try to hack things to make %_ClassOf return "global", but it's not yet clear to me how much work that'd be.

Given that this has been broken for two stable releases already, I'm inclined to just wait for it to be fixed on the v8 side. I've tentatively lowered priority and marked this as blocked on the v8 bug, but I'm open to doing something sooner if it seems worthwhile.

Update (2017-08-24):

The bug has been marked as fixed.