"Upgrade an element" step 4 should remove element, or an already constructed marker

[dominiccooney]

I'm trying to implement the HTMLElement constructor in Blink. Step 8 replaces an element with an already constructed marker. I don't see anywhere that removes already constructed markers from the stack, which could be a problem.

I think upgrade an element step 4 may intend to simply pop from the construction stack instead of popping element; in the common case the top of the stack will not be element but an already constructed marker. Maybe already constructed markers should be associated with the element that caused them, and step 4 can pop element or its associated already constructed marker.

[domenic]

Yes, I think you're exactly right. Looking back on @rniwa's original algorithm in WICG/webcomponents#403, he said "Pop the entry from the element construction stack of DEFINITION." In my translation of that into the spec, I assumed it was element, not realizing that the construction process would replace it with an already-constructed marker.

Maybe already constructed markers should be associated with the element that caused them, and step 4 can pop element or its associated already constructed marker.

I don't think that'll be quite necessary. We just want to pop the top element of the stack in any case. I'll add a note clarifying that in non-pathological cases this will be an already-constructed marker.

PR incoming.

[dominiccooney]

I'm trying to implement this is Blink. I'm glad there is now popping somewhere. Could you sanity check me on the following observation?

I believe by popping the construction stack in upgrade, the spec prevents reentering a constructor of a given type, during upgrade. The reason is subsequent constructor calls, with new or through upgrade (for example, innerHTML) will reach step 5 of HTMLElement constructor. To put it another way, popping in upgrade “protects” the time between the UA calling the author's constructor and super getting to run; but in addition it extends protection to the author's constructor body, too, because the popping happens after that.

While this is a severe restriction I believe this is probably the only sane situation to prevent “poaching.”

One interesting wrinkle this causes is that calling the same constructor, whether with new or upgrade, is allowed during a call caused by "new" because the construction stack is empty. This is not a particular problem for our implementation, but it is an asymmetry for the author. I believe they can detect whether a call is via new or upgrade this way:

class X extends HTMLElement() {
  constructor(opt_in_detect) {
    super();
    console.log('in upgrade? ', inUpgrade(opt_in_detect));
  }
  inUpgrade(opt_in_detect) {
    if (opt_in_detect) throw 'OK';
    else try {
      new X(true);
    } catch (e) {
      return e != 'OK';
    }
    // unreached
  }
}

…but I’m not sure. :)

[dominiccooney]

Actually I take that back; poaching is still possible by calling before super. In that case, maybe we must live with the problem and could relax the restriction.

[domenic]

This all looks very familiar. Did you see my revised commit over in #1242, as well as my comment #1242 (comment) and my question to you about whether we should tighten the restriction?

[dominiccooney]

I had not seen it yet—behind on email. #1242 looks good and addresses this issue.