59105 - chrome install hangs when target is not a zip archive.

Status: VERIFIED FIXED

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

Description

[John Morrison]

Overview Description:  

   If the target file of a 'InstallTrigger.installChrome(...)' does not
   exist, or is not a zip file (e.g. 'text/html'), then the application
   will hang.

Steps to Reproduce: 
  http://jrgm.mcom.com/bugs/58515/install-TEXT-HTML.html and click the 
  Install button.

Actual Results:   Hang
Expected Results: Error message/ abort the install
Reproducibility: 100%

Build Date & Platform Bug Found: 
   2000110309 win2k MN6 branch build

Additional Builds and Platforms Tested On: 
   didn't test mac/linux (it's late and I'm going home now).   

Additional Information: 

 Obviously, this is a stupid (or malicious) thing to do. In my case, I
 did this by accident (also known as Stoopidity).

 NOTE: this occurs whether the user selects "Use this Skin" or not. It
  occurs during the download/install phase.

Stack for the hang is:

NTDLL! 77f8fb68()
_PR_MD_READ(PRFileDesc * 0x00da7b78, void * 0x0012e1ec, int 22) line 277 + 30 
bytes
FileRead(PRFileDesc * 0x00da7b78, void * 0x0012e1ec, int 22) line 54 + 17 bytes
PR_Read(PRFileDesc * 0x00da7b78, void * 0x0012e1ec, int 22) line 120 + 20 bytes
nsZipArchive::BuildFileList() line 691 + 30 bytes
nsZipArchive::OpenArchiveWithFileDesc(PRFileDesc * 0x00da7b78) line 321
nsJAR::Open(nsJAR * const 0x039826c8) line 233 + 15 bytes
nsZipReaderCache::GetZip(nsZipReaderCache * const 0x03972c58, nsIFile * 
0x039789d8, nsIZipReader * * 0x036ccc04) line 1213 + 12 bytes
nsJARChannel::EnsureZipReader() line 744 + 62 bytes
nsJARChannel::Open(nsJARChannel * const 0x036ccbb8, char * * 0x00000000, int * 
0x00000000) line 754 + 11 bytes
nsJARChannel::OpenJARElement() line 255 + 20 bytes
nsJARChannel::OnDownloadComplete(nsJARChannel * const 0x036ccbbc, nsIDownloader 
* 0x03978720, nsISupports * 0x00000000, unsigned int 0, nsIFile * 0x039789d8) 
line 667 + 11 bytes
nsDownloader::Init(nsDownloader * const 0x03978720, nsIURI * 0x03987e40, 
nsIDownloadObserver * 0x036ccbbc, nsISupports * 0x00000000, int 1, nsILoadGroup 
* 0x00000000, nsIInterfaceRequestor * 0x00000000, unsigned int 2048, unsigned 
int 16384, unsigned int 262144) line 65 + 55 bytes
NS_NewDownloader(nsIDownloader * * 0x036ccc24, nsIURI * 0x03987e40, 
nsIDownloadObserver * 0x036ccbbc, nsISupports * 0x00000000, int 1, nsILoadGroup 
* 0x00000000, nsIInterfaceRequestor * 0x00000000, unsigned int 0, unsigned int 
16384, unsigned int 262144) line 337 + 59 bytes
nsJARChannel::EnsureJARFileAvailable() line 345 + 126 bytes
nsJARChannel::OpenInputStream(nsJARChannel * const 0x036ccbb0, nsIInputStream * 
* 0x0012ecf8) line 268 + 8 bytes
rdf_BlockingParse(nsIURI * 0x03987cc0, nsIStreamListener * 0x03894888) line 606 
+ 39 bytes
RDFXMLDataSourceImpl::Refresh(RDFXMLDataSourceImpl * const 0x0389487c, int 1) 
line 946 + 58 bytes
nsChromeRegistry::InstallProvider(nsChromeRegistry * const 0x00c2e248, const 
nsCString & {...}, const nsCString & {...}, int 1, int 0, int 0) line 1749
nsChromeRegistry::InstallSkin(nsChromeRegistry * const 0x00c2e248, const char * 
0x03709448, int 1, int 0) line 2096 + 46 bytes
XPTC_InvokeByIndex(nsISupports * 0x00c2e248, unsigned int 19, unsigned int 3, 
nsXPTCVariant * 0x0396fd60) line 139
EventHandler(PLEvent * 0x036ed418) line 513 + 41 bytes
PL_HandleEvent(PLEvent * 0x036ed418) line 576 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00b27130) line 509 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x000f07f4, unsigned int 49483, unsigned int 0, 
long 11694384) line 1054 + 9 bytes
USER32! 77e13eb0()
USER32! 77e1401a()
USER32! 77e192da()
nsAppShellService::Run(nsAppShellService * const 0x00b6ead0) line 408
main1(int 1, char * * 0x00497ed0, nsISupports * 0x00000000) line 1011 + 32 
bytes
main(int 1, char * * 0x00497ed0) line 1251 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

Comment 1

[Daniel Veditz [:dveditz] out until Nov 10]

Reassigning to Sean who already has a fix for this.

Comment 2

[Sean Su]

Attachment: patch #1

Comment 3

[Sean Su]

I've also cleaned up the ProcessWindowsMessages() routine in the patch.  It's
needed to remedy an annoying behavior in the standalone build.

Looking for reviewers now.

Comment 4

[Daniel Veditz [:dveditz] out until Nov 10]

I'd prefer that the ProcessWindowsMessages() calls remain inside "#if defined 
STANDALONE && defined WIN32" blocks. The code *looks* cleaner the way you've 
done it, but might be very confusing to people trying to understand why we're 
calling that in the Mozilla-build-on-unix case (for example).

If you make that change you can move ProcessWindowsMessages() definition inside 
the existing STANDALONE ifdef and just put #ifdef WIN32 around it. (Why WIN32? 
why not XP_WIN?)

otherwise, r=dveditz

Comment 5

[Sean Su]

okay, new patch coming up with the suggested changes.  WIN32 was used because it
was from the legacy code.  I've now changed it to use X_WIN.

Comment 6

[Sean Su]

Attachment: patch #2

Comment 7

[Daniel Veditz [:dveditz] out until Nov 10]

Thanks for the update, r=dveditz for the second patch.

I'm surprised Mac (at least) doesn't need to do the same thing in standalone. 
If it does we can in the future change the routine name to something like 
ProcessNativeEventLoop() and then change the (STANDALONE && XP_WIN) wrappers 
around the call to just STANDALONE (or STANDALONE && !XP_UNIX).

But not now! The routine does one thing, its name accurately reflects what it 
does, and the #ifdeffing around the calls both makes it clear when it's called 
and avoids the wasted function call overhead in the normal case.

Comment 8

[Daniel Veditz [:dveditz] out until Nov 10]

Moz 0.8 tasks 

Comment 9

[Scott MacGregor]

sr=mscott on the 2nd patch.

Comment 10

[Sean Su]

patch #2 checked in.

Comment 11

[Jimmy Lee]

Build: 2001-02-05-21-Mtrunk(WIN)

No more crashing.  After clicking Install button and clicking OK button to 
install theme, nothing is installed and control is restored to browser.

Marking Verified.

Comment 12

[Jimmy Lee]

Build: 2001-02-05-11-Mtrunk (Linux), 2001-02-05-12-Mtrunk(MAC)

Ok on Linux and Mac.