81706 - disable -turbo for multiple profile case

Status: VERIFIED FIXED

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Cathleen]

when running in -turbo mode (bug 75599), users with more than one profile will 
see profile manager come up.

selmer said that bhuvan can do a 2 line fix to prevent profile manager comes 
up. Wanted to make sure a bug is filed, and get bhuvan to help us implement the 
fix.

Comment 1

[Bill Law]

Attachment: Bhuvan's patch for profile manager

Comment 2

[Bill Law]

Patch attached; I haven't tried that, though.

Comment 3

[racham]

Reassigning to Conrad to take over from here...

Comment 4

[selmer (gone)]

Does this patch allow the case where -P is used to go into -turbo mode?

Comment 5

[Conrad Carlen (not reading bugmail)]

Steve: This will only effect the "-P" case when the profile used with "-P does
not exist. Assuming the profile exists, "-P" will start the program with that
profile and no UI. If that profile does not exist, the UI would come up and this
patch will supress that.

Bill, Bhuvan: What this patch ends up doing in the multiple profile "-turbo"
case is to select the first profile. The patch prevents the UI from coming up
but, if after that point, there is no current profile, the first profile will be
made the current profile. Is that the desired result? In some prior email, you
said that, in this case, the profile mgr should return an error and then, when
the program was *really* started, they get the profile mgr window. 

Comment 6

[Bill Law]

I envisioned that cases that would otherwise show the Profile Manager
profile-chooser would return an error in "turbo mode."  That way, user's don't
see any functional difference if the inadvertently choose this option when they
have multiple profiles.

But that's just my opinion.

When you say "the first profile" do you mean the first alphabetically, or what?
 If it were the one most recently used or something, that would seem more likely
to be the one they actually wanted.

Comment 7

[Conrad Carlen (not reading bugmail)]

> I envisioned that cases that would otherwise show the Profile Manager
> profile-chooser would return an error in "turbo mode."

That error would have to be handled somewhere. Currently, if an error is
returned from profile initialization, it will propagate all the way out of main1
and the program will terminate.

Actually, if you don't want profile mgr UI to show at startup while in server
mode, there's a better way: use the profile mgr like embedding apps do. Don't
call StartupWithArgs, but instead do something like this:

http://lxr.mozilla.org/seamonkey/source/embedding/browser/powerplant/source/CProfileManager.cpp#95

Here, the embedding app is made to deal with the case of not showing UI at
startup. And, it requires no changes to profile mgr and profile mgr does not
need to know about server mode. It's slightly more work, but I think better to
change the startup code in nsAppRunner.cpp. It's better to confine the changes
for server mode to there.

> When you say "the first profile" do you mean

Sorry, I was wrong. It's not the first profile, but the current profile which,
at this point, is the last used profile.

Comment 8

[Conrad Carlen (not reading bugmail)]

Just so we know the issues, here are the cases that we can encounter when
starting up mozilla in "server" mode and what I think should happen in each case:

I. Starting up in server mode
    1. No profiles
        - create default profile, start with it
    2. 1 4x profile
        - auto migrate it (without UI), start with it
    3. 1 6x profile
        - start with it
    4. Multiple 6x profiles
        - start with last used profile, return interractionNeeded
    5. Multiple 4x profiles, no 6x profile
        - create and start with default profile, return interractionNeeded

To do this, I propose we add a new method to nsIProfileInternal:

[noscript] void startUp(in boolean allowInterraction);

This method could replace startupWithArgs (which needs to die). When used from
"server" mode, it would be passed PR_FALSE for allowInterraction. This would
always put the program in a runnable state, but not nescesarily the right
profile. Then, when starting the program for real, it could be called again,
passing PR_TRUE. This would allow proper selection of profiles, migration, etc.
 

Comment 9

[Cathleen]

I can see people having problem with case #4 (unless I'm missing something)

 4. Multiple 6x profiles
    - start with last used profile, return interractionNeeded

I am not sure starting with the last used profile is a good thing, if people are
sharing the same machines at home or at school lab... wouldn't this cause
privacy issues?

Is there a way to load up everything to the point where profile picker comes up?

Comment 10

[Conrad Carlen (not reading bugmail)]

> I am not sure starting with the last used profile is a good thing,
I'm not saying we start the normal run of the program with the last used
profile. We just start the server in this way. When the program is started for
real, the profile selection dialog comes up and they get to choose the right
profile. 

Comment 11

[Cathleen]

got it!  :-)  that sounds good.  thanks!  

Comment 12

[Conrad Carlen (not reading bugmail)]

The plan is, when starting the app in server mode and there are multiple
profiles, start with the last used profile or a temp profile and then later show
the profile UI when the app is started for real.

The problem with this is the cached browser window. It is made at server startup
time - a time at which we may not have the right profile yet. What if the user's
different profiles have diferent UI locales? Say we start server mode and create
the cached window's chrome in en-US but then, when the program is started for
real, the user picks a profile which is ja-JP. That cached window's chrome had
better become ja-JP.

As a function of profile switching, this *may* work - I'll find out ASAP.
 

Comment 13

[Conrad Carlen (not reading bugmail)]

In today's performance meeting, the consensus was that we didn't need
-turbo/-server to work properly in the multiple profile case before RTM. That
instead, an error should be returned by the profile mgr when it would have to
put up UI. I don't see the point in that. That would mean, in -turbo/-server
mode with multiple profiles, the program wouldn't run. If we do nothing, the
profile mgr needing to put up UI will prevent -turbo/-server from happening.
That, IMO, is better than just not being able to run. 

Comment 14

[selmer (gone)]

