1867668 - Allow using Debugger onNativeCall for web page executions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task)

Description

[Alexandre Poirot [:ochameau]]

In order to implement bug 1835089, I would need to revisit a bit the behavior of Debugger API's onNativeCall.

The current behavior of it was especially designed to implement eager evaluation and comes with lots of specifics for this sole usage (bug 1576776, bug 1561424, bug 1602489).

Two things need to change:

1. For now onNativeCall is only called when a Debugger evaluates (so you can't record web page code, only debugger evaluations)
2. Evaluations made by a debugger doesn't trigger other debugger hooks, and especially not their onNativeCall.

For now these two behavior are both implemented around JSContext::insideDebuggerEvaluationWithOnNativeCallHook.
This flag will need to be revised in two distinct features and flags.

For 1), it is easy. Instead of gating onNativeCall calls on insideDebuggerEvaluationWithOnNativeCallHook being set by AutoNoteDebuggerEvaluationWithOnNativeCallHook only when Debugger evaluates, we should more simply gate it on Debugger having the onNativeHook set. For example rely on JSRealm::debuggerObservesAllExecution pattern which ultimately relies on hooks being set on Debugger instances. We would simply need a debuggerObservesNativeCall equivalent.
Doing this would remove most usages of JSContext::insideDebuggerEvaluationWithOnNativeCallHook

For 2), we need to revisit Debugger::isHookCallAllowed, which would be the last usages of JSContext::insideDebuggerEvaluationWithOnNativeCallHook.
This flag is set by AutoNoteDebuggerEvaluationWithOnNativeCallHook when evaluating JS from a Debugger. Today it is only set when the debugger observes native calls:
https://searchfox.org/mozilla-central/rev/d84469b005106c5ab0f65e283f71c1415ce76c54/js/src/debugger/Frame.cpp#1175-1178

  // Note whether we are in an evaluation that might invoke the OnNativeCall
  // hook, so that the JITs will be disabled.
  AutoNoteDebuggerEvaluationWithOnNativeCallHook noteEvaluation(
      cx, dbg->observesNativeCalls() ? dbg : nullptr);

It would be more flexible if the Debugger had an explicit flag, like exclusiveDebuggerOnEval which would control this behavior and decouple it from onNativeCall hook. We would keep the check in isHookCallAllowed:

inline bool js::Debugger::isHookCallAllowed(JSContext* cx) const {
  // If we are evaluating inside of an eval on a debugger that has an
  // onNativeCall hook, we want to _only_ call the hooks attached to that
  // specific debugger.
  return !cx->insideDebuggerEvaluationWithOnNativeCallHook ||
         this == cx->insideDebuggerEvaluationWithOnNativeCallHook;
}

But this would be controlled by this new flag manually set on Debugger which do not want to trigger the other Debuggers.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1867668 - [devtools] Decouple onNativeCall from Debugger evaluation/calls.

Instead of limiting Debugger onNativeCall hook to the debugger evaluation,
allow observing any code running, especially including the page executions.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1867668 - [devtools] Distinguish exclusive debugger on evaluation versus observing native calls.

For now, the two cases were using the same flag 'insideDebuggerEvaluationWithOnNativeCallHook',
but it would be benefitial to distinguish the two features:

1. have a debugger to have exclusive observation of its own evaluation and calls (exlusiveDebuggerOnEval),
2. observe native call and requires to disable JIT and enable slow path related to this observation (onNativeCall/observesNativeCall).

The previous patch addressed 2), but now it would be nice to have an explicit flag to say
that a given Debugger evaluation should avoid trigerring any other Debugger's hooks.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e42b5b31b58d
[devtools] Decouple onNativeCall from Debugger evaluation/calls. r=arai
https://hg.mozilla.org/integration/autoland/rev/97f759dedbc9
[devtools] Distinguish exclusive debugger on evaluation versus observing native calls. r=arai,devtools-reviewers,nchevobbe

Comment 4

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/e42b5b31b58d
https://hg.mozilla.org/mozilla-central/rev/97f759dedbc9