1926812 - Update the remaining in-tree consumers of -no-remote command-line argument

Status: NEW

(Toolkit :: Startup and Profile System, defect)

Description

[Tooru Fujisawa [:arai]]

Bug 1906260 removed the -no-remote command-line argument, but there are some remaining references
https://searchfox.org/mozilla-central/search?q=-no-remote&path=

Although some of them might be necessary to support older binaries, but others need update, either to just drop it or replace with -new-instance.
Also code that intend a backward compatibility would require an explicit comment.

Maybe each change can be done in separate bug in its own component, but filing a bug here to track the work.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1906260

:mossop, since you are the author of the regressor, bug 1906260, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Dave Townsend [:mossop]]

Are any of those uses actually broken in any way? I would not expect them to be since we ignore the argument when it is provided.

Comment 3

[Tooru Fujisawa [:arai]]

Possible case is bug 1926803 , not yet confirmed to be directly causing the trouble, but at least the code there isn't working as expected, to my understanding.

My understanding is that, some -no-remote consumers need to be rewritten to use -new-instance , in order to reflect the original intent.

Comment 4

[Dave Townsend [:mossop]]

(In reply to Tooru Fujisawa [:arai] from comment #3)

My understanding is that, some -no-remote consumers need to be rewritten to use -new-instance , in order to reflect the original intent.

Unless they want to see the profile locked dialog or the profile manager -new-instance should not be necessary. If you find other cases where it is please let me know.

Comment 5

[Tooru Fujisawa [:arai]]

(In reply to Dave Townsend [:mossop] from comment #4)

(In reply to Tooru Fujisawa [:arai] from comment #3)

My understanding is that, some -no-remote consumers need to be rewritten to use -new-instance , in order to reflect the original intent.

Unless they want to see the profile locked dialog or the profile manager -new-instance should not be necessary.

Here's my understanding of the bug 1926803's consumer.
The code is launching a new firefox process from an existing firefox process, where the new process is for the Browser Toolbox, and the existing process is going to be a debuggee.
The -no-remote argument had been used there to avoid using the "remote" feature while starting the Browser Toolbox process, in order to avoid processing the command line in the existing debuggee process or somewhere else.

The no-remote flag had been setting the mDisableRemoteClient flag, which is used as a condition to use the remote feature or not.
https://hg.mozilla.org/mozilla-central/rev/47ef7f983aba#l22.99

-  ar = CheckArg("no-remote");
-  if (ar == ARG_FOUND || EnvHasValue("MOZ_NO_REMOTE")) {
-    mDisableRemoteClient = true;

Then, I see the remote handling code path is now taken in the Browser Toolbox process during the process startup.
Although it doesn't result in opening the browser toolbox's window in the debuggee for me, there's a report that the window is opened in the debuggee process, instead of a new Browser Toolbox process, and to my understanding the remote feature can cause this case.

Then, in order to keep the previous behavior around the process startup, the alternative option is to use the -new-instance argument,
that also sets the mDisableRemoteClient flag, and so that the process doesn't try processing the command with "remote" feature:
https://searchfox.org/mozilla-central/rev/dca2603d55b5b39d3b8ab8e93c08b42563f5aad8/toolkit/xre/nsAppRunner.cpp#4340-4342

ar = CheckArg("new-instance");
if (ar == ARG_FOUND || EnvHasValue("MOZ_NEW_INSTANCE")) {
  mDisableRemoteClient = true;

Am I misunderstanding something?

Comment 6

[Dave Townsend [:mossop]]

(In reply to Tooru Fujisawa [:arai] from comment #5)

Am I misunderstanding something?

Note the line above. The remoting service is supposed to be specific to the selected profile directory, so a new toolbox process sending its command line to an existing running instance of Firefox should only happen if it is attempting to use the same profile directory that Firefox is already using. If we're somehow sending to a running instance using a different profile directory then that is a bug in the remoting service that we need to fix, using -new-instance would just wallpaper over it. It would be extremely useful if we can get steps to reproduce such a case.

Comment 7

[Tooru Fujisawa [:arai]]

(In reply to Dave Townsend [:mossop] from comment #6)

(In reply to Tooru Fujisawa [:arai] from comment #5)

Am I misunderstanding something?

Note the line above. The remoting service is supposed to be specific to the selected profile directory, so a new toolbox process sending its command line to an existing running instance of Firefox should only happen if it is attempting to use the same profile directory that Firefox is already using. If we're somehow sending to a running instance using a different profile directory then that is a bug in the remoting service that we need to fix, using -new-instance would just wallpaper over it. It would be extremely useful if we can get steps to reproduce such a case.

Thank you for the explanation.
It makes sense.
and, I think I figured out the underlying bug.
I'll file a bug shortly.

Comment 8

[Tooru Fujisawa [:arai]]

Now the issue with the remote handling is fixed by bug 1926884,
this bug is mostly about removing no-op argument, or update comments, or add a comment that says why the argument is still used there (such as, the code is targeting older version, etc)