1654587 - Use single shared module for Debugger settings

Status: RESOLVED FIXED

(DevTools :: Debugger, task, P3)

Description

[Julian Descottes [:jdescottes]]

The debugger stores its "settings" both in preferences and in the async store.

To interact with the async store, the Debugger uses a helper https://searchfox.org/mozilla-central/source/devtools/client/shared/async-store-helper.js . To interact with preferences, it uses another helper https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/utils/prefs.js . This prefs module also exposes some content from the async-store.

This layer of abstraction is used in the debugger to interact with settings.

However we also read debugger settings in a module which is now loaded by the toolbox: https://searchfox.org/mozilla-central/source/devtools/client/shared/thread-utils.js
For now this module only reads preferences but we are about to also read from the async store, in the scope of https://phabricator.services.mozilla.com/D75157 (Bug 1633727).

Which means we have two modules interacting with the same preferences and async store keys, using sometimes different names. And given the way prefs.js is written (the preferences names are split and harder to grep), there is a risk of maintenance issue.

For some settings (eg eventBreakpoints) we store non trivial structures in the async store. We actually store part of the debugger state, which makes it so that the thread-utils module ends up depending on the precise structure of the debugger state. Someone could very well update the debugger state's shape and completely miss that thread-utils needs to be updated.

We should have a single abstraction to retrieve debugger settings, ideally with a single shared instance between the toolbox and the debugger so that we can avoid race issues.

Comment 1

[Alexandre Poirot [:ochameau] [PTO back August 21]]

I thought that was a source of test failure in bug 1620280, and wrote a patch to fix this.
But my test failure was actually related to something else.
Nonetheless, let's fix this given that I have a patch already :)

Comment 2

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Attachment: Bug 1654587 - [devtools] Use a shared asyncStore instance between Debugger Frontend and TargetMixin.

Comment 3

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Attachment: Bug 1654587 - [devtools] Remove now-unused async-storage and async-store-helper modules.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70b213ca2795
[devtools] Use a shared asyncStore instance between Debugger Frontend and TargetMixin. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/69001a563d74
[devtools] Remove now-unused async-storage and async-store-helper modules. r=jdescottes,nchevobbe

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/70b213ca2795
https://hg.mozilla.org/mozilla-central/rev/69001a563d74