159261 - installer fails to shutdown the browser appropriately the first time around

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect)

Description

[Sean Su]

The problem is that when the installer dectects that the browser is running, it
will ask the user to either shut it down, or click OK and let it shut it down
for them.

On occasion, if the uses clicks on OK, the same dialog will come up again.  This
is because the way the installer shuts down the browser is by simply sending a
WM_QUIT message to all the *visible* windows.

This in turn tells the browser that it needs to be completely shutdown.  There
are times that this part (after all the visible windows have been closed) takes
a long time.  The installer then returns from sending all the WM_QUIT messages
and does another check (just to be sure) and dectects that it has not shutdown yet.

The fix is to also send WM_QUIT messages to all of the *invisible* windows
_after_ the *visible* windows have been closed.

Patch coming right up.

Comment 1

[Sean Su]

Attachment: patch v1.0

Comment 2

[Daniel Veditz [:dveditz]]

Since you've added the patch --> ssu

Comment 3

[Sean Su]

Attachment: patch v1.1

the previous patch was bitroted.  Here's an updated one also with clearer
comments.

Comment 4

[Curt Patrick (gone)]

Comment on attachment 92694 [details] [diff] [review]
patch v1.1

r=curt

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 92694 [details] [diff] [review]
patch v1.1

> DWORD   PX_THREAD;
>++DWORD   gCloseWindowProcessPass;

That extra plus isn't actually in the file, is it?

>+// This function will close all the visible and invisible windows that match
>+// a processe's id passed in from lParam.
> BOOL CALLBACK EnumWindowsProc(HWND hwnd, LPARAM lParam)

Not the best name... this function itself doesn't enumate anything, and
the fact that it's closing things is obscure. "CloseWindowProc" maybe?
"CloseEnumeratedWindow"?

The global 'pass' variable is icky, it should be an argument
to this function. Ah, pass 2 doesn't close anything. Ok, combine
these two and you could stick with the EnumWindowProc name with an
argument that says what to do, #defined to something readable.
I guess that means you'd have to define a struct to pass in since
you're stuck with just a lparam arg.

Types could be CLOSE_VISIBLE, CHECK_VISIBLE, and CLOSE_ALL or
something along those lines.

I guess if we were rushing to get it into the branch I could
live with it, but since you said this is for the trunk you can
go ahead and make the behavior a little less magical.

Comment 6

[Henrik Gemal]

what's the status on this baby? I sometimes have "hanging" mozilla threads that
the installer doesn't shutdown so that I after install I get the restart dialog...

Comment 7

[Sean Su]

fix is now part of the patch to bug 186703

Comment 8

[Sean Su]

bug 186703 has been fixed.  marking this bug fixed too.

Comment 9

[Grace Bush]

verified 1.3