b=499498 BadWindow errors on startup r=roc
authorKarl Tomlinson <karlt+@karlt.net>
Thu, 03 Sep 2009 08:47:44 +1200
changeset 32200 7bdfa4dfef47a1bbdf99c273b3b0e20d3ca52efd
parent 32199 19b213c1cc316baf3f4714585d50e3eb7a0ee2e8
child 32201 c93943dd0139a1f87a9054be9c4150a9d9f0cbde
push id8909
push userktomlinson@mozilla.com
push dateThu, 03 Sep 2009 01:42:13 +0000
treeherdermozilla-central@c93943dd0139 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs499498
milestone1.9.3a1pre
b=499498 BadWindow errors on startup r=roc
widget/src/xremoteclient/XRemoteClient.cpp
widget/src/xremoteclient/XRemoteClient.h
--- a/widget/src/xremoteclient/XRemoteClient.cpp
+++ b/widget/src/xremoteclient/XRemoteClient.cpp
@@ -1,10 +1,10 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim:expandtab:shiftwidth=4:tabstop=4:
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:expandtab:shiftwidth=2:tabstop=8:
  */
 /* vim:set ts=8 sw=2 et cindent: */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
@@ -83,16 +83,19 @@
 #define MAX_PATH 1024
 #endif
 #endif
 
 #define ARRAY_LENGTH(array_) (sizeof(array_)/sizeof(array_[0]))
 
 static PRLogModuleInfo *sRemoteLm = NULL;
 
+static int (*sOldHandler)(Display *, XErrorEvent *);
+static PRBool sGotBadWindow;
+
 XRemoteClient::XRemoteClient()
 {
   mDisplay = 0;
   mInitialized = PR_FALSE;
   mMozVersionAtom = 0;
   mMozLockAtom = 0;
   mMozCommandAtom = 0;
   mMozResponseAtom = 0;
@@ -178,93 +181,107 @@ XRemoteClient::Shutdown (void)
 nsresult
 XRemoteClient::SendCommand (const char *aProgram, const char *aUsername,
                             const char *aProfile, const char *aCommand,
                             const char* aDesktopStartupID,
                             char **aResponse, PRBool *aWindowFound)
 {
   PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("XRemoteClient::SendCommand"));
 
-  *aWindowFound = PR_FALSE;
-
-  Window w = FindBestWindow(aProgram, aUsername, aProfile, PR_FALSE);
-
-  nsresult rv = NS_OK;
-
-  if (w) {
-    // ok, let the caller know that we at least found a window.
-    *aWindowFound = PR_TRUE;
-
-    // make sure we get the right events on that window
-    XSelectInput(mDisplay, w,
-                 (PropertyChangeMask|StructureNotifyMask));
-        
-    PRBool destroyed = PR_FALSE;
-
-    // get the lock on the window
-    rv = GetLock(w, &destroyed);
-
-    if (NS_SUCCEEDED(rv)) {
-      // send our command
-      rv = DoSendCommand(w, aCommand, aDesktopStartupID, aResponse, &destroyed);
-
-      // if the window was destroyed, don't bother trying to free the
-      // lock.
-      if (!destroyed)
-          FreeLock(w); // doesn't really matter what this returns
-
-    }
-  }
-
-  PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("SendCommand returning 0x%x\n", rv));
-
-  return rv;
+  return SendCommandInternal(aProgram, aUsername, aProfile,
+                             aCommand, 0, nsnull,
+                             aDesktopStartupID,
+                             aResponse, aWindowFound);
 }
 
 nsresult
 XRemoteClient::SendCommandLine (const char *aProgram, const char *aUsername,
                                 const char *aProfile,
                                 PRInt32 argc, char **argv,
                                 const char* aDesktopStartupID,
                                 char **aResponse, PRBool *aWindowFound)
 {
   PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("XRemoteClient::SendCommandLine"));
 
-  *aWindowFound = PR_FALSE;
+  return SendCommandInternal(aProgram, aUsername, aProfile,
+                             nsnull, argc, argv,
+                             aDesktopStartupID,
+                             aResponse, aWindowFound);
+}
 
