1928824 - The debugger doesn't always move the source view to the break point when it's hit

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Jeff Muizelaar [:jrmuizel]]

I think it's often on the first breakpoint hit on during page load. The debugger won't automatically show the currently hit breakpoint and it requires clicking on the stack frame in the stack window to go there.

I've never noticed this behaviour in Chrome.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Alex is seeing a similar intermittent issue where the Debugger doesn't scroll the source to the paused line

Comment 2

[Alexandre Poirot [:ochameau]]

I reproduced against lemonde.fr, by pausing on next interruption and pretty printed the two possible scripts.
If you have simpler or different STR let me know as they may be different race conditions.

There seems to be a race condition between the pretty printing of the pause source and the selection of the paused frame.

When we pause, we immediately select the location of the frame, in the bundle/generated source (i.e. non-pretty printed):
https://searchfox.org/mozilla-central/rev/a4935c8cb9edb35981871dee4a9132ab3e77d92b/devtools/client/debugger/src/actions/pause/paused.js#53
In this action, we do detect it was a pretty printed source and start creating the pretty printed source:
https://searchfox.org/mozilla-central/rev/a4935c8cb9edb35981871dee4a9132ab3e77d92b/devtools/client/debugger/src/actions/sources/select.js#174
Within the process of creating the pretty printed source, we have some logic to maintain the currently selected location from the non-pretty printed to pretty printed location, over here:
https://searchfox.org/mozilla-central/rev/a4935c8cb9edb35981871dee4a9132ab3e77d92b/devtools/client/debugger/src/actions/sources/prettyPrint.js#246-262
But at this point in time, the actual selected location hasn't updated yet and is the one before the pause.
So that we select the top of the pretty printed source.
We then continue the execution of selectLocation action, but bail out as the selected location changed by the pretty print creation action...
https://searchfox.org/mozilla-central/rev/a4935c8cb9edb35981871dee4a9132ab3e77d92b/devtools/client/debugger/src/actions/sources/select.js#261-264

All these async work is really complex to compose against each other, I faced similar challenges in bug 980022.
We might want/have to simplify select and pretty print actions, somehow.

Comment 3

[Alexandre Poirot [:ochameau]]

Bug 1493094 is for when the source is empty in same cases.

Comment 4

[Hubert Boma Manilla (:bomsy)]

(In reply to Alexandre Poirot [:ochameau] from comment #3)

Bug 1493094 is for when the source is empty in same cases.

I think this probably meant to be Bug 1926486, which is fixed

Comment 5

[Alexandre Poirot [:ochameau]]

Reproduced again by Jeff while debugging https://qlover.jp/

Comment 6

[Alexandre Poirot [:ochameau]]

I found a couple of reason why the scroll wouldn't work while investigating test failures in bug 1938418.
I'll submit a couple of fixes.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1928824 - [devtools] Fix CodeMirror 6 helper checking for lines being in the viewport.

Adding a new test to extensively cover isScrolledPositionVisible behavior,
while doing strong assertion on the behavior of scroll on pause.

Also tweak isScrolledPositionVisible and scrollEditorIntoView test function to receive 1-based lines.
Many usages passed 1-based lines and could possibly have an unexpected -1 shift.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9dc0e10e7f1
[devtools] Fix CodeMirror 6 helper checking for lines being in the viewport. r=devtools-reviewers,bomsy

Comment 9

[amarc]

https://hg.mozilla.org/mozilla-central/rev/c9dc0e10e7f1