Closed Bug 1602699 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: JavaScript Engine, task, P3)

Product:
Core
Component:
JavaScript Engine
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla73
Tracking Flags:
Tracking Status
firefox73 --- fixed

People

(Reporter: loganfsmyth, Assigned: loganfsmyth)

References

Details

Attachments

(9 files)

Bug 1602699 - Part 1: Convert reportUncaughtException to return void. r=jimb!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 2: Change DebugAPI::onDebuggerStatement force-return to work via an error. r=jimb!,jandem!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 3: Change DebugAPI::onSingleStep force-return to work via an error. r=jimb!,jandem!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 4: Change DebugAPI::onTrap force-return to work via an error. r=jimb!,jandem!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 5: Change DebugAPI::onEnter/ResumeFrame to work via an error. r=jimb!,jandem!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 6: Change DebugAPI::onExceptionUnwind to work via an error. r=jimb!,jandem!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 7: Scope PropagateForcedReturn to just Debugger.cpp. r=jimb!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 8: Make ResumeMode a Debugger-internal enum. r=jimb!
(deleted), text/x-phabricator-request
Details
Bug 1602699 - Part 9: Centralize resumption value completion more. r=jimb!
(deleted), text/x-phabricator-request
Details

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.

Attachment

General

Creator:
Created:
Updated:
Size: