35559 - Profile Manager Needs to support Dynamic switching

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect, P1)

Description

[travis]

We need to be able to at runtime switch the profile that is currently in use.  
This is very important for embedding cases that provide any sort of user level 
separation.  The process may continue to be running yet they need to switch out 
the current profile to be a different user.

Comment 1

[shrirang khanzode]

Sorry for the spam, changing QA contact.

Comment 2

[Judson Valeski]

until conrad starts

Comment 3

[Judson Valeski]

nom.

Comment 4

[Jay Patel [:jay]]

Putting on [nsbeta2-] radar. Not critical to beta2.  Adding "nsbeta3" keyword 
for consideration of a fix for that milestone. 

Comment 5

[Judson Valeski]

over to conrad. and plussing.

Comment 6

[Conrad Carlen]

The plan is to use nsIProfileStartupListener or something similar. Code which is 
dependent on the current profile will register as a listener with the profile 
service. Before switching the profile, the profile service will traverse this 
list. The listener has to either break whatever ties it has to the current 
profile OR return a value which indicates "No, I can't or won't allow it to 
change." At this point, iteration stops and the profile can't be switched. This 
can happen in the case of a user downloading a large file. This listener might 
ask "Are you sure you want to cancel this download?" If the listener was a 
browser window, it could easily stop activity by using nsIWebNavigation::Stop. If 
the first notification round completes without being canceled, the profile 
service switches the profile, then calls the listeners to tell them the profile 
has been changed. Then the listeners respond to that. For example, nsIPref will 
be a listener. It will then load the prefs from the new profile and then the pref 
changing mechanism that nsIPref already has will handle whatever changes that 
causes.  

Comment 7

[Alec Flett]

this sounds fantastic.
it sounds like we need to add one thing to the profile startup listener,
something like

readonly attribute boolean canSwitchProfiles;

which would return whether or not the listener can actually switch profiles

the basic registration mechanism is already there, and in use via the category
manager. one trick is that the profile startup does a createInstance of each
startup listener.. this means that existing objects can't be startup listeners.
My thoughts on this are:
- switch to GetService and make the requirement that the CID that is the
listener is a service, not an object. The unfortunately side effect of this is
that the service will stick around after startup, even if it isn't used.
- add an additional method registerStartupListener to nsIProfile, which allows
you to add startup listeners at runtime. I prefer this option.

Comment 8

[Judson Valeski]

changing to new email

Comment 9

[Conrad Carlen (not reading bugmail)]

re-accepted at new email

Comment 10

[Judson Valeski]

minusing per PDT rules.

Comment 11

[Judson Valeski]

bumping up again.

Comment 12

[clayton]

Profile migration must work (except possibly for bizarre strange cases).  Seems 
this one is taken care of.  PDT marking P1 to encourage checkin.

Comment 13

[lchiang]

per email with Jud, changing nsbeta3+ to nsbeta3- on all "embed" bugs since
embedding changes will not be made in the mn6 branch.  If you feel this bug fix
needs to go into mn6 branch, please list the reasons/user impact/risk and
nominate for rtm.  Thanks.

Comment 14

[Conrad Carlen (not reading bugmail)]

Attachment: patch to make cookie a profile change observer

Comment 15

[Conrad Carlen (not reading bugmail)]

Attachment: patch to make wallet a profile change observer

Comment 16

[Brendan Eich [:brendan]]

Even in the old days, an nsAutoString should have been used here rather than an
nsString:

+    // Register as an observer of form submission
     nsString  topic; topic.AssignWithConversion(NS_FORMSUBMIT_SUBJECT);

Nowadays, the right thing is NS_LITERAL_STRING(), which IIRC should be used as
the value of the #define'd NS_FORMSUBMIT_SUBJECT macro if possible.  Cc'ing scc
for latest advice.

-    rv = svc->AddObserver( this, topic.GetUnicode());
-    nsServiceManager::ReleaseService( NS_OBSERVERSERVICE_CONTRACTID, svc );
+    svc->AddObserver(this, topic.GetUnicode());
+    // Register as an observer of profile changes
+    svc->AddObserver(this, PROFILE_DO_CHANGE_TOPIC);

Must run, more in a bit.  alecf, scc, please feel free to review too.

/be

Comment 17

[Conrad Carlen (not reading bugmail)]

I agree, the macro NS_FORMSUBMIT_SUBJECT should be redefined to include the
NS_LITERAL_STRING macro. Problem was, the code in wallet which used it was there
before me and changing the macro would involve changing quite a few other files :-/

Comment 18

[Conrad Carlen (not reading bugmail)]

Attachment: patch to make prefs a profile change observer

Comment 19

[Alec Flett]

