1145262 - Modularize debugger frontend

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[James Long (:jlongster)]

As part of a Q1 goal to being cleaning up the debugger some, let's start by making things more componentized and removing the globals. I think this might be pretty painless, but I could be wrong.

Comment 1

[James Long (:jlongster)]

Renaming because I'm not going to do as much refactoring as I originally thought. The most important thing right now is to split up the views into separate files and change some of the global references to local where it's easy. There are still a bunch of global variables but that's ok for now.

Comment 2

[James Long (:jlongster)]

Attachment: 1145262.patch

Comment 3

[James Long (:jlongster)]

Comment on attachment 8587026 [details] [diff] [review]
1145262.patch

This basically just splits up debugger-panes.js into multiple files (1 per view). I'd like to also do this to debugger-toolbar.js.

It's a lot more files, which is annoying because of jar.mn, but I find it a whole lot easier to navigate and we can start treating each view more modularly so they are easier to improve individually.

Comment 4

[James Long (:jlongster)]

Previously I wanted to re-use our module system but that requires a bunch of refactoring to pass window/document instances around. It pains me that we don't have modules yet, but this is a really good start at least.

Comment 5

[James Long (:jlongster)]

victor, I'm going on PTO for the rest of the week but it would be nice to land this when I get back, as it will bitrot quickly. if you have time!

Comment 6

[James Long (:jlongster)]

Attachment: 1145262.patch

Forgot to add the license text to the new files

Comment 7

[James Long (:jlongster)]

Attachment: 1145262-panes.patch

also forgot to add strict mode to the new files. I'm going to post another patch which breaks up debugger-toolbar.js in a similar fashion.

Comment 8

[James Long (:jlongster)]

Attachment: 1145262-toolbar.patch

Victor, don't worry if you can't get to these this week. I can get Eddy to take a look next week.

Note that these are only incremental restructurings, it feels a little weird how we are passing DebuggerView and DebuggerController instances around but this is just a first step towards making dependencies more explicit. Please keep in mind this a first step. Landing things in pieces is the only way this will work.

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8591891 [details] [diff] [review]
1145262-panes.patch

Review of attachment 8591891 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like a mechanical change to me.

rs=me with green try push

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8591894 [details] [diff] [review]
1145262-toolbar.patch

Review of attachment 8591894 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me with green try push

Comment 11

[James Long (:jlongster)]

I thought I had done a try push, but I can't find it anymore, so here's a new one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0700aea454ec

Comment 12

[James Long (:jlongster)]

Green try, let's try to get it checked in!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing.

Comment 14

[James Long (:jlongster)]

Attachment: 1145262-panes.patch

Comment 15

[James Long (:jlongster)]

Attachment: 1145262-toolbar.patch

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/972e2aea8912
https://hg.mozilla.org/integration/fx-team/rev/28cf16c94de5

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/972e2aea8912
https://hg.mozilla.org/mozilla-central/rev/28cf16c94de5