1403760 - Small breakpoint code health improvements

Status: RESOLVED WONTFIX

(DevTools :: Debugger, enhancement)

Description

[Jason Laster [:jlast]]

* make setBreakpoint async/await
* add dumpActors (similar to dumpPools) http://searchfox.org/mozilla-central/source/devtools/server/main.js#1907

Comment 1

[Jason Laster [:jlast]]

Attachment: bp-ch.patch

Comment 2

[Jason Laster [:jlast]]

* dumpActors is a really nice way of seeing what's in the breakpoint actor map, which is used everywhere.
* setBreakpoint is a great method, which can be de-promisified...

Comment 3

[Yulia Startsev [:yulia]]

Comment on attachment 8912980 [details] [diff] [review]
bp-ch.patch

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

Hey!

I went through the code and i have a few comments. I can't figure out how to do line by line comments so here they are as a list:

BreakpointActorMap.prototype : _dumpActors
Line 203: prefer `===` since `true == 1` is truthy
Line 216: the naming "lines" is misleading. it looks like this is an object that contains both a line and a column. `location` may be better here. another question here; why do we want to return line if we have 1 column element? does this mean that the column element is not valid? are we expecting 2 column elements (start and end)? maybe a comment would help here, or checking for columns.length < 2 would be more expressive?
Line 234: `out` is ambiguous, and that it is output can be inferred from the result being returned. what are we outputting? a string of actor ids?

What is the purpose of dumpactors? debugging?
Also, we should be careful saying that setBreakpoint is no longer a promise, or has been depromisifed. Async/await is syntactic sugar for promises. The function's output hasn't been changed. Mentioning this so that no one gets confused.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jlast, could you have a look please?

Comment 5

[Jason Laster [:jlast]]

not needeed anymore