Fix leaks in the xlib port (and some problems exposed by fixing those leaks). b=64534, 64767 r=blizzard@mozilla.org (Not part of the build.)

gfx/src/xlib/nsFontMetricsXlib.cpp

@@ -61,8 +61,6 @@ static NS_DEFINE_IID(kIFontMetricsIID, NS_IFONT_METRICS_IID);
 
 static PRLogModuleInfo * FontMetricsXlibLM = PR_NewLogModule("FontMetricsXlib");
 
-static Display *gDisplay = NULL;
-
 static int gFontMetricsXlibCount = 0;
 static int gInitialized = 0;
 
@@ -500,6 +498,12 @@ static void
 FreeStretch(nsFontStretch* aStretch)
 {
   PR_smprintf_free(aStretch->mScalable);
+
+  for (PRInt32 count = aStretch->mScaledFonts.Count()-1; count >= 0; count--) {
+    nsFontXlib *font = (nsFontXlib*)aStretch->mScaledFonts.ElementAt(count);
+    if (font) delete font;
+  }
+
   for (int i=0; i < aStretch->mSizesCount; i++)
     delete aStretch->mSizes[i];
 
@@ -1616,6 +1620,7 @@ MOZ_DECL_CTOR_COUNTER(nsFontXlib);
 nsFontXlib::nsFontXlib()
 {
   MOZ_COUNT_CTOR(nsFontXlib);
+  mFont = nsnull;
 }
 
 nsFontXlib::~nsFontXlib()

gfx/src/xlib/nsRegionXlib.cpp

@@ -38,18 +38,16 @@ nsRegionXlib::~nsRegionXlib()
 {
   if (mRegion)
     ::XDestroyRegion(mRegion);
-
-  mRegion = NULL;
 }
 
-NS_IMPL_QUERY_INTERFACE(nsRegionXlib, kRegionIID)
-NS_IMPL_ADDREF(nsRegionXlib)
-NS_IMPL_RELEASE(nsRegionXlib)
+NS_IMPL_ISUPPORTS1(nsRegionXlib, nsIRegion)
 
 nsresult
 nsRegionXlib::Init()
 {
-  NS_ADDREF_THIS();
+  //NS_ASSERTION(!mRegion, "already initialized");
+  if (mRegion)
+    ::XDestroyRegion(mRegion);
 
   mRegion = ::XCreateRegion();
   mRegionType = eRegionComplexity_empty;

gfx/src/xlib/nsRenderingContextXlib.cpp

@@ -43,16 +43,18 @@ public:
   ~GraphicsState();
 
   nsTransform2D  *mMatrix;
-  nsRegionXlib   *mClipRegion;
+  nsCOMPtr<nsIRegion> mClipRegion;
   nscolor         mColor;
   nsLineStyle     mLineStyle;
   nsIFontMetrics *mFontMetrics;
 };
 
+MOZ_DECL_CTOR_COUNTER(GraphicsState)
+
 GraphicsState::GraphicsState()
 {
+  MOZ_COUNT_CTOR(GraphicsState);
   mMatrix = nsnull;
-  mClipRegion = nsnull;
   mColor = NS_RGB(0, 0, 0);
   mLineStyle = nsLineStyle_kSolid;
   mFontMetrics = nsnull;
@@ -60,7 +62,7 @@ GraphicsState::GraphicsState()
 
 GraphicsState::~GraphicsState()
 {
-  NS_IF_RELEASE(mClipRegion);
+  MOZ_COUNT_DTOR(GraphicsState);
   NS_IF_RELEASE(mFontMetrics);
 }
 
@@ -104,7 +106,6 @@ nsRenderingContextXlib::~nsRenderingContextXlib()
   }
   if (mTMatrix)
     delete mTMatrix;
-  NS_IF_RELEASE(mClipRegion);
   NS_IF_RELEASE(mOffscreenSurface);
   NS_IF_RELEASE(mFontMetrics);
   NS_IF_RELEASE(mContext);
@@ -223,7 +224,8 @@ nsresult nsRenderingContextXlib::CommonInit(void)
                &border, 
                &depth);
 
