1839839 - Avoid cloning the frames objects unless it is strictly necessary

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Description

[Alexandre Poirot [:ochameau]]

Bug 1835985 removed some unecessary cloning of the frame objects.
But I missed that there were at least 3 other cases of such cloning.

One related to mapping frames to original locations:
https://searchfox.org/mozilla-central/rev/3424c000a7ff304b2d1efb8561a924232f7f12fc/devtools/client/debugger/src/actions/pause/mapFrames.js#36-46
Here we could avoid cloning the frame object if the location isn't mapped to an original source.

One related to WASM sources (there is nothing making it clear this code relates to WASM!):
https://searchfox.org/mozilla-central/rev/3424c000a7ff304b2d1efb8561a924232f7f12fc/devtools/client/debugger/src/actions/pause/mapFrames.js#94-118

And one last related to the computation of originalDisplayName, which could probably be computed while mapping the frames:
https://searchfox.org/mozilla-central/rev/3424c000a7ff304b2d1efb8561a924232f7f12fc/devtools/client/debugger/src/actions/pause/mapDisplayNames.js#26-27

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839839 - [devtools] Clarify expandFrames code being specific to WASM sources.

This isn't mentioned anywhere, but "expandFrames" is specific to WASM.
I've added some comments and renames, while highlighting the difference between the two possible types of frame objects.
At the end, frame objects for non-WASM orignal sources aren't special, they only have distinct location vs generatedLocation attributes.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839839 - [devtools] Clarify the computation of frame's originalDisplayName.

Only try to compute frame's originalDisplayName for frame relating to a non-WASM original source.
For WASM frames, it is already computed.
For generated frames, it isn't useful as frame.displayName is correct and provided by the server.
Also acknowledge why we are only computing original display from the select action.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839839 - [devtools] Move the computation of frame's originalDisplayFrame to the reducer.

This computation was only done from selectLocation action,
after the completion of the SET_SYMBOLS action. That, only if the current thread was paused.

Doing this right from the reducer helps avoiding thread's frames array duplication
unless this is strictly necessary.
It is probably also a clearer way to say this work should be done only after symbols are fetched.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6978d4ba749
[devtools] Clarify expandFrames code being specific to WASM sources. r=bomsy,devtools-reviewers
https://hg.mozilla.org/integration/autoland/rev/c100c0694778
[devtools] Clarify the computation of frame's originalDisplayName. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/5a81ad4ed7c9
[devtools] Move the computation of frame's originalDisplayFrame to the reducer. r=devtools-reviewers,bomsy

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c6978d4ba749
https://hg.mozilla.org/mozilla-central/rev/c100c0694778
https://hg.mozilla.org/mozilla-central/rev/5a81ad4ed7c9