1128918 - Store original location in BreakpointActor.

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Eddy Bruel [:ejpbruel]]

Attachment: patch

BreakpointActors currently store the generated location with which they are associated. In addition, they are stored by original location in the breakpoint store. Since our assumption that BreakpointActors correspond to a single generated location is wrong, it makes more sense to store them in the breakpoint store by generated location.

We can't simply replace the generated location stored in the BreakpointActor with the original location, since doing so would require multiple parts at once. Instead, we are going to proceed in steps. This bug is about the first step, which is about storing the original location in the BreakpointActor as well.

Comment 1

[James Long (:jlongster)]

Comment on attachment 8558487 [details] [diff] [review]
patch

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

Looks good, with a few nits.

::: toolkit/devtools/server/actors/script.js
@@ +2768,5 @@
>     */
> +  _getOrCreateBreakpointActor: function (originalLocation, generatedLocation,
> +                                         condition)
> +  {
> +    let actor = this.breakpointActorMap.getActor(generatedLocation);

Can you update the doc for this function with the new args?

@@ +2883,4 @@
>  
> +    return this.threadActor.sources.getGeneratedLocation(originalLocation)
> +                                   .then(generatedLocation =>
> +    {

style nit: can this go on the same line as the arrow function? that's more consistent with the rest of the code

@@ +4656,5 @@
>   * @param string aCondition
>   *        Optional. A condition which, when false, will cause the breakpoint to
>   *        be skipped.
>   */
> +function BreakpointActor(aThreadActor, aOriginalLocation, aGeneratedLocation, aCondition)

Same here, please update the docs.

Comment 2

[Eddy Bruel [:ejpbruel]]

Try run for this patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2594f8a070fd

Comment 3

[Eddy Bruel [:ejpbruel]]

Try run is green, pushed to fx-team:
https://hg.mozilla.org/integration/fx-team/rev/49be6ef4d093

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/49be6ef4d093