-  Window w = FindBestWindow(aProgram, aUsername, aProfile, PR_TRUE);
+static int
+HandleBadWindow(Display *display, XErrorEvent *event)
+{
+  if (event->error_code == BadWindow) {
+    sGotBadWindow = PR_TRUE;
+    return 0; // ignored
+  }
+  else {
+    return (*sOldHandler)(display, event);
+  }
+}
+
+nsresult
+XRemoteClient::SendCommandInternal(const char *aProgram, const char *aUsername,
+                                   const char *aProfile, const char *aCommand,
+                                   PRInt32 argc, char **argv,
+                                   const char* aDesktopStartupID,
+                                   char **aResponse, PRBool *aWindowFound)
+{
+  *aWindowFound = PR_FALSE;
+  PRBool isCommandLine = !aCommand;
+
+  // FindBestWindow() iterates down the window hierarchy, so catch X errors
+  // when windows get destroyed before being accessed.
+  sOldHandler = XSetErrorHandler(HandleBadWindow);
+
+  Window w = FindBestWindow(aProgram, aUsername, aProfile, isCommandLine);
 
   nsresult rv = NS_OK;
 
   if (w) {
     // ok, let the caller know that we at least found a window.
     *aWindowFound = PR_TRUE;
 
+    // Ignore BadWindow errors up to this point.  The last request from
+    // FindBestWindow() was a synchronous XGetWindowProperty(), so no need to
+    // Sync.  Leave the error handler installed to detect if w gets destroyed.
+    sGotBadWindow = PR_FALSE;
+
     // make sure we get the right events on that window
     XSelectInput(mDisplay, w,
                  (PropertyChangeMask|StructureNotifyMask));
-        
+
     PRBool destroyed = PR_FALSE;
 
     // get the lock on the window
     rv = GetLock(w, &destroyed);
 
     if (NS_SUCCEEDED(rv)) {
       // send our command
-      rv = DoSendCommandLine(w, argc, argv, aDesktopStartupID, aResponse, &destroyed);
+      if (isCommandLine) {
+        rv = DoSendCommandLine(w, argc, argv, aDesktopStartupID, aResponse,
+                               &destroyed);
+      }
+      else {
+        rv = DoSendCommand(w, aCommand, aDesktopStartupID, aResponse,
+                           &destroyed);
+      }
 
       // if the window was destroyed, don't bother trying to free the
       // lock.
       if (!destroyed)
           FreeLock(w); // doesn't really matter what this returns
 
     }
   }
 
-  PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("SendCommandLine returning 0x%x\n", rv));
+  XSetErrorHandler(sOldHandler);
+
+  PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("SendCommandInternal returning 0x%x\n", rv));
 
   return rv;
 }
 
 Window
 XRemoteClient::CheckWindow(Window aWindow)
 {
   Atom type = None;
@@ -333,16 +350,18 @@ XRemoteClient::CheckChildren(Window aWin
 
 nsresult
 XRemoteClient::GetLock(Window aWindow, PRBool *aDestroyed)
 {
   PRBool locked = PR_FALSE;
   PRBool waited = PR_FALSE;
   *aDestroyed = PR_FALSE;
 
+  nsresult rv = NS_OK;
+
   if (!mLockData) {
     
     char pidstr[32];
     char sysinfobuf[SYS_INFO_BUFFER_LENGTH];
     PR_snprintf(pidstr, sizeof(pidstr), "pid%d@", getpid());
     PRStatus status;
     status = PR_GetSystemInfo(PR_SI_HOSTNAME, sysinfobuf,
 			      SYS_INFO_BUFFER_LENGTH);
@@ -372,29 +391,38 @@ XRemoteClient::GetLock(Window aWindow, P
 
     result = XGetWindowProperty (mDisplay, aWindow, mMozLockAtom,
 				 0, (65536 / sizeof (long)),
 				 False, /* don't delete */
 				 XA_STRING,
 				 &actual_type, &actual_format,
 				 &nitems, &bytes_after,
 				 &data);
-    if (result != Success || actual_type == None) {
+
+    // aWindow may have been destroyed before XSelectInput was processed, in
+    // which case there may not be any DestroyNotify event in the queue to
+    // tell us.  XGetWindowProperty() was synchronous so error responses have
+    // now been processed, setting sGotBadWindow.
+    if (sGotBadWindow) {
+      *aDestroyed = PR_TRUE;
+      rv = NS_ERROR_FAILURE;
+    }
+    else if (result != Success || actual_type == None) {
       /* It's not now locked - lock it. */
       XChangeProperty (mDisplay, aWindow, mMozLockAtom, XA_STRING, 8,
 		       PropModeReplace,
 		       (unsigned char *)mLockData,
 		       strlen(mLockData));
       locked = True;
     }
 
     XUngrabServer(mDisplay);
-    XSync(mDisplay, False);
+    XFlush(mDisplay); // ungrab now!
 
-    if (!locked) {
+    if (!locked && !NS_FAILED(rv)) {
       /* We tried to grab the lock this time, and failed because someone
 	 else is holding it already.  So, wait for a PropertyDelete event
 	 to come in, and try again. */
       PR_LOG(sRemoteLm, PR_LOG_DEBUG, 
 	     ("window 0x%x is locked by %s; waiting...\n",
 	      (unsigned int) aWindow, data));
       waited = True;
       while (1) {
@@ -415,50 +443,53 @@ XRemoteClient::GetLock(Window aWindow, P
 	// add the x event queue to the select set
 	FD_SET(ConnectionNumber(mDisplay), &select_set);
 	select_retval = select(ConnectionNumber(mDisplay) + 1,
 			       &select_set, NULL, NULL, &delay);
 #endif
 	// did we time out?
 	if (select_retval == 0) {
 	  PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("timed out waiting for window\n"));
-	  return NS_ERROR_FAILURE;
+          rv = NS_ERROR_FAILURE;
+          break;
 	}
 	PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("xevent...\n"));
 	XNextEvent (mDisplay, &event);
 	if (event.xany.type == DestroyNotify &&
 	    event.xdestroywindow.window == aWindow) {
-	  PR_LOG(sRemoteLm, PR_LOG_DEBUG,
-		 ("window 0x%x unexpectedly destroyed.\n",
-		  (unsigned int) aWindow));
 	  *aDestroyed = PR_TRUE;
-	  return NS_ERROR_FAILURE;
+          rv = NS_ERROR_FAILURE;
+          break;
 	}
 	else if (event.xany.type == PropertyNotify &&
 		 event.xproperty.state == PropertyDelete &&
 		 event.xproperty.window == aWindow &&
 		 event.xproperty.atom == mMozLockAtom) {
 	  /* Ok!  Someone deleted their lock, so now we can try
 	     again. */
 	  PR_LOG(sRemoteLm, PR_LOG_DEBUG,
 		 ("(0x%x unlocked, trying again...)\n",
 		  (unsigned int) aWindow));
 		  break;
 	}
       }
     }
     if (data)
       XFree(data);
-  } while (!locked);
+  } while (!locked && !NS_FAILED(rv));
 
-  if (waited) {
+  if (waited && locked) {
     PR_LOG(sRemoteLm, PR_LOG_DEBUG, ("obtained lock.\n"));
+  } else if (*aDestroyed) {
+    PR_LOG(sRemoteLm, PR_LOG_DEBUG,
+           ("window 0x%x unexpectedly destroyed.\n",
+            (unsigned int) aWindow));
   }
 
-  return NS_OK;
+  return rv;
 }
 
 Window
 XRemoteClient::FindBestWindow(const char *aProgram, const char *aUsername,
                               const char *aProfile,
                               PRBool aSupportsCommandLine)
 {
   Window root = RootWindowOfScreen(DefaultScreenOfDisplay(mDisplay));
--- a/widget/src/xremoteclient/XRemoteClient.h
+++ b/widget/src/xremoteclient/XRemoteClient.h
@@ -1,8 +1,9 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
@@ -62,16 +63,21 @@ private:
   Window         CheckWindow      (Window aWindow);
   Window         CheckChildren    (Window aWindow);
   nsresult       GetLock          (Window aWindow, PRBool *aDestroyed);
   nsresult       FreeLock         (Window aWindow);
   Window         FindBestWindow   (const char *aProgram,
                                    const char *aUsername,
                                    const char *aProfile,
                                    PRBool aSupportsCommandLine);
+  nsresult     SendCommandInternal(const char *aProgram, const char *aUsername,
+                                   const char *aProfile, const char *aCommand,
+                                   PRInt32 argc, char **argv,
+                                   const char* aDesktopStartupID,
+                                   char **aResponse, PRBool *aWindowFound);
   nsresult       DoSendCommand    (Window aWindow,
                                    const char *aCommand,
                                    const char* aDesktopStartupID,
                                    char **aResponse,
                                    PRBool *aDestroyed);
   nsresult       DoSendCommandLine(Window aWindow,
                                    PRInt32 argc, char **argv,
                                    const char* aDesktopStartupID,