Should we be doing prepare to run a script/clean up after running a script more often? #855
Comments
|
Is this a problem for the new window/location security and structured clone algorithms too? There's various bugs against IDL that touch upon this: |
Yes. But I think only for entry settings object tracking, not for microtask-running, per my "In some cases" / "In all cases" paragraphs. |
|
I don't have obvious thoughts other than the fact that yes, before we go off and do any ES stuff we need to be managing the script settings object stack (or entry settings, or whatever) somehow. |
|
@bzbarsky, can you confirm for me that we do not need to do the "prepare to run a callback" stuff (which manages the incumbent mechanisms) unless we are saving a callback and calling it later? E.g. when structured cloning we do a few GetOwnProperty()s. Do we need to manage the backup incumbent settings object stack and increment the skip-when-determining-incumbent counter in those cases? I think that since we are immediately invoking code, those mechanisms are not relevant. But I'd appreciate confirmation. |
|
I believe this is correct, unless you are purposefully trying to change the entry or incumbent settings for your callee (and offhand I can't think of obvious cases where you do). Put another way, it's no different than what happens for |
|
Hmm, so I guess the worrying cases are when the algorithm might be called from outside JS. For example, However, StructuredClone is a bit worrying to me as I think it is sometimes called from UA code. All uses in the HTML spec seem to invoke StructuredClone or StructuredCloneWithTransfer from inside a called-from-JS method, so those are OK. But URL parsing invokes structured clone for blob URLs. I guess it will only clone blobs though in that case so things are fine? Hmm. I think my conclusion so far is:
|
|
@bzbarsky can you take a look at #855 (comment) and make sure I am on the right track? |
Well, its redundant if we don't want to change the entry settings we're operating with. Which may well be the case. The rest sounds totally fine. |
This ensures that the incumbent global is correctly set up when calling them. We also remove the redundant "prepare to run script" and "clean up after running script" in the element definition algorithm, since element definition happens synchronously from user code. Part of #855.
This closes #855 by adding explicit guidance for those using structured clone to ensure they properly set up entry and incumbent globals. It also adds some additional helpful scaffolding, such as adding an implicit conversion between Web IDL types and JS types.
This ensures that the incumbent global is correctly set up when calling them. We also remove the redundant "prepare to run script" and "clean up after running script" in the element definition algorithm, since element definition happens synchronously from user code. Part of #855.
This ensures that the incumbent global is correctly set up when calling them. We also remove the redundant "prepare to run script" and "clean up after running script" in the element definition algorithm, since element definition happens synchronously from user code. Part of whatwg#855.
This closes whatwg#855 by adding explicit guidance for those using structured clone to ensure they properly set up entry and incumbent globals. It also adds some additional helpful scaffolding, such as adding an implicit conversion between Web IDL types and JS types.
Now that we understand this issue better (see below for how we got there), here's a checklist of known places in HTML that should be doing this better.
In all cases, they need to be appropriately wrapped with prepare to run script to correctly set up the entry global, and I believe also prepare to run a callback to correctly set up the incumbent global. (The latter especially if there are any cases of callbacks being "saved" and called later, but maybe in general?)
Other specs are welcome to join this checklist.
Original post:
When speccing custom elements, which do their fair share of Get()s and Construct()s, I realized that currently we do those sorts of things and bypass the following hooks:
In all cases this impacts tracking of the entry settings object and JS execution context stack. (See, however, #473 for how I messed that up a bit and still need to fix it.)
In some cases, it would also cause microtasks (and "global script clean-up jobs") to run. This would be the case for e.g. constructing custom elements from the parser, where there's a clean stack, but it would not be the case for createElement() or for structured clone, where there's some user code in the stack.
It seems likely to me that we need to wrap all our Get()s, Construct()s, etc. in appropriate check/prepare/cleanup steps. (Even GetOwnProperty()s are affected if proxies are involved.)
When we use Web IDL, this is taken care of for us by the "invoke" algorithm. (Except for the fact that the invoke algorithm isn't up to date with last year's changes to HTML, i.e. the script settings object stack, but that's not a new problem...)
@ajklein says he thinks he filed a bug on this a long time ago against either HTML or Web IDL. I didn't remember seeing it in the HTML archives, so I'm creating a new one. @bzbarsky presumably has thoughts here too, which I'd love to hear.
I'd probably solve this by introducing a one- or two-word verb like "invoke" that links to the appropriate wrapper steps, so that instead of saying "Let x be Get(y, z)" we can do "Let x be the result of [invoking] Get(y, z)."The text was updated successfully, but these errors were encountered: