86976 - xpinstall needs to shutdown -turbo mode for file replace/remove

Status: VERIFIED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect)

Description

[Cathleen]

right now -turbo mode is windows only feature.

-turbo mode will start an instance of the app and keep it running in the
backgroud, without displaying any app window.

so, this can be a problem for xpintall when doing an update, we need a way to
tell the app to "really quit", so we can replace/delete files that are in use.

Comment 1

[dbragg]

I'm pretty sure this is going to be ugly.  We've had problems with the cleanup
routines since day one.  This turbo-mode thing is going to complicate things.

Comment 2

[Cathleen]

yup... unfortunately  :-(

Comment 3

[Sean Su]

Given that:
1) xpicleanup will place itself into the windows RunOnce regsitry key
2) the browser will not startup if a cleanup is required (a message is displayed 
indication so).  I hope this is true regardless if the -turbo is passed on the 
command line.

I think one solution is to have the the web page require the user to restart the 
system (as opposed to only restarting the browser) on any win os's (nt or 9x) 
when a restart is required.  This way, it'll give xpicleanup time to do it's 
thing.

This should solve the problem with the -turbo without requiring xpicleanup to 
force a quit of the browser (which in itself has problems).  Don what do you 
think?

Comment 4

[Sean Su]

Having the user require a restart of the system does take away the reason for 
having xpicleanup in the first place.

Perhaps we should offer the ability to check the browser prefs to see if the 
browser is running in turbo mode.  This way, if they are not,  then a simple 
restart of the browser should update as expected. 

Comment 5

[Cathleen]

> Perhaps we should offer the ability to check the browser prefs to see if the 
> browser is running in turbo mode.  This way, if they are not,  then a simple 
> restart of the browser should update as expected. 

I agree!

Comment 6

[Cathleen]

sean is this the bug that you said you and don are working on??  in the cleanup 
routine?

Comment 7

[Sean Su]

yeah.  I think there are several bugs related to this one.  we'll have to start 
dup'ing.

Comment 8

[Cathleen]

as long as we make one of them link to 75599, I don't mind which one is open :-)

Comment 9

[dbragg]

Ok, it's looking like things are getting nasty with this turbo mode thing.

XPInstall only kicks off the cleanup utility when it gets a shutdown message
(we're set up as a shutdown observer). It looks like 89156 is showing that the
shutdown routine is not being called so the xpicleanup isn't even being kicked
off.  This other problem with this is that you can't call netscape -kill (is
that right Sean?) to shut off turbo mode, again because the shutdown routine
isn't being called. 

To see what I'm talking about look at
mozilla/xpinstall/src/nsSoftwareUpdate.cpp. You should see the ::Shutdown*=()
routine at line 191.

Comment 10

[Jimmy Lee]

Adding K'Trina and Grace to keep in the loop.

Comment 11

[lchiang]

Was this supposed to be nsbranch, cathleen?   I'm reviewing your email minutes
from -turbo mtg.

Comment 12

[selmer (gone)]

This bug seems to be SmartUpdate specific.  I thought the PDT+ bug was going to
shut down the app during a normal install.  I'm also wondering if this was
supposed to be just nsBranch instead of PDT+.  Syd?

Comment 13

[Syd Logan]

I think if you want people to successfully upgrade to 6.1.1 or 6.2 or e-mojo, we
better fix this. 

Comment 14

[Cathleen]

Yes, this was the one item that sean was talking about in the turbo summit, and 
I think this fell through the crack for a decision when meeting ends.  Sean had 
mentioned that there is another bug in xpinstall cleanup routine that will fix 
this bug, or at least covers in the same area... and would like to try getting 
this fixed when other fix lands.

If we don't fix this, we're going to have upgrade issues for users who have 
turbo turned on and going through smartupdate.

The summary email I sent out earlier included this wrong bug number, it 
should've been bug 87033 instead, for installer to kill -turbo, which is already 
fixed.

Comment 15

[Syd Logan]

Smartupdate is a non-issue, as you recall, it will not be supported. However,
the ability to exit the browser before installing is important for the stub
installer.

Comment 16

[Sean Su]

Actually, this is only a smartupdate related issue.  The native installer 
already knows how to shutdown the browser when it's in the turbo mode.  The fix 
to do this was checked in last week.

Reassigning this to myself.

Comment 17

[Syd Logan]

Fantastic, I got the two bugs confused.

So, does this, will this affect plugin vendors? 

Comment 18

[Sean Su]

if affects anyone trying to smartupdate files that are *in use*.  If they are 
*in use*, it will require a restart of the browser.  The workaround right now is 
to reboot the entire system.

Comment 19

[Viswanath Ramachandran]

Adding Dan. Sean - did you say that Dan was going to take this bug? 

Comment 20

[Daniel Veditz [:dveditz]]

taking bug

Comment 21

[Daniel Veditz [:dveditz]]

Have a fix for this, would like a r= from Sean or Syd.

Comment 22

[Daniel Veditz [:dveditz]]

Attachment: unreviewed fix "v1"

Comment 23

[Viswanath Ramachandran]

adding Conrad as he did the first of similar changes for theme switching. 

Comment 24

[Syd Logan]

r=syd

Comment 25

[Daniel Veditz [:dveditz]]

*** Bug 89156 has been marked as a duplicate of this bug. ***

Comment 26

[Scott MacGregor]

sr=mscott

Comment 27

[Daniel Veditz [:dveditz]]

Fixed on trunk and branch

Comment 28

[Jimmy Lee]

Build: 2001-07-17-05-0.9.2(WIN)

Works for me on both NT and 98.  Turbo is disabled when replacing a file in use.
 Rebooting is necessary to enable turbo once again.

Adding 'vtrunk' keyword.  This needs to be verified on the trunk.

Comment 29

[Jimmy Lee]

Build: 2001-08-02-06-trunk(WIN)

Works for me on both NT and 98.  Turbo is disabled when replacing a file in use.
Rebooting is necessary to enable turbo once again.

Removing 'vtrunk' keyword.  Marking Verified.