the prefs thing looks good, but while you're there, can you consolidate this
code with nsPref::ResetPrefs()? it seems like they do ALMOST the same thing (one
re-reads the pref defaults, one doesn't) but the end results should be the same..
(right?)

Comment 20

[Conrad Carlen (not reading bugmail)]

ResetPrefs() is quite different from ResetUserPrefs(). The first deletes the
whole hash table and destroys the js context - it's a total reset. What I needed
here was just to wipe the user prefs while leaving the default prefs alone.

Comment 21

[Alec Flett]

I guess what I'm wondering is if that's all ResetPrefs() needs to do too?
If not, then sr=alecf on that prefs stuff

Comment 22

[Conrad Carlen (not reading bugmail)]

Attachment: patch to make bookmarks a profile change observer

Comment 23

[Conrad Carlen (not reading bugmail)]

Attachment: patch to make history a profile change observer

Comment 24

[Conrad Carlen (not reading bugmail)]

Attachment: patch to make chrome reg a profile change observer

Comment 25

[Conrad Carlen (not reading bugmail)]

The above patch (22053) has r=valeski,a=hyatt from email. Just putting here for
safe keeping.

Comment 26

[timeless]

coding style comment [relatively important] reason: explicit comparison with 0.
+    if (nsCRT::strcmp(aTopic, PROFILE_BEFORE_CHANGE_TOPIC) == 0) {

ruleset:
given "if (a==0)" rewrite as "if (!a)".
given "if (a!=0)" rewrite as "if (a)".

file style comment [relatively trivial]; reason: indent consistency, 2 or 4 
spaces. reference "RCS file: 
/cvsroot/mozilla/extensions/cookie/nsCookieService.cpp,v"
+  if (observerService) {
+    observerService->AddObserver(this, PROFILE_DO_CHANGE_TOPIC);
+  }

+NS_IMETHODIMP nsCookieService::Observe(nsISupports *aSubject, const PRUnichar 
*aTopic, const PRUnichar *someData)
+{
+    nsresult rv = NS_OK;

Comment 27

[Conrad Carlen (not reading bugmail)]

> given "if (a==0)" rewrite as "if (!a)".
> given "if (a!=0)" rewrite as "if (a)".
will make that change in all patches

> indent consistency, 2 or 4 spaces
fixing that up too

Comment 28

[Conrad Carlen (not reading bugmail)]

timeless, the style problems you pointed out have been fixed in all patches.

Comment 29

[timeless]

sue me. then talk to someone about NULL and nsnull.  otherwise i really like 
your work.

Comment 30

[Conrad Carlen (not reading bugmail)]

Attachment: patch to make http handler profile change observer

Comment 31

[Darin Fisher]

r=darin on nsHTTPHandler patch

Comment 32

[Scott MacGregor]

looks good to me sr=mscott ont he http handler patch. 

Comment 33

[Paul Wyskoczka]

Updating QA Contact

Comment 34

[cls]

FYI, this change has added a dependency on profile from netwerk and broke the
senna tinderbox.

Comment 35

[Alec Flett]

sr=alecf on both the history and bookmarks patches

Comment 36

[Brendan Eich [:brendan]]

This bug seems to have had enough review and super-review.  Does anyone on the
cc list disagree?

/be

Comment 37

[Conrad Carlen (not reading bugmail)]

The first two patches (cookie & wallet) have been reviewed by morse not not
sr='d. Brendan, you began to sr, said, "more in a bit", ... Everything else is
checked in.

Comment 38

[Alec Flett]

sr=alecf on the cookie and wallet patches - thanks for that extra clean up too!

Comment 39

[Conrad Carlen (not reading bugmail)]

Cookie and Wallet code checked in. With that - all patches are in. Waiting to
claim Fixed until PSM mods are working and checked in.

Comment 40

[John G. Myers]

The patch to wallet used FALSE instead of PR_FALSE.

Comment 41

[John G. Myers]

Attachment: Fix reference to FALSE

Comment 42

[Brian Ryner (not reading)]

r=bryner on latest patch

Comment 43

[Jesse Ruderman]

See also bug 60299, need "Profile Manager" option on the Tasks menu.

Comment 44

[Conrad Carlen (not reading bugmail)]

All patches are checked in and this is fixed. To see profile switching in
action, build mozilla/embedding/browser/powerplant/PPBrowser.mcp. Profile
management UI is also being added to Win embedding samples.

Comment 45

[Michael Dunn]

Correction: Changing QA contact for the Embed API bugs to David Epstein.

Comment 46

[David Epstein]

Profile switching is in both MFCEmbed and WinEmbed. SetCurrentProfile() is the 
method used for profile switching and it works (I tried it out).