[resize-observer] ResizeObserver callback order.

emilio commented 4 years ago

The specification has a well-defined callback order, however it requires keeping alive observers for too long (see this firefox bug).

It seems that WebKit is doing the same thing I'm going to do in Gecko (add to the Document.resizeObservers slot on observe(), and remove on the last unobserve() call).

It seems Blink uses a hash set so I don't even know if the iteration order there is well defined for them.

I'm going to align with WebKit for now as it seems simpler to implement and doesn't create an insert-only list / set.

Doing this creates an issue, which is that the observers can be removed when delivering observations. This was already a problem for additions, but explicitly copying the contents of the slot before iteration in the spec may be clearer.

cc @smfr @atotic

emilio commented 4 years ago

Test-case for the HashSet issue in Blink:

<!doctype html>
<style>
  div {
    width: 100px;
    height: 100px;
    background: green;
  }
</style>
<div id="observeme"></div>
<script>
  let div = document.querySelector("#observeme");
  for (let i = 0; i < 100; ++i) {
    new ResizeObserver(function() {
      console.log(i);
    }).observe(div);
  }
</script>

The following example should output all the numbers from 0 to 99. In blink order is pretty random. That's clearly a bug.

atotic commented 4 years ago

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=1026663 Chrome will fix this.

Loirooriol commented 2 years ago

var ro1 = new ResizeObserver(() => console.log("1"));
var ro2 = new ResizeObserver(() => console.log("2"));
ro2.observe(document.body);
ro1.observe(document.body);

WebKit and Gecko: 2, 1 Blink and spec: 1, 2

https://bugzilla.mozilla.org/show_bug.cgi?id=1596992#c10 explains how Blink did it without leaks.

w3c / csswg-drafts

[resize-observer] ResizeObserver callback order. #4518