1674977 - Enable watcher for all content toolboxes by default

Status: RESOLVED FIXED

(DevTools :: General, task, P3)

Description

[Julian Descottes [:jdescottes]]

Currently we only use the watcher when Fission is enabled, checked via gDevTools::isFissionContentToolboxEnabled

However, as we start to put more and more features in the Watcher, it would be useful if the watcher was always started by default, even if Fission is not enabled.

We can either relax the check made in target-list to decide to watch for frames:

--- a/devtools/shared/resources/target-list.js
+++ b/devtools/shared/resources/target-list.js
@@ -250,19 +250,19 @@ class TargetList extends EventEmitter {
     if (this.targetFront.isParentProcess) {
       const fissionBrowserToolboxEnabled = Services.prefs.getBoolPref(
         BROWSERTOOLBOX_FISSION_ENABLED
       );
       if (fissionBrowserToolboxEnabled) {
         types = TargetList.ALL_TYPES;
       }
     } else if (this.targetFront.isLocalTab) {
-      if (gDevTools.isFissionContentToolboxEnabled()) {
-        types = [TargetList.TYPES.FRAME];
-      }
+      // Always watch for frame targets when debugging local tabs, as we need
+      // the watcher to notify about new targets for target-switching.
+      types = [TargetList.TYPES.FRAME];
     }
     if (this.listenForWorkers && !types.includes(TargetList.TYPES.WORKER)) {
       types.push(TargetList.TYPES.WORKER);
     }
     if (
       this.listenForWorkers &&
       !types.includes(TargetList.TYPES.SHARED_WORKER)
     ) {

Or we can also fully remove isFissionContentToolboxEnabled
https://searchfox.org/mozilla-central/search?q=isFissionContentToolboxEnabled%28&path=

For the second option we should first review quickly the impacted code paths.

First option seems safer but on the other hand it can be weird to keep only part of the fission related logic behind this pref.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1674977 - [devtools] Enable Frame watching for all content toolboxes

This changeset only starts the FrameWatcher for any content toolbox.
The stack will progressively phase out the devtools.contenttoolbox.fission preference.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1674977 - [devtools] Always return true from gDevTools::isFissionContentToolboxEnabled

Depends on D95685

Keep the isFissionContentToolboxEnabled method but always return true to effectively enable all fission related behavior for all content toolboxes.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1674977 - [devtools] Remove gDevTools::isFissionContentToolboxEnabled

Depends on D95686

Finally, remove the method itself.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7079d8e4ed45
[devtools] Enable Frame watching for all content toolboxes r=ochameau
https://hg.mozilla.org/integration/autoland/rev/745be7d4d4ce
[devtools] Always return true from gDevTools::isFissionContentToolboxEnabled r=ochameau
https://hg.mozilla.org/integration/autoland/rev/8601089c63d5
[devtools] Remove gDevTools::isFissionContentToolboxEnabled r=ochameau

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/7079d8e4ed45
https://hg.mozilla.org/mozilla-central/rev/745be7d4d4ce
https://hg.mozilla.org/mozilla-central/rev/8601089c63d5