fixes bug 337824 "Mac OSX event processing problems related to threadmanager landing (Profile Manager completely horked)" patch and reviews by darin and mark respectively

widget/src/cocoa/nsAppShell.h

@@ -68,6 +68,8 @@ protected:
 
   NSPort*            mPort;
   AppShellDelegate*  mDelegate;
+
+  PRPackedBool       mRunningEventLoop;
 };
 
 #endif // nsAppShell_h__

widget/src/cocoa/nsAppShell.mm

@@ -63,6 +63,7 @@
 nsAppShell::nsAppShell()
 : mPort(nil)
 , mDelegate(nil)
+, mRunningEventLoop(PR_FALSE)
 {
   // mMainPool sits low on the autorelease pool stack to serve as a catch-all
   // for autoreleased objects on this thread.  Because it won't be popped
@@ -152,7 +153,7 @@ nsAppShell::Init()
 void
 nsAppShell::ProcessGeckoEvents()
 {
-  if (RunWasCalled()) {
+  if (mRunningEventLoop) {
     // We own the run loop.  Interrupt it.  It will be started again later
     // (unless exiting) by nsBaseAppShell.  Trust me, I'm a doctor.
     [NSApp stop:nil];
@@ -227,11 +228,17 @@ nsAppShell::ProcessNextNativeEvent(PRBool aMayWait)
       NSAutoreleasePool* localPool = [[NSAutoreleasePool alloc] init];
       [NSApp sendEvent:event];
       [localPool release];
+
+      // [NSApp run] calls updateWindows after each event.
+      [NSApp updateWindows];
     }
   }
   else {
     // Run the run loop until interrupted by a stop: message.
+    PRBool prevVal = mRunningEventLoop;
+    mRunningEventLoop = PR_TRUE;
     [NSApp run];
+    mRunningEventLoop = prevVal;
   }
 
   if ([NSApp nextEventMatchingMask:NSAnyEventMask
@@ -251,7 +258,7 @@ nsAppShell::ProcessNextNativeEvent(PRBool aMayWait)
 //
 // The selector called on the delegate object when nsAppShell::mPort is sent an
 // NSPortMessage by ScheduleNativeEventCallback.  Call into the nsAppShell
-// object for access to mRunWasCalled and NativeEventCallback.
+// object for access to mRunningEventLoop and NativeEventCallback.
 //
 - (void)handlePortMessage:(NSPortMessage*)aPortMessage
 {

widget/src/mac/nsAppShell.cpp

@@ -78,6 +78,7 @@ nsAppShell::nsAppShell()
 : mWNETransitionEventHandler(NULL)
 , mCFRunLoop(NULL)
 , mCFRunLoopSource(NULL)
+, mRunningEventLoop(PR_FALSE)
 {
 }
 
@@ -179,21 +180,6 @@ nsAppShell::ProcessNextNativeEvent(PRBool aMayWait)
 
   if (!aMayWait) {
     // Only process a single event.
-#if 0
-    EventQueueRef carbonEventQueue = ::GetCurrentEventQueue();
-
-    // This requires Mac OS X 10.3 or later... we cannot use it until the
-    // builds systems are upgraded --darin
-    if (EventRef carbonEvent =
-         ::AcquireFirstMatchingEventInQueue(carbonEventQueue,
-                                            0,
-                                            NULL,
-                                            kEventQueueOptionsNone)) {
-      ::SendEventToEventTarget(carbonEvent, ::GetEventDispatcherTarget());
-      ::RemoveEventFromQueue(carbonEventQueue, carbonEvent);
-      ::ReleaseEvent(carbonEvent);
-    }
-#endif
     EventRef carbonEvent;
     OSStatus err = ::ReceiveNextEvent(0, nsnull, kEventDurationNoWait, PR_TRUE,
                                       &carbonEvent);
@@ -204,11 +190,11 @@ nsAppShell::ProcessNextNativeEvent(PRBool aMayWait)
     }
   }
   else {
-    // XXX(darin): It seems to me that we should be using ReceiveNextEvent here
-    // as well to ensure that we only wait for and process a single event.
-
+    PRBool prevVal = mRunningEventLoop;
+    mRunningEventLoop = PR_TRUE;
     // Run the loop until interrupted by ::QuitApplicationEventLoop().
     ::RunApplicationEventLoop();
+    mRunningEventLoop = prevVal;
 
     eventProcessed = PR_TRUE;
   }
@@ -231,7 +217,7 @@ nsAppShell::ProcessGeckoEvents(void* aInfo)
 {
   nsAppShell* self = NS_STATIC_CAST(nsAppShell*, aInfo);
 
-  if (self->RunWasCalled()) {
+  if (self->mRunningEventLoop) {
     // We own the run loop.  Interrupt it.  It will be started again later
     // (unless exiting) by nsBaseAppShell.  Trust me, I'm a doctor.
     ::QuitApplicationEventLoop();

widget/src/mac/nsAppShell.h

@@ -82,6 +82,8 @@ protected:
 
   CFRunLoopRef                mCFRunLoop;
   CFRunLoopSourceRef          mCFRunLoopSource;
+
+  PRBool                      mRunningEventLoop;
 };
 
 #endif // nsAppShell_h__

widget/src/xpwidgets/nsBaseAppShell.cpp

@@ -37,13 +37,16 @@
 
 #include "nsBaseAppShell.h"
 #include "nsThreadUtils.h"
+#include "nsIObserverService.h"
+#include "nsServiceManagerUtils.h"
 
 // When processing the next thread event, the appshell may process native
 // events (if not in performance mode), which can result in suppressing the
 // next thread event for at most this many ticks:
 #define THREAD_EVENT_STARVATION_LIMIT PR_MillisecondsToInterval(20)
 
-NS_IMPL_THREADSAFE_ISUPPORTS2(nsBaseAppShell, nsIAppShell, nsIThreadObserver)
+NS_IMPL_THREADSAFE_ISUPPORTS3(nsBaseAppShell, nsIAppShell, nsIThreadObserver,
+                              nsIObserver)
 
 nsBaseAppShell::nsBaseAppShell()
   : mFavorPerf(0)
@@ -67,6 +70,11 @@ nsBaseAppShell::Init()
   NS_ENSURE_STATE(threadInt);
 
   threadInt->SetObserver(this);
+
+  nsCOMPtr<nsIObserverService> obsSvc =
+      do_GetService("@mozilla.org/observer-service;1");
+  if (obsSvc)
+    obsSvc->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, PR_FALSE);
   return NS_OK;
 }
 
@@ -76,8 +84,20 @@ nsBaseAppShell::NativeEventCallback()
   PRInt32 hasPending = PR_AtomicSet(&mNativeEventPending, 0);
   if (hasPending == 0)
     return;
-  if (mProcessingNextNativeEvent)
+
+  // If DoProcessNextNativeEvent is on the stack, then we assume that we can
+  // just unwind and let nsThread::ProcessNextEvent process the next event.
+  // However, if we are called from a nested native event loop (maybe via some
+  // plug-in or library function), then we need to eventually process the event
+  // ourselves.  To make that happen, we schedule another call to ourselves and
+  // unset the flag set by DoProcessNextNativeEvent.  This increases the
+  // workload on the native event queue slightly.
+  if (mProcessingNextNativeEvent) {
+    mProcessingNextNativeEvent = PR_FALSE;
+    if (NS_HasPendingEvents(NS_GetCurrentThread()))
+      OnDispatchedEvent(nsnull);
     return;
+  }
 
   // nsBaseAppShell::Run is not being used to pump events, so this may be
   // our only opportunity to process pending gecko events.
@@ -97,10 +117,19 @@ nsBaseAppShell::DoProcessNextNativeEvent(PRBool mayWait)
   // The next native event to be processed may trigger our NativeEventCallback,
   // in which case we do not want it to process any thread events since we'll
   // do that when this function returns.
+  //
+  // If the next native event is not our NativeEventCallback, then we may end
+  // up recursing into this function.
+  //
+  // However, if the next native event is not our NativeEventCallback, but it
+  // results in another native event loop, then our NativeEventCallback could
+  // fire and it will see mProcessNextNativeEvent as true.
+  //
+  PRBool prevVal = mProcessingNextNativeEvent;
 
   mProcessingNextNativeEvent = PR_TRUE;
   PRBool result = ProcessNextNativeEvent(mayWait); 
-  mProcessingNextNativeEvent = PR_FALSE;
+  mProcessingNextNativeEvent = prevVal;
   return result;
 }
 
@@ -190,7 +219,7 @@ nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal *thr, PRBool mayWait,
   while (!NS_HasPendingEvents(thr)) {
     // If we have been asked to exit from Run, then we should not wait for
     // events to process.  
-    if (mRunWasCalled && mExiting && mayWait)
+    if (mExiting && mayWait)
       mayWait = PR_FALSE;
 
     mLastNativeEventTime = PR_IntervalNow();
@@ -209,3 +238,12 @@ nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal *thr, PRBool mayWait,
 
   return NS_OK;
 }
+
+NS_IMETHODIMP
+nsBaseAppShell::Observe(nsISupports *subject, const char *topic,
+                        const PRUnichar *data)
+{
+  NS_ASSERTION(!strcmp(topic, NS_XPCOM_SHUTDOWN_OBSERVER_ID), "oops");
+  Exit();
+  return NS_OK;
+}

widget/src/xpwidgets/nsBaseAppShell.h

@@ -40,6 +40,7 @@
 
 #include "nsIAppShell.h"
 #include "nsIThreadInternal.h"
+#include "nsIObserver.h"
 #include "nsIRunnable.h"
 #include "nsCOMPtr.h"
 #include "prinrval.h"
@@ -48,12 +49,14 @@
  * A singleton that manages the UI thread's event queue.  Subclass this class
  * to enable platform-specific event queue support.
  */
-class nsBaseAppShell : public nsIAppShell, public nsIThreadObserver
+class nsBaseAppShell : public nsIAppShell, public nsIThreadObserver,
+                       public nsIObserver
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIAPPSHELL
   NS_DECL_NSITHREADOBSERVER
+  NS_DECL_NSIOBSERVER
 
   nsBaseAppShell();
 
@@ -91,14 +94,6 @@ protected:
    */
   virtual PRBool ProcessNextNativeEvent(PRBool mayWait) = 0;
 
-  /**
-   * Indicates whether or not nsIAppShell::Run was called.  In an embedding
-   * application, the embedder usually spins up a native event loop on their
-   * own and does not call nsIAppShell::Run.  In such cases, we have to go to
-   * extra lengths to properly hook ourselves into that native event loop.
-   */
-  PRBool RunWasCalled() { return mRunWasCalled; }
-
 private:
   PRBool DoProcessNextNativeEvent(PRBool mayWait);