Closed Bug 1602699 Opened 2 months ago Closed 1 month ago

[jsdbg2] Convert DebugAPI hook calls to use propagateForcedReturn instead of ResumeMode

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: loganfsmyth, Assigned: loganfsmyth)

References

(Blocks 2 open bugs)

Details

Attachments

(9 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

I've been looking this over a bunch and as far as I can tell, this will dramatically simplify a lot of code.

SpiderMonkey supports a way to have the error handler (JS exception, or execution termination) to jump to the end of a given function. It currently uses this to implement

  • PropagatingForcedReturn allows the debugger to force-return a value in the specific context of a SpiderMonkey interrupt handler
  • .return on generators, to force-return a value from a generator when it is suspended at a yield.

We already use this logic to implement forced-return for the debugger in one place, and if we used it consistently across all of the hook implementations that need to force-return values, we could eliminate a ton of places that have to use the ResumeMode enum, and in fact it will allow us to make the ResumeMode enum entirely an internal detail of the debugger itself, so we can remove it from DebugAPI (kind of, we still need something like it for onNativeCall).

In turn, relying on this should allow us to start simplifying the logic for how we handle completion values within the debugger itself in https://bugzilla.mozilla.org/show_bug.cgi?id=1602429

Priority: -- → P3

This function is used in a bunch of places, some of which will return
false instead of ResumeKind::Terminate in later parts of this patch set.
To simplify things, I figured removing the return value entirely would be
the easiest and simplest change to make.

Blocks: 1603920
Pushed by loganfsmyth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d64b4042eb5a
Part 1: Convert reportUncaughtException to return void. r=jimb
https://hg.mozilla.org/integration/autoland/rev/96c9ae162472
Part 2: Change DebugAPI::onDebuggerStatement force-return to work via an error. r=jimb,jandem
https://hg.mozilla.org/integration/autoland/rev/0340540c6470
Part 3: Change DebugAPI::onSingleStep force-return to work via an error. r=jimb,jandem
https://hg.mozilla.org/integration/autoland/rev/4cdc7061129b
Part 4: Change DebugAPI::onTrap force-return to work via an error. r=jimb,jandem
https://hg.mozilla.org/integration/autoland/rev/b2ad424d4882
Part 5: Change DebugAPI::onEnter/ResumeFrame to work via an error. r=jimb,jandem
https://hg.mozilla.org/integration/autoland/rev/46b011b3ac0f
Part 6: Change DebugAPI::onExceptionUnwind to work via an error. r=jimb,jandem
https://hg.mozilla.org/integration/autoland/rev/8ea0a6fda65d
Part 7: Scope PropagateForcedReturn to just Debugger.cpp. r=jimb
https://hg.mozilla.org/integration/autoland/rev/0141d065fe28
Part 8: Make ResumeMode a Debugger-internal enum. r=jimb
https://hg.mozilla.org/integration/autoland/rev/cac7612e534b
Part 9: Centralize resumption value completion more. r=jimb
You need to log in before you can comment on or make changes to this bug.