Closed Bug 35559 Opened 20 years ago Closed 19 years ago

Profile Manager Needs to support Dynamic switching

Categories

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

x86
Other
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.8

People

(Reporter: travis, Assigned: ccarlen)

References

Details

(Keywords: embed, Whiteboard: [nsbeta2-][nsbeta3-][PDTP1])

Attachments

(8 files)

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.
Blocks: 35556
Sorry for the spam, changing QA contact.
QA Contact: paulmac → jrgm
until conrad starts
Assignee: travis → valeski
nom.
Keywords: nsbeta2
Putting on [nsbeta2-] radar. Not critical to beta2.  Adding "nsbeta3" keyword 
for consideration of a fix for that milestone. 
Keywords: nsbeta3
Whiteboard: [nsbeta2-]
Keywords: embed
over to conrad. and plussing.
Assignee: valeski → conrad
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]
Target Milestone: --- → M18
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.  
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.
changing to new email
Assignee: conrad → ccarlen
re-accepted at new email
Status: NEW → ASSIGNED
minusing per PDT rules.
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3-]
bumping up again.
Priority: P3 → P1
Whiteboard: [nsbeta2-][nsbeta3-] → [nsbeta2-][nsbeta3+]
Profile migration must work (except possibly for bizarre strange cases).  Seems 
this one is taken care of.  PDT marking P1 to encourage checkin.
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3+][PDTP1]
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.
Whiteboard: [nsbeta2-][nsbeta3+][PDTP1] → [nsbeta2-][nsbeta3-][PDTP1]
Target Milestone: M18 → mozilla0.8
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
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 :-/
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?)
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.
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
The above patch (22053) has r=valeski,a=hyatt from email. Just putting here for
safe keeping.
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;
Keywords: approval, patch, review
> 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

timeless, the style problems you pointed out have been fixed in all patches.
sue me. then talk to someone about NULL and nsnull.  otherwise i really like 
your work.
r=darin on nsHTTPHandler patch
looks good to me sr=mscott ont he http handler patch. 
Blocks: 64833
Updating QA Contact
QA Contact: jrgm → mdunn
FYI, this change has added a dependency on profile from netwerk and broke the
senna tinderbox.
sr=alecf on both the history and bookmarks patches
This bug seems to have had enough review and super-review.  Does anyone on the
cc list disagree?

/be
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.
sr=alecf on the cookie and wallet patches - thanks for that extra clean up too!
Cookie and Wallet code checked in. With that - all patches are in. Waiting to
claim Fixed until PSM mods are working and checked in.
Keywords: review
The patch to wallet used FALSE instead of PR_FALSE.
r=bryner on latest patch
See also bug 60299, need "Profile Manager" option on the Tasks menu.
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.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Correction: Changing QA contact for the Embed API bugs to David Epstein.
QA Contact: mdunn → depstein
Profile switching is in both MFCEmbed and WinEmbed. SetCurrentProfile() is the 
method used for profile switching and it works (I tried it out).
Status: RESOLVED → VERIFIED
No longer blocks: 64833
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.