Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
Since there were no further concerns voiced to the final proposal I've gone ahead and landed the change in https://bugzilla.mozilla.org/show_bug.cgi?id=1653384. To be respected you must now set the MOZ_FORCE_DISABLE_E10S environment variable to the full application version. `mach run --disable-e10s` has been updated to do this automatically. On Wed, Jun 17, 2020 at 1:45 PM Gijs Kruitbosch wrote: > Having read all the responses here, I guess an adjusted proposal would > be to switch to requiring the variable to be set to the build's version. > Does that seem like it'd address your concerns? > > There were two other points raised that I wanted to briefly respond to: > > > What does this proposal mean for ./mach run --disable-e10s ? > > In the adjusted proposal, this would set the right env var, as it does > today, so there should be no difference. > > > * particularly on Windows where even basic `printf` and `dump` from the > child process don't show up in the console. > > As I have suggested to you elsewhere, I think this is a serious problem > that inhibits adoption of Windows as a development platform for Firefox, > and we should address this orthogonal to whatever we do with e10s. > Inasmuch as this isn't already covered in > https://bugzilla.mozilla.org/show_bug.cgi?id=1257155, please file bugs. > > Note that you do not actually need to disable e10s, running with the > `-attach-console` commandline switch (which you need anyway, also if you > disable e10s) and the `MOZ_DISABLE_CONTENT_SANDBOX=1` env var are > sufficient to get dump logging from the content process in my testing. > > > Longer-term, it would be useful to think about how else we could cater > to your debugging needs, without completely disabling e10s - whether > that's something like the layout debugger (which was switched to using > e10s a while back) to reduce the noise of the rest of Firefox, or a way > to directly inspect the a11y API output of the content process, or > whatever it is - it is not ultimately feasible to keep "supporting" > several modes of operation forever. > > ~ Gijs > > > On 10/06/2020 19:58, David Major wrote: > > I agree that it's a bad idea for users to be running permanently with > this > > setting on their daily driver browsers. > > > > But the environment variable has been a huge productivity enhancer to > > reduce my mental load when setting up an extra-hairy debug session or > > taking system traces. > > > > I wish we could have a way to allow this for one-off cases but not > > long-term usage. Unfortunately I can't settle for a proposal like "allow > it > > only in debug or only in nightlies" because I often need to debug actual > > user-facing builds. Is there any way we could build some auto-expiration > > into this setting, like maybe you'd have to set the env var equal to the > > build ID or today's date? > > > > > > > > On Wed, Jun 10, 2020 at 2:44 PM Dave Townsend > wrote: > > > >> Non-e10s is such a different environment that I don't think we have any > >> hope of keeping it working without running the full test suite in that > mode > >> and I don't think anyone wants to do that. Now that this has started > >> breaking I think it is actively harmful to our users for us to allow > them > >> to disable e10s. > >> > >> On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch < > gijskruitbo...@gmail.com > >>> > >> wrote: > >> > >>> (Copied to fx-dev; Replies to dev-platform please.) > >>> > >>> Hello, > >>> > >>> Just over a year ago, I started a discussion[0] about our support for > >>> disabling e10s. The outcome of that was that we removed support for > >>> disabling e10s with a pref on Desktop Firefox with version 68, except > >>> for use from automation. We kept support for using the environment > >>> variable. [1] > >>> > >>> Last week, we released Firefox 77, which turned out to break all > >>> webpages sent using compression (like gzip) if you had disabled e10s > >>> using this environment variable. [2] > >>> > >>> So here we are again. I'd like to propose we also stop honouring the > >>> environment variable unless we're running tests in automation. We > >>> clearly do not have sufficient test coverage to guarantee basic things > >>> like "the browser works", it lacks security sandboxing, and a number of > >>> other projects require it (fission, gpu process, socket process, ...), > >>> so I think it's time to stop supporting this configuration at all. > >>> > >>> I hope to make this change for the 79 cycle. I'm open to arguments > >>> either way about what to do for 78 esr (assuming the patch for 79 turns > >>> out to be simple; the work to remove the pref had a number of annoying > >>> corner-cases at the time). > >>> > >>> Please speak up if you think that this plan needs adjusting. > >>> > >>> ~ Gijs > >>> > >>> > >>> [0] > >>> > >>> > >> > https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ > >>> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 > >>> [2]
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
I think I have found an inconsistency in TB's pref setting during
mochitest regarding e10s.
The gory detail is in
https://bugzilla.mozilla.org/show_bug.cgi?id=1629433#c10
Major find (excerpt from the above bugzilla.)
--- begin quote ---
1) Dynamically generated user.js files during tests of C-C TB mochitest
execution, contain conflicting stetting.
pref("browser.tabs.remote.autostart", {true|false});
The value is either true or false. Not consistent. In mochitest of C-C
TB, there may be a few very browser-like test. In those cases,
|browser.tabs.remote.autostart| can be true? However, if we ALWAYS need
to set the value for false for C-C TB mochitest,
>> // No e10s in Thunderbird for now.
>> pref("browser.tabs.remote.autostart", false); <=== This
we may be in a big trouble.
I observe that there are multiple lines to set
"browser.tabs.remote.autostart" in a user.js with different
setting. Of course, the last one sets the used value. Inquiring mind
wants to know how the setting is composed (presumably concatenate some
settings prepackaged in user.js files in test file directories? I have
noticed that there are user.js files in test directory. See (3)
below.) Clear description of how the run-time user.js is created for
each test during c-c TB mochitest run would be desirable.
In any case, I DID find cases where sub-tests (albeit web
browser-oriented?) run with a user.js with
|pref("browser.tabs.remote.autostart", true);| which may not sit well
for C-C thunderbird binary.
I have a nagging feeling that this bad setting may/could be one of the
causes of the failure to run valgrind because we need more than 1500+
threads to run the C-C TB mochitest(!)
(2) user.js file under MOZ_OBJ/temp,
/NEW-SSD/moz-obj-dir/objdir-tb3/temp/user.js, does NOT set
|pref("browser.tabs.remote.autostart", false);|
I wonder if this user.js in MOZ_OBJ directory is used or not during C-C
mochitest. If it is used, I think we are in a big trouble since it does
not set the value to false.
(3) There are a few |user.js| files with conflicting settings in the
test directories. Some set the pref to false, but some set to true.
I hope they are used appropriately. Since the C-C mochitest works more
or less, I believe they are used correctly, but a clear
documentation/explanation of how each is used and how each test creates
user.js dynamically from the prepackaged user.js for TB
mochitest would be desirable.
--- end quote ---
So if the testing of web browser function in C-C TB mochitest runs with
e10s enabled, then it is not testing the real world usage because
e10s is disabled for TB usually.
(OTOH, it is hard to believe that if TB needs to disable e10s, the
built-in web renderer in TB can run with e10s enabled without disrupting
the behavior of the rest of the code. Something is screwed up in
dynamically generated mochitest user.js )
I hope someone familiar with C-C TB mochitest investigates this and
figures if the current state of the affairs is correct or not.
Chiaki
On 2020/06/18 16:46, Magnus Melin wrote:
Currently Thunderbird doesn't work with e10s.
Longer term I'm assuming we'll need to do necessary adaptions so that
we can - but I suspect this is a slightly larger project...
I've filed bug 1646648 to track this work.
-Magnus
On 2020-06-10 22:05, Gijs Kruitbosch wrote:
I can't speak for Thunderbird's plans, but either way these plans
shouldn't affect them and is restricted to desktop Firefox; the pref
still works there:
https://searchfox.org/mozilla-central/rev/4bb2401ecbfce89af06fb2b4d0ea3557682bd8ff/toolkit/xre/nsAppRunner.cpp#5020-5024
, and they set it:
https://searchfox.org/comm-central/rev/e62a0af3cba6e1c65b2d4be02d3fefce88cf3f8f/mail/app/profile/all-thunderbird.js#654
Of course, if TB needs this configuration then that may complicate
removing support for non-e10s entirely...
~ Gijs
On 10/06/2020 19:56, Emilio Cobos Álvarez wrote:
What is the situation of Thunderbird? I think they don't have e10s
enabled
yet, and it may be worth at least knowing what their plans are.
-- Emilio
On Wed, Jun 10, 2020, 8:44 PM Dave Townsend
wrote:
Non-e10s is such a different environment that I don't think we have
any
hope of keeping it working without running the full test suite in
that mode
and I don't think anyone wants to do that. Now that this has started
breaking I think it is actively harmful to our users for us to
allow them
to disable e10s.
On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch
wrote:
(Copied to fx-dev; Replies to dev-platform please.)
Hello,
Just over a year ago, I started a discussion[0] about our support for
disabling e10s. The outcome of that was that we removed support for
disabling e10s with a pref on Desktop Firefox with version 68, except
for use from automation. We kept support for using the environment
variable. [1]
Last week, we released Firefox 77, which turned out to break all
webpages sent using compression (like
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
Currently Thunderbird doesn't work with e10s. Longer term I'm assuming we'll need to do necessary adaptions so that we can - but I suspect this is a slightly larger project... I've filed bug 1646648 to track this work. -Magnus On 2020-06-10 22:05, Gijs Kruitbosch wrote: I can't speak for Thunderbird's plans, but either way these plans shouldn't affect them and is restricted to desktop Firefox; the pref still works there: https://searchfox.org/mozilla-central/rev/4bb2401ecbfce89af06fb2b4d0ea3557682bd8ff/toolkit/xre/nsAppRunner.cpp#5020-5024 , and they set it: https://searchfox.org/comm-central/rev/e62a0af3cba6e1c65b2d4be02d3fefce88cf3f8f/mail/app/profile/all-thunderbird.js#654 Of course, if TB needs this configuration then that may complicate removing support for non-e10s entirely... ~ Gijs On 10/06/2020 19:56, Emilio Cobos Álvarez wrote: What is the situation of Thunderbird? I think they don't have e10s enabled yet, and it may be worth at least knowing what their plans are. -- Emilio On Wed, Jun 10, 2020, 8:44 PM Dave Townsend wrote: Non-e10s is such a different environment that I don't think we have any hope of keeping it working without running the full test suite in that mode and I don't think anyone wants to do that. Now that this has started breaking I think it is actively harmful to our users for us to allow them to disable e10s. On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch wrote: (Copied to fx-dev; Replies to dev-platform please.) Hello, Just over a year ago, I started a discussion[0] about our support for disabling e10s. The outcome of that was that we removed support for disabling e10s with a pref on Desktop Firefox with version 68, except for use from automation. We kept support for using the environment variable. [1] Last week, we released Firefox 77, which turned out to break all webpages sent using compression (like gzip) if you had disabled e10s using this environment variable. [2] So here we are again. I'd like to propose we also stop honouring the environment variable unless we're running tests in automation. We clearly do not have sufficient test coverage to guarantee basic things like "the browser works", it lacks security sandboxing, and a number of other projects require it (fission, gpu process, socket process, ...), so I think it's time to stop supporting this configuration at all. I hope to make this change for the 79 cycle. I'm open to arguments either way about what to do for 78 esr (assuming the patch for 79 turns out to be simple; the work to remove the pref had a number of annoying corner-cases at the time). Please speak up if you think that this plan needs adjusting. ~ Gijs [0] https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 ___ firefox-dev mailing list firefox-...@mozilla.org https://mail.mozilla.org/listinfo/firefox-dev ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
Having read all the responses here, I guess an adjusted proposal would be to switch to requiring the variable to be set to the build's version. Does that seem like it'd address your concerns? There were two other points raised that I wanted to briefly respond to: What does this proposal mean for ./mach run --disable-e10s ? In the adjusted proposal, this would set the right env var, as it does today, so there should be no difference. * particularly on Windows where even basic `printf` and `dump` from the child process don't show up in the console. As I have suggested to you elsewhere, I think this is a serious problem that inhibits adoption of Windows as a development platform for Firefox, and we should address this orthogonal to whatever we do with e10s. Inasmuch as this isn't already covered in https://bugzilla.mozilla.org/show_bug.cgi?id=1257155, please file bugs. Note that you do not actually need to disable e10s, running with the `-attach-console` commandline switch (which you need anyway, also if you disable e10s) and the `MOZ_DISABLE_CONTENT_SANDBOX=1` env var are sufficient to get dump logging from the content process in my testing. Longer-term, it would be useful to think about how else we could cater to your debugging needs, without completely disabling e10s - whether that's something like the layout debugger (which was switched to using e10s a while back) to reduce the noise of the rest of Firefox, or a way to directly inspect the a11y API output of the content process, or whatever it is - it is not ultimately feasible to keep "supporting" several modes of operation forever. ~ Gijs On 10/06/2020 19:58, David Major wrote: I agree that it's a bad idea for users to be running permanently with this setting on their daily driver browsers. But the environment variable has been a huge productivity enhancer to reduce my mental load when setting up an extra-hairy debug session or taking system traces. I wish we could have a way to allow this for one-off cases but not long-term usage. Unfortunately I can't settle for a proposal like "allow it only in debug or only in nightlies" because I often need to debug actual user-facing builds. Is there any way we could build some auto-expiration into this setting, like maybe you'd have to set the env var equal to the build ID or today's date? On Wed, Jun 10, 2020 at 2:44 PM Dave Townsend wrote: Non-e10s is such a different environment that I don't think we have any hope of keeping it working without running the full test suite in that mode and I don't think anyone wants to do that. Now that this has started breaking I think it is actively harmful to our users for us to allow them to disable e10s. On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch wrote: (Copied to fx-dev; Replies to dev-platform please.) Hello, Just over a year ago, I started a discussion[0] about our support for disabling e10s. The outcome of that was that we removed support for disabling e10s with a pref on Desktop Firefox with version 68, except for use from automation. We kept support for using the environment variable. [1] Last week, we released Firefox 77, which turned out to break all webpages sent using compression (like gzip) if you had disabled e10s using this environment variable. [2] So here we are again. I'd like to propose we also stop honouring the environment variable unless we're running tests in automation. We clearly do not have sufficient test coverage to guarantee basic things like "the browser works", it lacks security sandboxing, and a number of other projects require it (fission, gpu process, socket process, ...), so I think it's time to stop supporting this configuration at all. I hope to make this change for the 79 cycle. I'm open to arguments either way about what to do for 78 esr (assuming the patch for 79 turns out to be simple; the work to remove the pref had a number of annoying corner-cases at the time). Please speak up if you think that this plan needs adjusting. ~ Gijs [0] https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 ___ firefox-dev mailing list firefox-...@mozilla.org https://mail.mozilla.org/listinfo/firefox-dev ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
On Wed, Jun 10, 2020 at 11:13 PM James Teh wrote: > In general, this obviously makes a lot of sense. However, because there is > so much extra complication for accessibility when e10s is enabled, I find > myself disabling e10s in local opt/debug builds to isolate problems to the > core a11y engine (vs the a11y e10s stuff). This is also relevant to other debugging scenarios, especially when not being able to use Pernosco to search for the right process. What does this proposal mean for ./mach run --disable-e10s ? -- Henri Sivonen hsivo...@mozilla.com ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
On 6/10/2020 2:09 PM, tom...@gmail.com wrote: * GeckoView still supports running in non-e10s mode, and inability to mimic that environment on desktop builds would complicate writing code that works on android. GeckoView's only technical reason for supporting non-e10s was FxR, but they have since switched to e10s we can (and want to) deprecate GV support for non-e10s. - Aaron ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
I can't speak for what TB development plan is.
One thing I observe as an occasional submitter of TB patches is this.
Thunderbird ditched |mozmill| test December 2019, and switched to
mochitest in place of mozmill test.
Unfortunately, valgrind no longer works locally for mochitest.
This is quite a loss for keeping the code in sane state. I have
uncovered quite a few memory-related bugs over the years by running TB
under valgrind during mozmill-based test.
(I don't know if there is any official valgrind run of TB on tryserver
despite arcane description I read years ago that
it was performed from time to time.).
xpcshell-tests still runs under valgrind. I found a few memory-related
errors by running TB under valgrind during xpcshell-tests this year.
So for me, valgrind test is very important for keeping TB in sane state
after a large patch set lands, etc.
(Actually, I have a large patch set that enables buffered-write for
message handling and this will boost write performance very much. But it
is a rather lage patch set, and I want to run tests under valgrind to
make sure nothing undesirable is introduced.)
Looking at the posts, I have a feeling that TB may be able to run under
valgrind during mochitest if this
e10s setting is properly handled. Right now, TB under valgrind during
mochitest starts more than 1500 threads (too many IMHO), but less than
5000 threads (I have not figured out exactly how many threads are
required), and valgrind barfs. I need to increase the number of threads
valgrind supports,
but maybe because of the large number of threads, tests time out before
anything useful is performed. Thus no useful memory test is done under
mochitest currently.
Something is wrong there.
I suspect this e10s setting *may* have a bit to do with the situation.
I think I have to make sure TB runs in non e10s mode to run under
valgrind during mochitest.
Specifically I am not sure if the setting is honored in test user
profile prepared for mochitest.:
https://searchfox.org/comm-central/rev/e62a0af3cba6e1c65b2d4be02d3fefce88cf3f8f/mail/app/profile/all-thunderbird.js#654
||
|// No e10s in Thunderbird for now. |
|pref("browser.tabs.remote.autostart", false); |
||
*Maybe*, by making sure that this is set to false, valgrind might work
under mochitest.
Maybe a fishing trip in the coming weekend.
Chiaki
On 2020/06/11 4:05, Gijs Kruitbosch wrote:
I can't speak for Thunderbird's plans, but either way these plans
shouldn't affect them and is restricted to desktop Firefox; the pref
still works there:
https://searchfox.org/mozilla-central/rev/4bb2401ecbfce89af06fb2b4d0ea3557682bd8ff/toolkit/xre/nsAppRunner.cpp#5020-5024
, and they set it:
https://searchfox.org/comm-central/rev/e62a0af3cba6e1c65b2d4be02d3fefce88cf3f8f/mail/app/profile/all-thunderbird.js#654
Of course, if TB needs this configuration then that may complicate
removing support for non-e10s entirely...
~ Gijs
On 10/06/2020 19:56, Emilio Cobos Álvarez wrote:
What is the situation of Thunderbird? I think they don't have e10s
enabled
yet, and it may be worth at least knowing what their plans are.
-- Emilio
On Wed, Jun 10, 2020, 8:44 PM Dave Townsend
wrote:
Non-e10s is such a different environment that I don't think we have any
hope of keeping it working without running the full test suite in
that mode
and I don't think anyone wants to do that. Now that this has started
breaking I think it is actively harmful to our users for us to allow
them
to disable e10s.
On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch
wrote:
(Copied to fx-dev; Replies to dev-platform please.)
Hello,
Just over a year ago, I started a discussion[0] about our support for
disabling e10s. The outcome of that was that we removed support for
disabling e10s with a pref on Desktop Firefox with version 68, except
for use from automation. We kept support for using the environment
variable. [1]
Last week, we released Firefox 77, which turned out to break all
webpages sent using compression (like gzip) if you had disabled e10s
using this environment variable. [2]
So here we are again. I'd like to propose we also stop honouring the
environment variable unless we're running tests in automation. We
clearly do not have sufficient test coverage to guarantee basic things
like "the browser works", it lacks security sandboxing, and a
number of
other projects require it (fission, gpu process, socket process, ...),
so I think it's time to stop supporting this configuration at all.
I hope to make this change for the 79 cycle. I'm open to arguments
either way about what to do for 78 esr (assuming the patch for 79
turns
out to be simple; the work to remove the pref had a number of annoying
corner-cases at the time).
Please speak up if you think that this plan needs adjusting.
~ Gijs
[0]
https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941
[2]
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
In general, this obviously makes a lot of sense. However, because there is so much extra complication for accessibility when e10s is enabled, I find myself disabling e10s in local opt/debug builds to isolate problems to the core a11y engine (vs the a11y e10s stuff). The ability to do this was instrumental in some of the perf work I've been doing lately. For example, it allowed me to determine that some of the perf problems I had originally attributed to the a11y e10s layer were actually problems in the core a11y engine. I'm sure there's some way I could have achieved that with e10s enabled, but it probably would have taken me weeks longer. All of that said, I realise this is an obscure case and I don't want to stand in the way of progress; I'm well aware legacy has to die eventually. Nevertheless, I thought I'd at least flag this concern. Btw, the need to isolate the core a11y engine from the a11y e10s stuff is also why some of our a11y tests still run with e10s disabled in automation. Jamie On Thu, Jun 11, 2020 at 4:56 AM David Major wrote: > I agree that it's a bad idea for users to be running permanently with this > setting on their daily driver browsers. > > But the environment variable has been a huge productivity enhancer to > reduce my mental load when setting up an extra-hairy debug session or > taking system traces. > > I wish we could have a way to allow this for one-off cases but not > long-term usage. Unfortunately I can't settle for a proposal like "allow it > only in debug or only in nightlies" because I often need to debug actual > user-facing builds. Is there any way we could build some auto-expiration > into this setting, like maybe you'd have to set the env var equal to the > build ID or today's date? > > > > On Wed, Jun 10, 2020 at 2:44 PM Dave Townsend > wrote: > > > Non-e10s is such a different environment that I don't think we have any > > hope of keeping it working without running the full test suite in that > mode > > and I don't think anyone wants to do that. Now that this has started > > breaking I think it is actively harmful to our users for us to allow them > > to disable e10s. > > > > On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch < > gijskruitbo...@gmail.com > > > > > wrote: > > > > > (Copied to fx-dev; Replies to dev-platform please.) > > > > > > Hello, > > > > > > Just over a year ago, I started a discussion[0] about our support for > > > disabling e10s. The outcome of that was that we removed support for > > > disabling e10s with a pref on Desktop Firefox with version 68, except > > > for use from automation. We kept support for using the environment > > > variable. [1] > > > > > > Last week, we released Firefox 77, which turned out to break all > > > webpages sent using compression (like gzip) if you had disabled e10s > > > using this environment variable. [2] > > > > > > So here we are again. I'd like to propose we also stop honouring the > > > environment variable unless we're running tests in automation. We > > > clearly do not have sufficient test coverage to guarantee basic things > > > like "the browser works", it lacks security sandboxing, and a number of > > > other projects require it (fission, gpu process, socket process, ...), > > > so I think it's time to stop supporting this configuration at all. > > > > > > I hope to make this change for the 79 cycle. I'm open to arguments > > > either way about what to do for 78 esr (assuming the patch for 79 turns > > > out to be simple; the work to remove the pref had a number of annoying > > > corner-cases at the time). > > > > > > Please speak up if you think that this plan needs adjusting. > > > > > > ~ Gijs > > > > > > > > > [0] > > > > > > > > > https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ > > > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 > > > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 > > > ___ > > > firefox-dev mailing list > > > firefox-...@mozilla.org > > > https://mail.mozilla.org/listinfo/firefox-dev > > > > > ___ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
I agree about not shipping this to our users, but I see several needs to keep this option for developers working on Firefox: * GeckoView still supports running in non-e10s mode, and inability to mimic that environment on desktop builds would complicate writing code that works on android. * As David mentioned, debugging with multiple processes is still a pain, and particularly on Windows where even basic `printf` and `dump` from the child process don't show up in the console. On Wednesday, June 10, 2020 at 8:56:47 PM UTC+2, David Major wrote: > I agree that it's a bad idea for users to be running permanently with this > setting on their daily driver browsers. > > But the environment variable has been a huge productivity enhancer to > reduce my mental load when setting up an extra-hairy debug session or > taking system traces. > > I wish we could have a way to allow this for one-off cases but not > long-term usage. Unfortunately I can't settle for a proposal like "allow it > only in debug or only in nightlies" because I often need to debug actual > user-facing builds. Is there any way we could build some auto-expiration > into this setting, like maybe you'd have to set the env var equal to the > build ID or today's date? > > > > On Wed, Jun 10, 2020 at 2:44 PM Dave Townsend wrote: > > > Non-e10s is such a different environment that I don't think we have any > > hope of keeping it working without running the full test suite in that mode > > and I don't think anyone wants to do that. Now that this has started > > breaking I think it is actively harmful to our users for us to allow them > > to disable e10s. > > > > On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch > > > > wrote: > > > > > (Copied to fx-dev; Replies to dev-platform please.) > > > > > > Hello, > > > > > > Just over a year ago, I started a discussion[0] about our support for > > > disabling e10s. The outcome of that was that we removed support for > > > disabling e10s with a pref on Desktop Firefox with version 68, except > > > for use from automation. We kept support for using the environment > > > variable. [1] > > > > > > Last week, we released Firefox 77, which turned out to break all > > > webpages sent using compression (like gzip) if you had disabled e10s > > > using this environment variable. [2] > > > > > > So here we are again. I'd like to propose we also stop honouring the > > > environment variable unless we're running tests in automation. We > > > clearly do not have sufficient test coverage to guarantee basic things > > > like "the browser works", it lacks security sandboxing, and a number of > > > other projects require it (fission, gpu process, socket process, ...), > > > so I think it's time to stop supporting this configuration at all. > > > > > > I hope to make this change for the 79 cycle. I'm open to arguments > > > either way about what to do for 78 esr (assuming the patch for 79 turns > > > out to be simple; the work to remove the pref had a number of annoying > > > corner-cases at the time). > > > > > > Please speak up if you think that this plan needs adjusting. > > > > > > ~ Gijs > > > > > > > > > [0] > > > > > > > > https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ > > > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 > > > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 > > > ___ > > > firefox-dev mailing list > > > firefox-...@mozilla.org > > > https://mail.mozilla.org/listinfo/firefox-dev > > > > > ___ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
I can't speak for Thunderbird's plans, but either way these plans shouldn't affect them and is restricted to desktop Firefox; the pref still works there: https://searchfox.org/mozilla-central/rev/4bb2401ecbfce89af06fb2b4d0ea3557682bd8ff/toolkit/xre/nsAppRunner.cpp#5020-5024 , and they set it: https://searchfox.org/comm-central/rev/e62a0af3cba6e1c65b2d4be02d3fefce88cf3f8f/mail/app/profile/all-thunderbird.js#654 Of course, if TB needs this configuration then that may complicate removing support for non-e10s entirely... ~ Gijs On 10/06/2020 19:56, Emilio Cobos Álvarez wrote: What is the situation of Thunderbird? I think they don't have e10s enabled yet, and it may be worth at least knowing what their plans are. -- Emilio On Wed, Jun 10, 2020, 8:44 PM Dave Townsend wrote: Non-e10s is such a different environment that I don't think we have any hope of keeping it working without running the full test suite in that mode and I don't think anyone wants to do that. Now that this has started breaking I think it is actively harmful to our users for us to allow them to disable e10s. On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch wrote: (Copied to fx-dev; Replies to dev-platform please.) Hello, Just over a year ago, I started a discussion[0] about our support for disabling e10s. The outcome of that was that we removed support for disabling e10s with a pref on Desktop Firefox with version 68, except for use from automation. We kept support for using the environment variable. [1] Last week, we released Firefox 77, which turned out to break all webpages sent using compression (like gzip) if you had disabled e10s using this environment variable. [2] So here we are again. I'd like to propose we also stop honouring the environment variable unless we're running tests in automation. We clearly do not have sufficient test coverage to guarantee basic things like "the browser works", it lacks security sandboxing, and a number of other projects require it (fission, gpu process, socket process, ...), so I think it's time to stop supporting this configuration at all. I hope to make this change for the 79 cycle. I'm open to arguments either way about what to do for 78 esr (assuming the patch for 79 turns out to be simple; the work to remove the pref had a number of annoying corner-cases at the time). Please speak up if you think that this plan needs adjusting. ~ Gijs [0] https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 ___ firefox-dev mailing list firefox-...@mozilla.org https://mail.mozilla.org/listinfo/firefox-dev ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
I agree that it's a bad idea for users to be running permanently with this setting on their daily driver browsers. But the environment variable has been a huge productivity enhancer to reduce my mental load when setting up an extra-hairy debug session or taking system traces. I wish we could have a way to allow this for one-off cases but not long-term usage. Unfortunately I can't settle for a proposal like "allow it only in debug or only in nightlies" because I often need to debug actual user-facing builds. Is there any way we could build some auto-expiration into this setting, like maybe you'd have to set the env var equal to the build ID or today's date? On Wed, Jun 10, 2020 at 2:44 PM Dave Townsend wrote: > Non-e10s is such a different environment that I don't think we have any > hope of keeping it working without running the full test suite in that mode > and I don't think anyone wants to do that. Now that this has started > breaking I think it is actively harmful to our users for us to allow them > to disable e10s. > > On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch > > wrote: > > > (Copied to fx-dev; Replies to dev-platform please.) > > > > Hello, > > > > Just over a year ago, I started a discussion[0] about our support for > > disabling e10s. The outcome of that was that we removed support for > > disabling e10s with a pref on Desktop Firefox with version 68, except > > for use from automation. We kept support for using the environment > > variable. [1] > > > > Last week, we released Firefox 77, which turned out to break all > > webpages sent using compression (like gzip) if you had disabled e10s > > using this environment variable. [2] > > > > So here we are again. I'd like to propose we also stop honouring the > > environment variable unless we're running tests in automation. We > > clearly do not have sufficient test coverage to guarantee basic things > > like "the browser works", it lacks security sandboxing, and a number of > > other projects require it (fission, gpu process, socket process, ...), > > so I think it's time to stop supporting this configuration at all. > > > > I hope to make this change for the 79 cycle. I'm open to arguments > > either way about what to do for 78 esr (assuming the patch for 79 turns > > out to be simple; the work to remove the pref had a number of annoying > > corner-cases at the time). > > > > Please speak up if you think that this plan needs adjusting. > > > > ~ Gijs > > > > > > [0] > > > > > https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ > > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 > > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 > > ___ > > firefox-dev mailing list > > firefox-...@mozilla.org > > https://mail.mozilla.org/listinfo/firefox-dev > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
I was asked off-list why I'm not suggesting we remove support for the environment variable entirely (ie why keep it for tests). That's a good question, so I will attempt to address it. I think that's a laudable goal, but it's more work. Practically speaking, AIUI valgrind still runs with e10s disabled [0], as does gtest [1], some xpcshell tests but not others [2], and we run some mochitest stuff in both configurations. There might be others. I think it's valuable to migrate all of those to e10s, or stop reading it, but I believe it may take some time, while I also think that we should get rid of the user footgun ASAP. Blanket-disabling all the non-e10s tests feels like a separate decision; I assume we are unwilling to lose valgrind coverage, some portions of gtest, and I don't know what else would break. I'd be happy to explore that in the 80 cycle if others agree that's worth doing, though I am probably not the best person to try to fix our valgrind/gtest stuff... [0] https://bugzilla.mozilla.org/show_bug.cgi?id=1549856 [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1552140 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1568333 On 10/06/2020 19:30, Gijs Kruitbosch wrote: (Copied to fx-dev; Replies to dev-platform please.) Hello, Just over a year ago, I started a discussion[0] about our support for disabling e10s. The outcome of that was that we removed support for disabling e10s with a pref on Desktop Firefox with version 68, except for use from automation. We kept support for using the environment variable. [1] Last week, we released Firefox 77, which turned out to break all webpages sent using compression (like gzip) if you had disabled e10s using this environment variable. [2] So here we are again. I'd like to propose we also stop honouring the environment variable unless we're running tests in automation. We clearly do not have sufficient test coverage to guarantee basic things like "the browser works", it lacks security sandboxing, and a number of other projects require it (fission, gpu process, socket process, ...), so I think it's time to stop supporting this configuration at all. I hope to make this change for the 79 cycle. I'm open to arguments either way about what to do for 78 esr (assuming the patch for 79 turns out to be simple; the work to remove the pref had a number of annoying corner-cases at the time). Please speak up if you think that this plan needs adjusting. ~ Gijs [0] https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
What is the situation of Thunderbird? I think they don't have e10s enabled yet, and it may be worth at least knowing what their plans are. -- Emilio On Wed, Jun 10, 2020, 8:44 PM Dave Townsend wrote: > Non-e10s is such a different environment that I don't think we have any > hope of keeping it working without running the full test suite in that mode > and I don't think anyone wants to do that. Now that this has started > breaking I think it is actively harmful to our users for us to allow them > to disable e10s. > > On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch > > wrote: > > > (Copied to fx-dev; Replies to dev-platform please.) > > > > Hello, > > > > Just over a year ago, I started a discussion[0] about our support for > > disabling e10s. The outcome of that was that we removed support for > > disabling e10s with a pref on Desktop Firefox with version 68, except > > for use from automation. We kept support for using the environment > > variable. [1] > > > > Last week, we released Firefox 77, which turned out to break all > > webpages sent using compression (like gzip) if you had disabled e10s > > using this environment variable. [2] > > > > So here we are again. I'd like to propose we also stop honouring the > > environment variable unless we're running tests in automation. We > > clearly do not have sufficient test coverage to guarantee basic things > > like "the browser works", it lacks security sandboxing, and a number of > > other projects require it (fission, gpu process, socket process, ...), > > so I think it's time to stop supporting this configuration at all. > > > > I hope to make this change for the 79 cycle. I'm open to arguments > > either way about what to do for 78 esr (assuming the patch for 79 turns > > out to be simple; the work to remove the pref had a number of annoying > > corner-cases at the time). > > > > Please speak up if you think that this plan needs adjusting. > > > > ~ Gijs > > > > > > [0] > > > > > https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ > > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 > > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 > > ___ > > firefox-dev mailing list > > firefox-...@mozilla.org > > https://mail.mozilla.org/listinfo/firefox-dev > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
Non-e10s is such a different environment that I don't think we have any hope of keeping it working without running the full test suite in that mode and I don't think anyone wants to do that. Now that this has started breaking I think it is actively harmful to our users for us to allow them to disable e10s. On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch wrote: > (Copied to fx-dev; Replies to dev-platform please.) > > Hello, > > Just over a year ago, I started a discussion[0] about our support for > disabling e10s. The outcome of that was that we removed support for > disabling e10s with a pref on Desktop Firefox with version 68, except > for use from automation. We kept support for using the environment > variable. [1] > > Last week, we released Firefox 77, which turned out to break all > webpages sent using compression (like gzip) if you had disabled e10s > using this environment variable. [2] > > So here we are again. I'd like to propose we also stop honouring the > environment variable unless we're running tests in automation. We > clearly do not have sufficient test coverage to guarantee basic things > like "the browser works", it lacks security sandboxing, and a number of > other projects require it (fission, gpu process, socket process, ...), > so I think it's time to stop supporting this configuration at all. > > I hope to make this change for the 79 cycle. I'm open to arguments > either way about what to do for 78 esr (assuming the patch for 79 turns > out to be simple; the work to remove the pref had a number of annoying > corner-cases at the time). > > Please speak up if you think that this plan needs adjusting. > > ~ Gijs > > > [0] > > https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 > ___ > firefox-dev mailing list > firefox-...@mozilla.org > https://mail.mozilla.org/listinfo/firefox-dev > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
(Copied to fx-dev; Replies to dev-platform please.) Hello, Just over a year ago, I started a discussion[0] about our support for disabling e10s. The outcome of that was that we removed support for disabling e10s with a pref on Desktop Firefox with version 68, except for use from automation. We kept support for using the environment variable. [1] Last week, we released Firefox 77, which turned out to break all webpages sent using compression (like gzip) if you had disabled e10s using this environment variable. [2] So here we are again. I'd like to propose we also stop honouring the environment variable unless we're running tests in automation. We clearly do not have sufficient test coverage to guarantee basic things like "the browser works", it lacks security sandboxing, and a number of other projects require it (fission, gpu process, socket process, ...), so I think it's time to stop supporting this configuration at all. I hope to make this change for the 79 cycle. I'm open to arguments either way about what to do for 78 esr (assuming the patch for 79 turns out to be simple; the work to remove the pref had a number of annoying corner-cases at the time). Please speak up if you think that this plan needs adjusting. ~ Gijs [0] https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
