Closed Bug 1564164 Opened 1 year ago Closed 1 year ago

[jsdbg2] Split up Debugger.cpp

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED

People

(Reporter: jimb, Assigned: jimb)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [debugger-mvp])

Attachments

(8 files, 2 obsolete 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

The Debugger implementation is currently a single 428kiB, 13kloc file, js/src/vm/Debugger.cpp. Phabricator refuses to colorize the code for patch review due to its size. It's difficult to find the code one is looking for.

The implementation should be relocated to its own directory, js/src/dbg, and broken up into separate files. There would ideally be one file per Debugger API object type (Debugger, Debugger.Object, Debugger.Script, and so on). At first, Debugger.h could remain a single file, but as separable pieces are identified, it could be broken up. Other files for shared functionality could be pulled out as needed.

A few other files in js/src/vm that probably also belong in js/src/dbg: DebuggerMemory.cpp, and perhaps the UbiNode... files.

Blocks: dbg-70
Priority: -- → P3
Whiteboard: [debugger-mvp]
Assignee: nobody → jimb
Status: NEW → ASSIGNED
Attachment #9076667 - Attachment description: Bug 1564164: Move Debugger.cpp, DebuggerMemory.cpp, and related files into js/src/dbg. → Bug 1564164: Move Debugger.cpp, DebuggerMemory.cpp, and related files into js/src/dbg. r?jorendorff
Keywords: leave-open
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e59a0e725e9f
Move Debugger.cpp, DebuggerMemory.cpp, and related files into js/src/dbg. r=jorendorff
Attached patch Rebased version of D37334. (obsolete) — Splinter Review

Here's a version of D37334 rebased on top of the changes to js/src/vm/Debugger.cpp and Debugger.h in central.

Comment on attachment 9076984 [details]
Bug 1564164: Give Debugger.Script instances their own NativeObject subclass, DebuggerScript.

Revision D37509 was moved to bug 1564170. Setting attachment 9076984 [details] to obsolete.

Attachment #9076984 - Attachment is obsolete: true
Attachment #9076992 - Attachment is obsolete: true
Attachment #9076668 - Attachment description: Bug 1564164: Move EnterDebuggeeNoExecute and LeaveDebuggeeNoExecute into their own header, dbg/NoExecute.h. → Bug 1564164: Move EnterDebuggeeNoExecute and LeaveDebuggeeNoExecute into their own header, dbg/NoExecute.h. r?jorendorff
Attachment #9076669 - Attachment description: Bug 1564164: Move DebuggerObject into its own file, js/src/dbg/Object.cpp. → Bug 1564164: Move DebuggerObject into its own file, js/src/dbg/Object.cpp. r?jorendorff
Attachment #9076710 - Attachment description: Bug 1564164: Move DebuggerFrame into its own file, js/src/dbg/Frame.cpp. → Bug 1564164: Move DebuggerFrame into its own file, js/src/dbg/Frame.cpp. r?jorendorff
Attachment #9076717 - Attachment description: Bug 1564164: Move DebuggerEnvironment into its own file, js/src/dbg/Environment.cpp. → Bug 1564164: Move DebuggerEnvironment into its own file, js/src/dbg/Environment.cpp. r?jorendorff
Attachment #9076983 - Attachment description: Bug 1564164: Let all DebuggerFoo::initClass methods take their arguments in a consistent order. → Bug 1564164: Let all DebuggerFoo::initClass methods take their arguments in a consistent order. r?jorendorff
Attachment #9077017 - Attachment description: Bug 1564164: Move DebuggerScript into its own file, js/src/dbg/Script.cpp. → Bug 1564164: Move DebuggerScript into its own file, js/src/dbg/Script.cpp. r?jorendorff
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/89e578d07137
Move EnterDebuggeeNoExecute and LeaveDebuggeeNoExecute into their own header, dbg/NoExecute.h. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/6c763326b518
Move DebuggerObject into its own file, js/src/dbg/Object.cpp. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/10ac90913cdf
Move DebuggerFrame into its own file, js/src/dbg/Frame.cpp. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/530d71d677d5
Move DebuggerEnvironment into its own file, js/src/dbg/Environment.cpp. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/5fe29dc3c21e
Let all DebuggerFoo::initClass methods take their arguments in a consistent order. r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/0f741c61e16b
Move DebuggerScript into its own file, js/src/dbg/Script.cpp. r=jorendorff
Regressions: 1567117

This adds js/src/dbg/Source.{cpp,h}, and moves DebuggerSource's definition
there. No intended change in implementation or visible behavior.

Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2c345c044ca1
Move DebuggerSource into its own file. r=jorendorff
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.