-  mClipRegion = new nsRegionXlib();
+  mClipRegion = do_QueryInterface(new nsRegionXlib());
+  if (!mClipRegion) return NS_ERROR_OUT_OF_MEMORY;
   mClipRegion->Init();
   mClipRegion->SetTo(0, 0, width, height);
 
@@ -326,11 +328,11 @@ nsRenderingContextXlib::PushState(void)
     mTMatrix = new nsTransform2D(mTMatrix);
   
   if (mClipRegion) {
-    NS_IF_ADDREF(mClipRegion);
     state->mClipRegion = mClipRegion;
-    mClipRegion = new nsRegionXlib();
+    mClipRegion = do_QueryInterface(new nsRegionXlib());
+    if (!mClipRegion) return NS_ERROR_OUT_OF_MEMORY;
     mClipRegion->Init();
-    mClipRegion->SetTo((const nsIRegion &)*(state->mClipRegion));
+    mClipRegion->SetTo(*state->mClipRegion);
   }
   
   NS_IF_ADDREF(mFontMetrics);
@@ -357,8 +359,6 @@ nsRenderingContextXlib::PopState(PRBool &aClipState)
       delete mTMatrix;
     mTMatrix = state->mMatrix;
     
-    NS_IF_RELEASE(mClipRegion);
-    
     mClipRegion = state->mClipRegion;
     if (mFontMetrics != state->mFontMetrics)
       SetFont(state->mFontMetrics);
@@ -471,7 +471,7 @@ NS_IMETHODIMP
 nsRenderingContextXlib::CopyClipRegion(nsIRegion &aRegion)
 {
   PR_LOG(RenderingContextXlibLM, PR_LOG_DEBUG, ("nsRenderingContextXlib::CopyClipRegion()\n"));
-  aRegion.SetTo(*NS_STATIC_CAST(nsIRegion*, mClipRegion));
+  aRegion.SetTo(*mClipRegion);
   return NS_OK;
 }
 
@@ -484,8 +484,7 @@ nsRenderingContextXlib::GetClipRegion(nsIRegion **aRegion)
   NS_ASSERTION(!(nsnull == aRegion), "no region ptr");
   
   if (*aRegion) {
-    nsIRegion *nRegion = (nsIRegion *)mClipRegion;
-    (*aRegion)->SetTo(*nRegion);
+    (*aRegion)->SetTo(*mClipRegion);
   }
   return rv;
 }

gfx/src/xlib/nsRenderingContextXlib.h

@@ -201,7 +201,7 @@ private:
   nsDrawingSurfaceXlib    *mRenderingSurface;
   nsIDeviceContext        *mContext;
   nsIFontMetrics          *mFontMetrics;
-  nsRegionXlib            *mClipRegion;
+  nsCOMPtr<nsIRegion>      mClipRegion;
   nsTransform2D           *mTMatrix;
   float                    mP2T;
   nscolor                  mCurrentColor;

widget/src/xlib/nsClipboard.cpp

@@ -60,7 +60,7 @@
 
 
 // The class statics:
-nsCOMPtr<nsITransferable>   nsClipboard::mTransferable = nsnull;
+nsITransferable            *nsClipboard::mTransferable = nsnull;
 Window                      nsClipboard::sWindow;
 Display                    *nsClipboard::sDisplay;
 
