1625069 - Move eager-eval's effectful offset and maybe timeout handling into C++ and skip stepping-related hooks

Status: NEW

(DevTools :: Console, enhancement, P2)

Description

[Logan Smyth [:loganfsmyth]]

This doesn't need to strictly block eager-eval, but I do think that the current state of things, especially due the patches needed for https://bugzilla.mozilla.org/show_bug.cgi?id=1621572, has left things in a bit of an ugly state for the low-level API we use.

Eager eval is currently implemented using SpiderMonkey Debugger API's "hook" system, which has worked well as a way to prototype this out, but we end up with a few hard-to-answer questions.

What low-level hooks should run during eager evaluation? Does running an onStep hook count as a side-effect? If there are multiple Debugger objects monitoring a realm and one does a side-effect-free evaluation, should other Debugger objects be notified via their hooks? They have no way to know that the evaluation is meant to be side-effect-free, which caused https://bugzilla.mozilla.org/show_bug.cgi?id=1621572, so now we restrict eager-eval to only run the hooks for the Debugger doing the evaluation, but that is still weird.

It seems like in an ideal world, an eager eval would run no hooks, except to call the function to validate a given native function. The onNativeCall approach to that would then need to change to instead be a callback passed directly to the debugger's eval functions, but that seems more in keeping with the purpose of the function anyway, especially since onNativeCall was already restricted to only run for the debugger performing the eval, independent of anything else.

That might for instance leave us with an API more like

frame.evalWithBindings("code;", {}, {
  timeout: 100,
  validateNative: function(callee, reason) {
   // do current onNativeCall logic
  },
});

which would not run onNewScript for scripts introduced within the evaled source, and skips all frame-related stepping hooks like onEnter/Step/Pop, or other stuff like onExceptionUnwind/onDebuggerStatement. I haven't dug into whether we'd want to skip other hooks like onNewPromise, but maybe we'd want to just terminate if one of those hooks is used? There is also the question of whether scripts introduced during an eager evaluation should show up via findScripts in the API.

We'd also want to explicitly have it so that during evaluation of validateNative or any hooks that we do decide to dispatch, we exit the "eager" scope to avoid treating those as abort-able.