Replace the ThreadActor's private debugger object and its getter with a public property

RESOLVED FIXED in Firefox 18

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: past, Assigned: anton)

Tracking

Trunk
Firefox 18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=past][lang=js])

Attachments

(1 attachment)

No need for this complication. See also bug 783393 comment 48.
Whiteboard: [good first bug][mentor=past][lang=js]

Comment 1

5 years ago
Sir,
I would like to contribute to this bug, Will you explain the bug in detail ?
(In reply to Amod from comment #1)
> Sir,
> I would like to contribute to this bug, Will you explain the bug in detail ?

Yes, thank you. In toolkit/devtools/debugger/server/dbg-script-actors.js there is a _dbg property on the ThreadActor prototype as well as a dbg getter for that property. This distinction has never been of much use in practice, so for simplicity, we would like to just have a plain dbg property on the prototype, without a getter. 

This is pretty much a search-and-replace job, but you will also need to run the xpcshell tests and the mochitests to be sure nothing is broken after this change. The locations of the tests are:

toolkit/devtools/debugger/tests/unit/
browser/devtools/debugger/test/

Pop in the #devtools channel in IRC (I'm past) if you need any help with the above.
To expand on what Panos has said, to actually run the tests you can use the following commands (after you have rebuilt Firefox with your changes):

To run the xpcshell tests:

$ make -C $OBJ/toolkit/devtools/debugger/tests/ xpcshell-tests

To run the mochitests:

$ TEST_PATH=browser/devtools/debugger/test/ make -C obj/ mochitest-browser-chrome
Where $OBJ is your object directory.

(Mine is simply "obj", which is why that second command is "make -C obj/". I forgot to change that to "$OBJ" when pasting.)

Comment 5

5 years ago
Is getter used in any function? I have not found any function that uses get dbg()

Comment 6

5 years ago
(In reply to Panos Astithas [:past] from comment #2)
> (In reply to Amod from comment #1)
> > Sir,
> > I would like to contribute to this bug, Will you explain the bug in detail ?
> 
> Yes, thank you. In toolkit/devtools/debugger/server/dbg-script-actors.js
> there is a _dbg property on the ThreadActor prototype as well as a dbg
> getter for that property. This distinction has never been of much use in
> practice, so for simplicity, we would like to just have a plain dbg property
> on the prototype, without a getter. 
> 
> This is pretty much a search-and-replace job, but you will also need to run
> the xpcshell tests and the mochitests to be sure nothing is broken after
> this change. The locations of the tests are:
> 
> toolkit/devtools/debugger/tests/unit/
> browser/devtools/debugger/test/
> 
> Pop in the #devtools channel in IRC (I'm past) if you need any help with the
> above.


Yes Sir, there is a line where get dbg() is used and in the body dbg property is used using 'this' keyword..so after that what exactly should be done...and how to recognize whether the property is public or private ?
(In reply to WangJun from comment #5)
> Is getter used in any function? I have not found any function that uses get
> dbg()

Getters are triggered from plain property accesses, like "this.dbg".

(In reply to Amod from comment #6)
> Yes Sir, there is a line where get dbg() is used and in the body dbg
> property is used using 'this' keyword..so after that what exactly should be
> done...and how to recognize whether the property is public or private ?

As I said in comment 2, you should replace all instances of this._dbg with this.dbg and then run the tests to make sure everything still works as expected. We prefix private properties with an underscore, so |_foo| is considered a private property, whereas |foo| is considered public.
(Assignee)

Updated

5 years ago
Assignee: nobody → anton
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 8

5 years ago
Created attachment 663166 [details] [diff] [review]
Patch to replace _dbg property and dbg getter with a dbg property

I ran both xpcshell tests and mochi tests, they all passed. I also find/grep'ed toolkit/ and browser/devtools directory to see if _dbg was ever used as a public property but didn't notice any cases.

Let me know if I missed anything.
Attachment #663166 - Flags: review?(past)
Comment on attachment 663166 [details] [diff] [review]
Patch to replace _dbg property and dbg getter with a dbg property

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

Looks like you got them all, thanks!
Attachment #663166 - Flags: review?(past) → review+
Whiteboard: [good first bug][mentor=past][lang=js] → [good first bug][mentor=past][lang=js][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/48ed2f6d569f
Whiteboard: [good first bug][mentor=past][lang=js][land-in-fx-team] → [good first bug][mentor=past][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/48ed2f6d569f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=past][lang=js][fixed-in-fx-team] → [good first bug][mentor=past][lang=js]
Target Milestone: --- → Firefox 18
You need to log in before you can comment on or make changes to this bug.