1609443 - Add deprecation comment to fission.autostart pref

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, task, P1)

Description

[Chris Peterson [:cpeterson]]

The fission.autostart pref was added to StaticPrefList.yaml (in bug 1574090) so the compositor process could read the pref. But Fission being enabled is not a global setting; we can have both Fission and non-Fission windows in the same browsing session (using the “Open new Fission window” and “Open new non-Fission window” File menu items).

Code should check DocShell.useRemoteSubframes instead of reading the fission.autostart pref.

https://searchfox.org/mozilla-central/rev/9b4b41b95cbcda63f565bdc24411e15248f91d83/modules/libpref/init/StaticPrefList.yaml#2963-2968

TBD: should other Fission prefs (like fission.preserve_browsing_contexts and fission.sessionHistoryInParent) also be removed from StaticPrefList.yaml?

Comment 1

[Chris Peterson [:cpeterson]]

We don't want to remove the pref, but we should add a deprecation comment to pref definition.

Adding a useRemoteSubframes to BrowsingContext would be a handy alternative for code that wants to read the pref.

Tracking for Fission Dogfooding (M5)

Comment 2

[Andrew McCreight [:mccr8]]

Adding a useRemoteSubframes to BrowsingContext would be a handy alternative for code that wants to read the pref."

I think that should be a separate bug. I'll file it. I filed bug 1609791 as a meta bug for this work. We can file subbugs for the various ideas.

Comment 3

[Andrew McCreight [:mccr8]]

The followup to bug 1574090 was closed as WONTFIX, and indeed I don't see any uses of the pref from C++, so I think we can just remove it from StaticPrefList.yaml. It would still be good to add a comment to the all.js use of the pref. If it does get removed from the yaml file, it looks like it could also be removed from the IGNORE_PREFS list in tools/lint/libpref/__init__.py.

Comment 4

[Andrew McCreight [:mccr8]]

Oops, I'm wrong about it not being used from C++. The new C++ pref getter is just different: https://searchfox.org/mozilla-central/search?q=fission_autostart&path=

Comment 5

[Chris Peterson [:cpeterson]]

Tracking for Fission Nightly (M6)

This bug is just about adding a code comment. There are already new code reading this pref directly that shouldn't be.

Comment 6

[Kris Maglione [:kmag]]

Attachment: Bug 1609443: Part 1 - Add comments warning against checking fission.autostart directly. r=mccr8

Comment 7

[Kris Maglione [:kmag]]

Attachment: Bug 1609443: Part 2 - Fix most dodgy uses of fission.autostart pref. r=mccr8

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa15f3519362
Part 1 - Add comments warning against checking fission.autostart directly. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/93f5a5134436
Part 2 - Fix most dodgy uses of fission.autostart pref. r=mccr8

Comment 9

[Kris Maglione [:kmag]]

Attachment: Bug 1609443: Follow-up: Fix null deref in assertion.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df7a9dd1525f
Follow-up: Fix null deref in assertion.

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/fa15f3519362
https://hg.mozilla.org/mozilla-central/rev/93f5a5134436
https://hg.mozilla.org/mozilla-central/rev/df7a9dd1525f