Closed Bug 1564164 Opened 5 years ago Closed 5 years ago

[jsdbg2] Split up Debugger.cpp

Categories

(Core :: JavaScript Engine, task, P3)

Product:
Core
Component:
JavaScript Engine
Type:
task

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jimb, Assigned: jimb)

References

(Blocks 1 open bug)

Details

(Whiteboard: [debugger-mvp])

Attachments

(8 files, 2 obsolete files)

Bug 1564164: Move Debugger.cpp, DebuggerMemory.cpp, and related files into js/src/dbg. r?jorendorff
(deleted), text/x-phabricator-request
Details
Bug 1564164: Move EnterDebuggeeNoExecute and LeaveDebuggeeNoExecute into their own header, dbg/NoExecute.h. r?jorendorff
(deleted), text/x-phabricator-request
Details
Bug 1564164: Move DebuggerObject into its own file, js/src/dbg/Object.cpp. r?jorendorff
(deleted), text/x-phabricator-request
Details
Bug 1564164: Move DebuggerFrame into its own file, js/src/dbg/Frame.cpp. r?jorendorff
(deleted), text/x-phabricator-request
Details
Bug 1564164: Move DebuggerEnvironment into its own file, js/src/dbg/Environment.cpp. r?jorendorff
(deleted), text/x-phabricator-request
Details
Bug 1564164: Let all DebuggerFoo::initClass methods take their arguments in a consistent order. r?jorendorff
(deleted), text/x-phabricator-request
Details
Bug 1564164: Move DebuggerScript into its own file, js/src/dbg/Script.cpp. r?jorendorff
(deleted), text/x-phabricator-request
Details
Bug 1564164: Move DebuggerSource into its own file. r?jorendorff
(deleted), text/x-phabricator-request
Details

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

Depends on D37508

Attached patch Rebased version of D37334. (obsolete) (deleted) — — 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: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: