Splitting this out from Bug #1037992 This is the latest review from Bug 1037992, comment 14, by jryans: Review of attachment 8664982 [details] [diff] [review] [diff] [review]: ----------------------------------------------------------------- ::: devtools/server/actors/script.js @@ +3255,2 @@ > > + initialize: function (aThreadActor, aOriginalLocation) { You could convert away from `aArg` style to `arg` if you wanted... @@ +3392,5 @@ > + delete: method(function () { > + return this._delete(); > + }), > + > + _delete: function () { You don't need 2 separate functions. It's safe for an actor to call a function defined via p.js `method(...)` directly, because `method(...)` returns the function itself, which will be available on the actor object like normal.

This patch addresses the issues raised in the last review. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c99c6737571f

This patch addresses the previous feedback and moves BreakpointActor to its own file (as described in the META).

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de5cd72475c6

Comment on attachment 8706943 [details] [diff] [review] Bug1235374.patch Review of attachment 8706943 [details] [diff] [review]: ----------------------------------------------------------------- Overall seems good! Just a few more cleanup nits to fix. ::: devtools/server/actors/breakpoint.js @@ +46,5 @@ > + /** > + * Called when this same breakpoint is added to another Debugger.Script > + * instance. > + * > + * @param aScript Debugger.Script Nit: Comment needs updating @@ +80,5 @@ > + * `result` will be `undefined`. > + * - message: string > + * If the condition throws, this is the thrown message. > + */ > + checkCondition: function(aFrame) { Nit: Rename to frame and update comment @@ +112,5 @@ > + > + /** > + * A function that the engine calls when a breakpoint has been hit. > + * > + * @param aFrame Debugger.Frame Nit: Update arg name @@ +115,5 @@ > + * > + * @param aFrame Debugger.Frame > + * The stack frame that contained the breakpoint. > + */ > + hit: function (frame) { Nit: No space after function @@ +157,5 @@ > + > + /** > + * Handle a protocol request to remove this breakpoint. > + * > + * @param aRequest object Nit: Remove unused arg from comment

Gah, how did I leave so many nits for you to find? Sorry about that, and thanks for finding them. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bebe069cb0e&selectedJob=15764166

[bugday-20160323] Status: RESOLVED,FIXED -> UNVERIFIED Comments: STR: Not clear. Developer specific testing Component: Name Firefox Version 46.0b9 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: Developer specific testing Actual Results: As expected