Profile Manager Needs to support Dynamic switching

VERIFIED FIXED in mozilla0.8

Status

()

Core
Embedding: APIs
P1
normal
VERIFIED FIXED
19 years ago
17 years ago

People

(Reporter: travis, Assigned: Conrad Carlen (not reading bugmail))

Tracking

({embed})

Trunk
mozilla0.8
x86
Other
embed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta2-][nsbeta3-][PDTP1])

Attachments

(8 attachments)

(Reporter)

Description

19 years ago
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.
(Reporter)

Updated

19 years ago
Blocks: 35556

Comment 1

18 years ago
Sorry for the spam, changing QA contact.
QA Contact: paulmac → jrgm

Comment 2

18 years ago
until conrad starts
Assignee: travis → valeski

Comment 3

18 years ago
nom.
Keywords: nsbeta2

Comment 4

18 years ago
Putting on [nsbeta2-] radar. Not critical to beta2.  Adding "nsbeta3" keyword 
for consideration of a fix for that milestone. 
Keywords: nsbeta3
Whiteboard: [nsbeta2-]

Updated

18 years ago
Keywords: embed

Comment 5

18 years ago
over to conrad. and plussing.
Assignee: valeski → conrad
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]

Updated

18 years ago
Target Milestone: --- → M18

Comment 6

18 years ago
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

18 years ago
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

18 years ago
changing to new email
Assignee: conrad → ccarlen
(Assignee)

Comment 9

18 years ago
re-accepted at new email
Status: NEW → ASSIGNED

Comment 10

18 years ago
minusing per PDT rules.
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3-]

Comment 11

18 years ago
bumping up again.
Priority: P3 → P1
Whiteboard: [nsbeta2-][nsbeta3-] → [nsbeta2-][nsbeta3+]

Comment 12

18 years ago
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]

Comment 13

18 years ago
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]

Updated

18 years ago
Target Milestone: M18 → mozilla0.8
(Assignee)

Comment 14

18 years ago
Created attachment 21683 [details] [diff] [review]
patch to make cookie a profile change observer
(Assignee)

Comment 15

18 years ago
Created attachment 21684 [details] [diff] [review]
patch to make wallet a profile change observer
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
(Assignee)

Comment 17

18 years ago
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 :-/
(Assignee)

Comment 18

18 years ago
Created attachment 21777 [details] [diff] [review]
patch to make prefs a profile change observer

Comment 19

18 years ago
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?)
(Assignee)

Comment 20

18 years ago
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

18 years ago
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
(Assignee)

Comment 22

18 years ago
Created attachment 22050 [details] [diff] [review]
patch to make bookmarks a profile change observer
(Assignee)

Comment 23

18 years ago
Created attachment 22051 [details] [diff] [review]
patch to make history a profile change observer
(Assignee)

Comment 24

18 years ago
Created attachment 22053 [details] [diff] [review]
patch to make chrome reg a profile change observer
(Assignee)

Comment 25

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

Comment 26

18 years ago
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
(Assignee)

Comment 27

18 years ago
> 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

(Assignee)

Comment 28

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

Comment 29

18 years ago
sue me. then talk to someone about NULL and nsnull.  otherwise i really like 
your work.
(Assignee)

Comment 30

18 years ago
Created attachment 22146 [details] [diff] [review]
patch to make http handler profile change observer

Comment 31

18 years ago
r=darin on nsHTTPHandler patch

Comment 32

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

Updated

18 years ago
Blocks: 64833

Comment 33

18 years ago
Updating QA Contact
QA Contact: jrgm → mdunn

Comment 34

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

Comment 35

18 years ago
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
(Assignee)

Comment 37

18 years ago
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

18 years ago
sr=alecf on the cookie and wallet patches - thanks for that extra clean up too!
(Assignee)

Comment 39

18 years ago
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

Comment 40

18 years ago
The patch to wallet used FALSE instead of PR_FALSE.

Comment 41

18 years ago
Created attachment 23396 [details] [diff] [review]
Fix reference to FALSE
r=bryner on latest patch

Comment 43

18 years ago
See also bug 60299, need "Profile Manager" option on the Tasks menu.
(Assignee)

Comment 44

18 years ago
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
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 45

17 years ago
Correction: Changing QA contact for the Embed API bugs to David Epstein.
QA Contact: mdunn → depstein

Comment 46

17 years ago
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

Updated

17 years ago
No longer blocks: 64833
You need to log in before you can comment on or make changes to this bug.