Bugzilla Bug 91961 [xlib] Thread Local Storage issues in nsToolkit.cpp result in crash

patch by timecop@network.email.ne.jp r=zuperdee,Tomi.Leppikangas@oulu.fi sr=blizzard

gfx/src/xlib/nsRenderingContextXlib.cpp

@@ -211,6 +211,8 @@ nsRenderingContextXlib::Init(nsIDeviceContext* aContext, nsIWidget *aWindow)
 
     mOffscreenSurface = mRenderingSurface;
     NS_ADDREF(mRenderingSurface);
+    /* aWindow->GetNativeData() ref'd the gc */
+    gc->Release();
   }
 
   return CommonInit();

widget/src/xlib/nsToolkit.cpp

@@ -1,4 +1,5 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ * vim:ts=2:et:sw=2
  *
  * The contents of this file are subject to the Netscape Public
  * License Version 1.1 (the "License"); you may not use this file
@@ -17,52 +18,58 @@
  * Copyright (C) 1998 Netscape Communications Corporation. All
  * Rights Reserved.
  *
- * Contributor(s): 
+ * Contributor(s):
  */
 
+#include "nscore.h" // needed for 'nsnull'
+#include "xlibrgb.h"
 #include "nsToolkit.h"
-#include "nsGUIEvent.h"
-#include "plevent.h"
 #include "nsGCCache.h"
 
 // Static Thread Local Storage index of the toolkit object associated with
 // a given thread...
-
 static PRUintn gToolkitTLSIndex = 0;
 
 nsToolkit::nsToolkit()
 {
   NS_INIT_REFCNT();
   mGC = NULL;
+  mDisplay = xlib_rgb_get_display();
 }
 
 nsToolkit::~nsToolkit()
 {
-  if(mGC)
+  if (mGC)
     mGC->Release();
+
+  // Remove the TLS reference to the toolkit...
+  PR_SetThreadPrivate(gToolkitTLSIndex, nsnull);
 }
 
-NS_DEFINE_IID(kIToolkitIID, NS_ITOOLKIT_IID);
-NS_IMPL_ISUPPORTS(nsToolkit,kIToolkitIID);
+NS_IMPL_ISUPPORTS1(nsToolkit, nsIToolkit)
 
-void nsToolkit::CreateSharedGC(Display *display, Drawable d)
+void nsToolkit::CreateSharedGC()
 {
   if (mGC)
     return;
-  mGC = new xGC(display, d, 0, NULL); 
+
+  mGC = new xGC(mDisplay, DefaultRootWindow(mDisplay), 0, NULL);
   mGC->AddRef(); // this is for us
 }
 
-xGC *nsToolkit::GetSharedGC(Display *display, Drawable d)
+xGC *nsToolkit::GetSharedGC()
 {
-  if(!mGC)
-    CreateSharedGC(display, d); 
+  if (!mGC)
+    CreateSharedGC();
 
+  mGC->AddRef();
   return mGC;
 }
 
 NS_METHOD nsToolkit::Init(PRThread *aThread)
 {
+  CreateSharedGC();
+
   return NS_OK;
 }
 
@@ -102,11 +109,11 @@ NS_METHOD NS_GetCurrentToolkit(nsIToolkit* *aResult)
         // nsToolkit destructor...
         //
         PR_SetThreadPrivate(gToolkitTLSIndex, (void*)toolkit);
-    }
+      }
     } else {
       NS_ADDREF(toolkit);
-  }
+    }
     *aResult = toolkit;
   }
-  return NS_OK;
+  return rv;
 }

widget/src/xlib/nsToolkit.h

@@ -28,9 +28,6 @@
 #include <X11/Xlib.h>
 #include <X11/Xutil.h>
 
-struct PLEventQueue;
-struct MethodInfo;
-
 /**
  * Wrapper around the thread running the message pump.
  * The toolkit abstraction is necessary because the message pump must
@@ -40,17 +37,17 @@ struct MethodInfo;
 class nsToolkit : public nsIToolkit
 {
 private:
-  void CreateSharedGC(Display *display, Drawable d);
+  Display *mDisplay;
+  void CreateSharedGC();
   xGC *mGC;
 
 public:
   nsToolkit();
-  virtual		~nsToolkit();
+  virtual ~nsToolkit();
   
   NS_DECL_ISUPPORTS
-  NS_IMETHOD            Init(PRThread *aThread);
-  xGC *GetSharedGC(Display *display, Drawable d);
-
+  NS_IMETHOD Init(PRThread *aThread);
+  xGC *GetSharedGC();
 };
 
 #endif

widget/src/xlib/nsWidget.cpp

@@ -583,9 +583,8 @@ void * nsWidget::GetNativeData(PRUint32 aDataType)
     return (void *)mDisplay;
     break;
   case NS_NATIVE_GRAPHIC:
-    // XXX implement this...
     NS_ASSERTION(nsnull != mToolkit, "NULL toolkit, unable to get a GC");
-    return (void *)NS_STATIC_CAST(nsToolkit*,mToolkit)->GetSharedGC(mDisplay, mBaseWindow);
+    return (void *)NS_STATIC_CAST(nsToolkit*,mToolkit)->GetSharedGC();
     break;
   default:
     fprintf(stderr, "nsWidget::GetNativeData(%d) called with crap value.\n",
@@ -707,18 +706,24 @@ NS_IMETHODIMP nsWidget::ScreenToWidget(const nsRect& aOldRect,
 
 NS_IMETHODIMP nsWidget::SetCursor(nsCursor aCursor)
 {
+
   if (!mBaseWindow)
     return NS_ERROR_FAILURE;
 
+  /* don't bother setting if it it isnt mapped, duh */
+  if (!mMapped)
+    return NS_OK;
+  
   // Only change cursor if it's changing
   if (aCursor != mCursor) {
-    Cursor newCursor = 0;
+    Cursor newCursor = None;
 
     newCursor = XlibCreateCursor(aCursor);
 
     if (None != newCursor) {
       mCursor = aCursor;
       XDefineCursor(mDisplay, mBaseWindow, newCursor);
+      XFlush(mDisplay);
     }
   }
   return NS_OK;