Conrad, that is not currently the case.  Right now, the profile picker comes up
when -server is used.  This doesn't _prevent_ -turbo, it just requires the user
to respond to something they don't expect at startup and then -turbo continues.

We want to punt on the multi-profile issues for RTM because they're complex and
only affect a smallish portion of the market.  Worse than not giving those folks
this feature is to give them something very broken that makes them give up on
us.  Any solution that has us guessing the profile for them is going to cause
problems in the RTM timeframe.

We previously discussed stopping before picking the profile, but this doesn't
give enough improvement to be worth the headaches right now.

Let's stick with what we agreed to in the meeting:  put in the fix to disable
-turbo in the multiple profile case (unless the -P option was explicitly passed
along with -turbo) and file another bug about how that sucks and we should fix
it after RTM.

Comment 15

[Conrad Carlen (not reading bugmail)]

Steve, here is what currently happens with a build from yesterday:

1. With one profile
   mozilla -server - works as expected, no initial browser window
2. With > one profile
   mozilla -server - profile picker comes up, pick profile, browser starts with
a visible window. When this window is closed, the program exits. Exact same
behavior as if you started without -server.

Here's why the appearance of a profile dialog does in fact turn off -server
mode: Profile mgr, when putting up dialogs, gets the appshell service and spins
up an event loop. The profile mgr front-end code calls nsIAppShellService::Quit
to end the modal loop. As you can see here:
http://lxr.mozilla.org/seamonkey/source/xpfe/appshell/src/nsAppShellService.cpp#434,
this destroys the native app support and thus -server mode.

Comment 16

[timeless]

if that's the case then create profile, profile manager, installer and anything 
else that creates profiles or enables -turbo MUST warn the user.

Comment 17

[timeless]

sorry, that's a response to Additional Comments From Conrad Carlen 2001-05-31 
15:31

Comment 18

[Judson Valeski]

what's the latest w/ this? is this bug still valid now that we're no longer
persuing multi profile turbo mode? or should the summary be changed?

Comment 19

[Conrad Carlen (not reading bugmail)]

Changing the summary and futuring. As I explained 2001-06-01 06:31, any case in
which the profile mgr would have to put up UI *will* turn off -turbo mode. Given
that we're not properly supporting multiple profiles for -turbo mode for RTM,
there's nothing to do here. I don't think that the profile mgr should pop a
warning saying that -turbo mode is being supressed. I think that's best left to
the installer or a release note.

Comment 20

[Cathleen]

ccarlen, do we have everything we need in the tree to
1) disable -turbo mode in multiple profile cases and 
2) enable -turbo mode when users launch by using -p with specific profile?

Comment 21

[Conrad Carlen (not reading bugmail)]

Catleen - Yes.

Comment 22

[Bill Law]

The original bug comment said we want to prevent the profile picker dialog
appearing when starting with "-turbo mode."  I think that's what we want.
It doesn't work that way now.

Comment 23

[Conrad Carlen (not reading bugmail)]

Preventing the UI from showing is *not* what we want. See the last paragraph of
selmer's comment from 001-06-01 01:38.

The reason it's not what we want is that if you just supress the UI in a case in
which it's needed, the only options are to:
(1) Just pick the last used profile. That idea was very unpopular.
(2) Fail. There is no point in failing unless the app startup code can remember
the failure and then present the dialog later when it *is* appropriate to show it. 

Comment 24

[Cathleen]

so, I think there is a confusion here.  
I thought we want to bail out for multiple profile users because the profile
picker dialog gets displayed...  And if we make mozilla -turbo to start at
machine boot time, it's going to look real weird that our profile picker comes
up.  I believe we want to prevent profile picker dialog from coming up in
conjuction with not allow -turbo to work for multiple profile users.

Comment 25

[Conrad Carlen (not reading bugmail)]

Attachment: patch to die silently-1

Comment 26

[Conrad Carlen (not reading bugmail)]

Attachment: patch to die silently-part2

Comment 27

[Bill Law]

Thanks, Conrad.  r=law

But I have one suggestion: Can we extract the "get server mode" code into a
little static utility function and re-use that in this new place, and, where it
already exists in Ensure1Window?  I hate to see the same code replicated more
than once and the need to call it has a tendency to expand over time.

Comment 28

[Conrad Carlen (not reading bugmail)]

> But I have one suggestion: Can we extract the "get server mode" code

Definately. I should have known it was already in nsAppRunner. I'll make a new
patch.

Comment 29

[Blake Ross]

+#ifndef nsError_h
+#include "nsError.h"
+#endif

Do we want to #define nsError_h in that case?

Nit: one 'r' in "interact"/"interaction"?

I think a helper function to retrieve the server mode is a good idea.

sr=blake

Comment 30

[timeless]

blake: no nsError.h defines it.

Comment 31

[Conrad Carlen (not reading bugmail)]

Attachment: new patch - part 1

Comment 32

[Conrad Carlen (not reading bugmail)]

Attachment: new patch part-2

Comment 33

[Conrad Carlen (not reading bugmail)]

New patches:

(1) It's not nescesary to include nsError.h in any .idl file because it gets
included already in order for nsresult to be defined.
(2) Misspellings of interact are gone.
(3) The static routine in nsAppRunner.cpp actually needed to be two if anything.
One caller needed the nsINativeAppSupport, one just needed to know if we were in
server mode.

Comment 34

[Cathleen]

update summary and target milestone...  :-)

Comment 35

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 36

[Conrad Carlen (not reading bugmail)]

Fix checked in. This is hopefully a temporary measure. Continuing to work on
making multiple profiles work - bug 86021.

Comment 37

[K'Trina Medina]

Verified on branch build 2001071306
Added vtrunk 

Comment 38

[K'Trina Medina]

Verified on branch and trunk (build 2001072406)