Last Comment Bug 785044 - Replace the ThreadActor's private debugger object and its getter with a public property
: Replace the ThreadActor's private debugger object and its getter with a publi...
Status: RESOLVED FIXED
[good first bug][mentor=past][lang=js]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 18
Assigned To: Anton Kovalyov (:anton)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-23 06:05 PDT by Panos Astithas [:past]
Modified: 2012-09-22 00:35 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to replace _dbg property and dbg getter with a dbg property (2.05 KB, patch)
2012-09-20 14:19 PDT, Anton Kovalyov (:anton)
past: review+
Details | Diff | Review

Description Panos Astithas [:past] 2012-08-23 06:05:48 PDT
No need for this complication. See also bug 783393 comment 48.
Comment 1 Amod [:AMoz] 2012-09-09 11:32:07 PDT
Sir,
I would like to contribute to this bug, Will you explain the bug in detail ?
Comment 2 Panos Astithas [:past] 2012-09-10 01:35:50 PDT
(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.
Comment 3 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2012-09-10 08:23:11 PDT
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
Comment 4 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2012-09-10 08:25:08 PDT
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 WangJun 2012-09-15 04:54:13 PDT
Is getter used in any function? I have not found any function that uses get dbg()
Comment 6 Amod [:AMoz] 2012-09-15 08:58:04 PDT
(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 ?
Comment 7 Panos Astithas [:past] 2012-09-17 02:42:40 PDT
(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.
Comment 8 Anton Kovalyov (:anton) 2012-09-20 14:19:58 PDT
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.
Comment 9 Panos Astithas [:past] 2012-09-21 00:03:24 PDT
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!
Comment 10 Panos Astithas [:past] 2012-09-21 01:01:26 PDT
https://hg.mozilla.org/integration/fx-team/rev/48ed2f6d569f
Comment 11 Tim Taubert [:ttaubert] 2012-09-22 00:35:31 PDT
https://hg.mozilla.org/mozilla-central/rev/48ed2f6d569f

Note You need to log in before you can comment on or make changes to this bug.