@@ -79,12 +79,22 @@ nsClipboard::nsClipboard() {
 }
 
 nsClipboard::~nsClipboard() {
+  NS_IF_RELEASE(sWidget);
+}
+
+/*static*/ void nsClipboard::Shutdown() {
+  NS_IF_RELEASE(mTransferable);
 }
 
 // Initialize the clipboard and create a nsWidget for communications
 
 void nsClipboard::Init() {
+  NS_ASSERTION(!sWidget, "already initialized");
+
   sWidget = new nsWidget();
+  if (!sWidget) return;
+  NS_ADDREF(sWidget);
+
   const nsRect rect(0,0,100,100);
 
   sWidget->Create((nsIWidget *)nsnull, rect, Callback,
@@ -196,8 +206,11 @@ NS_IMETHODIMP nsClipboard::SetNativeClipboardData(PRInt32 aWhichClipboard)
   // FIXME Need to make sure mTransferable has reference to selectionclipboard.
   // This solves the problem with copying to an external app.
   // but cannot be sure if its fully correct until menu copy/paste is working.
- if (aWhichClipboard == kSelectionClipboard)
+  if (aWhichClipboard == kSelectionClipboard) {
+    NS_IF_RELEASE(mTransferable);
     mTransferable = transferable; 
+    NS_IF_ADDREF(mTransferable);
+  }
   
   // make sure we have a good transferable
   if (nsnull == transferable) {
@@ -286,7 +299,9 @@ NS_IMETHODIMP nsClipboard::GetData(nsITransferable *aTransferable,
 
 
   // Get which transferable we should use.
+  NS_IF_RELEASE(mTransferable);
   mTransferable = GetTransferable(aWhichClipboard);
+  NS_IF_ADDREF(mTransferable);
   
   // If we currently own the selection, we will handle the paste 
   // internally, otherwise get the data from the X server

widget/src/xlib/nsClipboard.h

@@ -49,6 +49,8 @@ public:
   nsClipboard();
   virtual ~nsClipboard();
 
+  static void Shutdown();
+
   //nsISupports
   NS_DECL_ISUPPORTS
 
@@ -64,7 +66,7 @@ protected:
   NS_IMETHOD SetNativeClipboardData(PRInt32 aWhichClipboard);
 
 private:
-  static nsCOMPtr<nsITransferable>   mTransferable;
+  static nsITransferable*     mTransferable; // strong reference
   nsCOMPtr<nsIClipboardOwner> mSelectionOwner;
   nsCOMPtr<nsIClipboardOwner> mGlobalOwner;
   nsCOMPtr<nsITransferable>   mSelectionTransferable;

widget/src/xlib/nsDragService.cpp

@@ -70,7 +70,6 @@ NS_IMETHODIMP nsDragService::InvokeDragSession (nsIDOMNode *aDOMNode,
     return NS_ERROR_INVALID_ARG;
 
   mDataItems = anArrayTransferables;
-  NS_ADDREF(mDataItems);
 
   rv = mDataItems->Count(&numItemsToDrag);
   if (!numItemsToDrag)

widget/src/xlib/nsDragService.h

@@ -117,7 +117,7 @@ private:
   PRBool mCanDrop;
 
   nsWidget *mDropWidget;
-  nsISupportsArray *mDataItems;
+  nsCOMPtr<nsISupportsArray> mDataItems;
 };
 
 #endif // nsDragService_h__

widget/src/xlib/nsWidget.cpp

@@ -23,6 +23,7 @@
  *   Quy Tonthat <quy@igelaus.com.au>
  *   B.J. Rossiter <bj@igelaus.com.au>
  *   Tony Tsui <tony@igelaus.com.au>
+ *   L. David Baron <dbaron@fas.harvard.edu>
  */
 
 #include "nsWidget.h"
@@ -47,7 +48,7 @@ PRLogModuleInfo *XlibWidgetsLM = PR_NewLogModule("XlibWidgets");
 PRLogModuleInfo *XlibScrollingLM = PR_NewLogModule("XlibScrolling");
 
 // set up our static members here.
-nsHashtable *nsWidget::gsWindowList = nsnull;
+nsHashtable *nsWidget::gsWindowList = nsnull; // WEAK references to nsWidget*
 
 // this is a class for generating keys for
 // the list of windows managed by mozilla.
@@ -151,9 +152,41 @@ nsWidget::~nsWidget()
 
 }
 
+// Borrowed heavily from GTK. This should go through heirarchy of XWindow
+// windows, and destroy the appropriate children. 
+// KenF
 void
-nsWidget::DestroyNative(void)
+nsWidget::DestroyNativeChildren()
 {
+  Window       root_return;
+  Window       parent_return;
+  Window      *children_return = nsnull;
+  unsigned int nchildren_return = 0;
+  unsigned int i = 0;
+  
+  XQueryTree(mDisplay, mBaseWindow, &root_return, &parent_return,
+             &children_return, &nchildren_return);
+  // walk the list of children
+  for (i=0; i < nchildren_return; i++) {
+    nsWidget *thisWidget = GetWidgetForWindow(children_return[i]);
+    if (thisWidget) {
+      thisWidget->Destroy();
+    }
+  }      
+
+  // free up this struct
+  if (children_return)
+    XFree(children_return);
+}
+
+void
+nsWidget::DestroyNative()
+{
+  // We have to destroy the children ourselves before we call XDestroyWindow
+  // because otherwise XDestroyWindow will destroy the windows and leave us
+  // with dangling references.
+  DestroyNativeChildren();
+
   // This is handled in nsDrawingSurfaceXlib for now
 #if 0
   if (mGC)
@@ -215,12 +248,13 @@ nsWidget::StandardWidgetCreate(nsIWidget *aParent,
                                nsWidgetInitData *aInitData,
                                nsNativeWidget aNativeParent)
 {
-  unsigned long ValueMask;  
-  XSetWindowAttributes SetWinAttr;  
   unsigned long attr_mask;
   XSetWindowAttributes attr;
   Window parent=nsnull;
 
+  NS_ASSERTION(!mBaseWindow, "already initialized");
+  if (mBaseWindow) return NS_ERROR_ALREADY_INITIALIZED;
+
   mDisplay = xlib_rgb_get_display();
   mScreen = xlib_rgb_get_screen();
   mVisual = xlib_rgb_get_visual();
@@ -326,7 +360,7 @@ NS_IMETHODIMP nsWidget::Destroy()
   if (mIsDestroying)
     return NS_OK;
 
-  mIsDestroying = TRUE;
+  mIsDestroying = PR_TRUE;
 
   nsBaseWidget::Destroy();
   NS_IF_RELEASE(mParentWidget);	//????
@@ -334,10 +368,10 @@ NS_IMETHODIMP nsWidget::Destroy()
 
   if (mBaseWindow) {
     DestroyNative();
-    //mBaseWindow = NULL;
+    //mBaseWindow = nsnull;
     if (PR_FALSE == mOnDestroyCalled)
       OnDestroy();
-    mBaseWindow = NULL;
+    mBaseWindow = nsnull;
     mEventCallback = nsnull;
   }
 
@@ -820,6 +854,9 @@ nsWidget::GetEventMask()
 void nsWidget::CreateNativeWindow(Window aParent, nsRect aRect,
                                   XSetWindowAttributes aAttr, unsigned long aMask)
 {
+  NS_ASSERTION(!mBaseWindow, "already initialized");
+  if (mBaseWindow) return;
+
   mBaseWindow = XCreateWindow(mDisplay,
                               aParent,
                               aRect.x, aRect.y,
@@ -842,9 +879,8 @@ nsWidget::GetWidgetForWindow(Window aWindow)
   if (gsWindowList == nsnull) {
     return nsnull;
   }
-  nsWindowKey *window_key = new nsWindowKey(aWindow);
-  nsWidget *retval = (nsWidget *)gsWindowList->Get(window_key);
-  delete window_key;
+  nsWindowKey window_key(aWindow);
+  nsWidget *retval = (nsWidget *)gsWindowList->Get(&window_key);
   return retval;
 }
 
@@ -855,10 +891,8 @@ nsWidget::AddWindowCallback(Window aWindow, nsWidget *aWidget)
   if (gsWindowList == nsnull) {
     gsWindowList = new nsHashtable();
   }
-  nsWindowKey *window_key = new nsWindowKey(aWindow);
-  gsWindowList->Put(window_key, aWidget);
-  // add a new ref to this widget
-  NS_ADDREF(aWidget);
+  nsWindowKey window_key(aWindow);
+  gsWindowList->Put(&window_key, aWidget);
 
   // make sure that if someone is listening that we inform
   // them of the new window
@@ -866,24 +900,24 @@ nsWidget::AddWindowCallback(Window aWindow, nsWidget *aWidget)
   {
     (*gsWindowCreateCallback)(aWindow);
   }
-
-  delete window_key;
 }
 
 void
 nsWidget::DeleteWindowCallback(Window aWindow)
 {
-  nsWindowKey *window_key = new nsWindowKey(aWindow);
-  nsWidget *widget = (nsWidget *)gsWindowList->Get(window_key);
-  NS_RELEASE(widget);
-  gsWindowList->Remove(window_key);
+  nsWindowKey window_key(aWindow);
+  gsWindowList->Remove(&window_key);
+
+  if (gsWindowList->Count() == 0) {
+    delete gsWindowList;
+    gsWindowList = nsnull;
+  }
 
   if (gsWindowDestroyCallback) 
   {
     (*gsWindowDestroyCallback)(aWindow);
   }
 
-  delete window_key;
 }
 
 #undef TRACE_PAINT
@@ -899,7 +933,10 @@ nsWidget::OnPaint(nsPaintEvent &event)
   nsresult result = PR_FALSE;
   if (mEventCallback) {
     event.renderingContext = GetRenderingContext();
-    result = DispatchWindowEvent(event);
+    if (event.renderingContext) {
+      result = DispatchWindowEvent(event);
+      NS_RELEASE(event.renderingContext);
+    }
   }
   
 #ifdef TRACE_PAINT
@@ -1209,7 +1246,7 @@ nsWidget::DebugPrintEvent(nsGUIEvent &   aEvent,
   printf("%4d %-26s(this=%-8p , window=%-8p",
          sPrintCount++,
          (const char *) eventString,
-         this,
+         (void *) this,
          (void *) aWindow);
          
   printf(" , x=%-3d, y=%d)",aEvent.point.x,aEvent.point.y);

widget/src/xlib/nsWidget.h

@@ -169,7 +169,8 @@ protected:
   virtual void CreateNativeWindow(Window aParent, nsRect aRect,
                                   XSetWindowAttributes aAttr, unsigned long aMask);
   virtual void CreateNative(Window aParent, nsRect aRect);
-  virtual void DestroyNative(void);
+  virtual void DestroyNative();
+  virtual void DestroyNativeChildren();
   void         CreateGC(void);
   void         Map(void);
   void         Unmap(void);

widget/src/xlib/nsWidgetFactory.cpp

@@ -206,4 +206,12 @@ static nsModuleComponentInfo components[] =
     nsFileSpecWithUIImplConstructor }
 };
 
-NS_IMPL_NSGETMODULE("nsWidgetXLIBModule", components)
+PR_STATIC_CALLBACK(void)
+nsWidgetXLIBModuleDtor(nsIModule *self)
+{
+  nsClipboard::Shutdown();
+}
+
+NS_IMPL_NSGETMODULE_WITH_DTOR("nsWidgetXLIBModule",
+                              components,
+                              nsWidgetXLIBModuleDtor)

widget/src/xlib/nsWindow.cpp

@@ -366,59 +366,6 @@ NS_IMETHODIMP nsWindow::SetFocus(void)
   return NS_OK;
 }
 
-
-// Borrowed heavily from GTK. This should go through heirarchy of XWindow
-// windows, and destroy the appropriate children. 
-// KenF
-void
-nsWindow::DestroyNativeChildren(void)
-{
-  Display     *display;
-  Window       window;
-  Window       root_return;
-  Window       parent_return;
-  Window      *children_return = NULL;
-  unsigned int nchildren_return = 0;
-  unsigned int i = 0;
-  
-  display = mDisplay;
-  window = mBaseWindow;
-  XQueryTree(display, window, &root_return, &parent_return,
-             &children_return, &nchildren_return);
-  // walk the list of children
-  for (i=0; i < nchildren_return; i++)
-    {
-      Window child_window = children_return[i];
-      nsWindow *thisWindow = (nsWindow*) GetWidgetForWindow(child_window);
-      if (thisWindow)
-        {
-          thisWindow->Destroy();
-        }
-    }      
-
-  // free up this struct
-  if (children_return)
-    XFree(children_return);
-}
-
-void nsWindow::DestroyNative(void)
-{
-
-  // Destroy Children. DOH!!!! KenF
-  DestroyNativeChildren();
-
-  // This is handled in nsDrawingSurfaceXlib for now
-#if 0
-  if (mGC)
-    XFreeGC(mDisplay, mGC);
-#endif
-
-  if (mBaseWindow) {
-      XDestroyWindow(mDisplay, mBaseWindow);
-      DeleteWindowCallback(mBaseWindow);
-  }
-}
-
 /* NOTE: Originally, nsWindow just uses Resize from nsWidget. 
  * Changed so it will first use the nsWidget resizing routine, then
  * send out a NS_SIZE event. This makes sure that the resizing is known
@@ -588,8 +535,10 @@ NS_IMETHODIMP nsWindow::Update()
     if (numRects != 1 && numRects < 10) {
       nsRegionRectSet *regionRectSet = nsnull;
 
-      if (NS_FAILED(mUpdateArea->GetRects(&regionRectSet)))
+      if (NS_FAILED(mUpdateArea->GetRects(&regionRectSet))) {
+        delete pevent.rect;
         return NS_ERROR_FAILURE;
+      }
 
       PRUint32 len;
       PRUint32 i;
@@ -609,6 +558,8 @@ NS_IMETHODIMP nsWindow::Update()
       }
       mUpdateArea->FreeRects(regionRectSet);
       mUpdateArea->SetTo(0,0,0,0);
+
+      delete pevent.rect;
       return NS_OK;
     } else {
       PRInt32 x,y,w,h;

widget/src/xlib/nsWindow.h

@@ -99,9 +99,6 @@ class nsWindow : public nsWidget
   NS_IMETHOD GetAttention(void);
   
 protected:
-  virtual void DestroyNative(void);
-  virtual void DestroyNativeChildren(void);
-
   virtual long GetEventMask();
 
   // Keyboard and Pointer Grabbing

widget/timer/src/unix/xlib/nsTimerXlib.cpp

@@ -79,7 +79,27 @@ nsTimerXlib::~nsTimerXlib()
   NS_IF_RELEASE(mCallback);
 }
 
-NS_IMPL_ISUPPORTS(nsTimerXlib, kITimerIID)
+/*static*/ void nsTimerXlib::Shutdown()
+{
+  delete nsTimerXlib::gHighestList;
+  nsTimerXlib::gHighestList = nsnull;
+
+  delete nsTimerXlib::gHighList;
+  nsTimerXlib::gHighList = nsnull;
+
+  delete nsTimerXlib::gNormalList;
+  nsTimerXlib::gNormalList = nsnull;
+
+  delete nsTimerXlib::gLowList;
+  nsTimerXlib::gLowList = nsnull;
+
+  delete nsTimerXlib::gLowestList;
+  nsTimerXlib::gLowestList = nsnull;
+
+  nsTimerXlib::gTimeoutAdded = PR_FALSE;
+}
+
+NS_IMPL_ISUPPORTS1(nsTimerXlib, nsITimer)
 
 nsresult
 nsTimerXlib::Init(nsTimerCallbackFunc aFunc,
@@ -90,7 +110,6 @@ nsTimerXlib::Init(nsTimerCallbackFunc aFunc,
 {
   mFunc = aFunc;
   mClosure = aClosure;
-  mPriority = aPriority;
   mType = aType;
   return Init(aDelay, aPriority);
 }
@@ -116,6 +135,7 @@ nsTimerXlib::Init(PRUint32 aDelay, PRUint32 aPriority)
 #endif
   
   mDelay = aDelay;
+  mPriority = aPriority;
   // get the cuurent time
   
   gettimeofday(&Now, NULL);
@@ -162,9 +182,6 @@ nsTimerXlib::Init(PRUint32 aDelay, PRUint32 aPriority)
       break;
   }
 
-  //FIXME Do we need this???? 
-  NS_ADDREF(this);
-
   EnsureWindowService();
 
   return NS_OK;

widget/timer/src/unix/xlib/nsTimerXlib.h

@@ -37,6 +37,7 @@ public:
   static void ProcessTimeouts(nsVoidArray *array);
   nsTimerXlib();
   virtual ~nsTimerXlib();
+  static void Shutdown();
 
   virtual nsresult Init(nsTimerCallbackFunc aFunc,
                 void *aClosure,

widget/timer/src/unix/xlib/nsTimerXlibFactory.cpp

@@ -48,4 +48,12 @@ static nsModuleComponentInfo components[] =
     nsTimerXlibConstructor }
 };
 
-NS_IMPL_NSGETMODULE("nsXlibTimerModule", components)
+PR_STATIC_CALLBACK(void)
+nsXlibTimerModuleDtor(nsIModule *self)
+{
+    nsTimerXlib::Shutdown();
+}
+
+NS_IMPL_NSGETMODULE_WITH_DTOR("nsXlibTimerModule",
+                              components,
+                              nsXlibTimerModuleDtor)