On 12/21/2017 5:55 PM, Ken Brown wrote:
On 12/20/2017 12:09 PM, Ken Brown wrote:
On 12/20/2017 11:19 AM, Jon Turney wrote:
On 19/12/2017 00:53, Ken Brown wrote:
The message box produced by TOPLEVEL_CATCH could be hidden by whatever
window was previously being displayed, so that setup appeared to hang.
Fix this by giving fatal error message boxes type MB_SETFOREGROUND.

This is good as far as it goes, but is kind of working around the fact that fatal() is being called with an NULL owner HWND.

I think I have a better solution, which simply avoids calling fatal() with a NULL owner HWND (with one exception).

New patch attached.

Ken
From b47adcb00209d717177d8932e8756736f61c80f2 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Mon, 18 Dec 2017 19:46:36 -0500
Subject: [PATCH setup 1/2] Give TOPLEVEL_CATCH an owner window

The fatal message box produced by TOPLEVEL_CATCH had a NULL owner
window.  This meant that the box could be hidden by whatever window
was previously being displayed, so that setup appeared to hang.  In
addition, the user could interact with the propsheet window while the
message box was displayed.

Fix this by giving TOPLEVEL_CATCH an "owner" parameter.  In all uses
but one (TOPLEVEL_CATCH("main")), there is a non-NULL owner available
that we can use.
---
 Exception.h    | 6 +++---
 download.cc    | 2 +-
 ini.cc         | 2 +-
 install.cc     | 2 +-
 main.cc        | 2 +-
 postinstall.cc | 2 +-
 prereq.cc      | 2 +-
 proppage.cc    | 2 +-
 site.cc        | 2 +-
 9 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Exception.h b/Exception.h
index 7b16612..4aeb49e 100644
--- a/Exception.h
+++ b/Exception.h
@@ -37,15 +37,15 @@ private:
 #define APPERR_IO_ERROR                2
 #define APPERR_LOGIC_ERROR     3
 
-#define TOPLEVEL_CATCH(threadname)                                      \
+#define TOPLEVEL_CATCH(owner, threadname)                             \
   catch (Exception *e)                                                  \
   {                                                                     \
-    fatal(NULL, IDS_UNCAUGHT_EXCEPTION_WITH_ERRNO, (threadname),        \
+    fatal((owner), IDS_UNCAUGHT_EXCEPTION_WITH_ERRNO, (threadname),   \
         typeid(*e).name(), e->what(), e->errNo());                      \
   }                                                                     \
   catch (std::exception *e)                                             \
   {                                                                     \
-    fatal(NULL, IDS_UNCAUGHT_EXCEPTION, (threadname),                   \
+    fatal((owner), IDS_UNCAUGHT_EXCEPTION, (threadname),              \
         typeid(*e).name(), e->what());                                  \
   }
 
diff --git a/download.cc b/download.cc
index 697f94a..683376e 100644
--- a/download.cc
+++ b/download.cc
@@ -384,7 +384,7 @@ do_download_reflector (void *p)
     // Tell the progress page that we're done downloading
     Progress.PostMessageNow (WM_APP_DOWNLOAD_THREAD_COMPLETE, 0, next_dialog);
   }
-  TOPLEVEL_CATCH("download");
+  TOPLEVEL_CATCH((HWND) context[1], "download");
 
   ExitThread(0);
 }
diff --git a/ini.cc b/ini.cc
index f021ed2..e9152ec 100644
--- a/ini.cc
+++ b/ini.cc
@@ -422,7 +422,7 @@ do_ini_thread_reflector (void* p)
     // Tell the progress page that we're done downloading
     Progress.PostMessageNow (WM_APP_SETUP_INI_DOWNLOAD_COMPLETE, 0, succeeded);
   }
-  TOPLEVEL_CATCH ("ini");
+  TOPLEVEL_CATCH ((HWND) context[1], "ini");
 
   ExitThread (0);
 }
diff --git a/install.cc b/install.cc
index 8505199..d6af331 100644
--- a/install.cc
+++ b/install.cc
@@ -997,7 +997,7 @@ do_install_reflector (void *p)
     // Tell the progress page that we're done downloading
     Progress.PostMessageNow (WM_APP_INSTALL_THREAD_COMPLETE);
   }
-  TOPLEVEL_CATCH("install");
+  TOPLEVEL_CATCH((HWND) context[1], "install");
 
   ExitThread (0);
 }
diff --git a/main.cc b/main.cc
index b44f9b6..7c1170e 100644
--- a/main.cc
+++ b/main.cc
@@ -349,7 +349,7 @@ WinMain (HINSTANCE h,
 finish_up:
     ;
   }
-  TOPLEVEL_CATCH("main");
+  TOPLEVEL_CATCH(NULL, "main");
 
   // Never reached
   return 0;
diff --git a/postinstall.cc b/postinstall.cc
index c871201..3cd6ff0 100644
--- a/postinstall.cc
+++ b/postinstall.cc
@@ -253,7 +253,7 @@ do_postinstall_reflector (void *p)
     Progress.PostMessageNow (WM_APP_POSTINSTALL_THREAD_COMPLETE, 0,
                           s.empty() ? IDD_DESKTOP : IDD_POSTINSTALL);
   }
-  TOPLEVEL_CATCH("postinstall");
+  TOPLEVEL_CATCH((HWND) context[1], "postinstall");
 
   /* Revert primary group to admins group.  This allows to create all the
      state files written by setup as admin group owned. */
diff --git a/prereq.cc b/prereq.cc
index 8b1bbbb..0a46ad1 100644
--- a/prereq.cc
+++ b/prereq.cc
@@ -364,7 +364,7 @@ do_prereq_check_reflector (void *p)
     // Tell the progress page that we're done prereq checking
     Progress.PostMessageNow (WM_APP_PREREQ_CHECK_THREAD_COMPLETE, 0, 
next_dialog);
   }
-  TOPLEVEL_CATCH("prereq_check");
+  TOPLEVEL_CATCH((HWND) context[1], "prereq_check");
 
   ExitThread(0);
 }
diff --git a/proppage.cc b/proppage.cc
index d4d2926..6b83640 100644
--- a/proppage.cc
+++ b/proppage.cc
@@ -359,7 +359,7 @@ PropertyPage::DialogProc (UINT message, WPARAM wParam, 
LPARAM lParam)
       return OnMessageApp (message, wParam, lParam);
     }
   }
-  TOPLEVEL_CATCH("DialogProc");
+  TOPLEVEL_CATCH(GetHWND (), "DialogProc");
 
   // Wasn't handled
   return FALSE;
diff --git a/site.cc b/site.cc
index a945d28..5e20b3b 100644
--- a/site.cc
+++ b/site.cc
@@ -445,7 +445,7 @@ do_download_site_info_thread (void *p)
       Progress.PostMessageNow (WM_APP_SITE_INFO_DOWNLOAD_COMPLETE, 0, 
IDD_SITE);
     }
   }
-  TOPLEVEL_CATCH("site");
+  TOPLEVEL_CATCH((HWND) context[1], "site");
 
   ExitThread(0);
 }
-- 
2.15.1

Reply via email to