oleaut32: Fix IFont::AddRefHFont and IFont::ReleaseRefHFont.

Robert Shearman rob at codeweavers.com
Mon Feb 19 15:45:12 CST 2007


The tests show that there is a global cache that keeps references to 
HFONTs that is released when all IFont objects are released.

(Based on a patch by Benjamin Arai.)
---
  dlls/oleaut32/olefont.c       |  151 
+++++++++++++++++++++++++++++++++--------
  dlls/oleaut32/tests/olefont.c |   66 ++++++++----------
  2 files changed, 151 insertions(+), 66 deletions(-)
-------------- next part --------------
diff --git a/dlls/oleaut32/olefont.c b/dlls/oleaut32/olefont.c
index 706ecf0..3920b9d 100644
--- a/dlls/oleaut32/olefont.c
+++ b/dlls/oleaut32/olefont.c
@@ -34,6 +34,7 @@ #include "windef.h"
 #include "winbase.h"
 #include "wingdi.h"
 #include "winuser.h"
+#include "wine/list.h"
 #include "wine/unicode.h"
 #include "objbase.h"
 #include "oleauto.h"    /* for SysAllocString(....) */
@@ -52,6 +53,48 @@ #define FONTPERSIST_ITALIC        0x02
 #define FONTPERSIST_UNDERLINE     0x04
 #define FONTPERSIST_STRIKETHROUGH 0x08
 
+
+/***********************************************************************
+ * List of the HFONTs it has given out, with each one having a separate
+ * ref count.
+ */
+typedef struct _HFONTItem
+{
+  struct list entry;
+
+  /* Reference count for that instance of the class. */
+  LONG ref;
+
+  /* Contain the font associated with this object. */
+  HFONT gdiFont;
+
+} HFONTItem, *PHFONTItem;
+
+static struct list OLEFontImpl_hFontList = LIST_INIT(OLEFontImpl_hFontList);
+
+/* Counts how many fonts contain at least one lock */
+static LONG ifont_cnt = 0;
+
+/***********************************************************************
+ * Critical section for OLEFontImpl_hFontList
+ */
+static CRITICAL_SECTION OLEFontImpl_csHFONTLIST;
+static CRITICAL_SECTION_DEBUG OLEFontImpl_csHFONTLIST_debug =
+{
+  0, 0, &OLEFontImpl_csHFONTLIST,
+  { &OLEFontImpl_csHFONTLIST_debug.ProcessLocksList,
+    &OLEFontImpl_csHFONTLIST_debug.ProcessLocksList },
+    0, 0, { (DWORD_PTR)(__FILE__ ": OLEFontImpl_csHFONTLIST") }
+};
+static CRITICAL_SECTION OLEFontImpl_csHFONTLIST = { &OLEFontImpl_csHFONTLIST_debug, -1, 0, 0, 0, 0 };
+
+static void HFONTItem_Delete(PHFONTItem item)
+{
+  DeleteObject(item->gdiFont);
+  list_remove(&item->entry);
+  HeapFree(GetProcessHeap(), 0, item);
+}
+
 /***********************************************************************
  * Declaration of the implementation class for the IFont interface
  */
@@ -87,11 +130,6 @@ struct OLEFontImpl
   HFONT gdiFont;
 
   /*
-   * Font lock count.
-   */
-  DWORD fontLock;
-
-  /*
    * Size ratio
    */
   long cyLogical;
@@ -406,6 +444,7 @@ static void OLEFont_SendNotify(OLEFontIm
   CONNECTDATA CD;
   HRESULT hres;
 
+  this->gdiFont = 0;
   hres = IConnectionPoint_EnumConnections(this->pPropertyNotifyCP, &pEnum);
   if (SUCCEEDED(hres))
   {
@@ -509,7 +548,6 @@ static OLEFontImpl* OLEFontImpl_Construc
    * Initializing all the other members.
    */
   newObject->gdiFont  = 0;
-  newObject->fontLock = 0;
   newObject->cyLogical  = 72L;
   newObject->cyHimetric = 2540L;
   newObject->pPropertyNotifyCP = NULL;
@@ -524,6 +562,8 @@ static OLEFontImpl* OLEFontImpl_Construc
     return NULL;
   }
 
+  InterlockedIncrement(&ifont_cnt);
+
   TRACE("returning %p\n", newObject);
   return newObject;
 }
@@ -631,17 +671,26 @@ ULONG WINAPI OLEFontImpl_Release(
 {
   OLEFontImpl *this = (OLEFontImpl *)iface;
   ULONG ret;
+  PHFONTItem ptr, next;
   TRACE("(%p)->(ref=%d)\n", this, this->ref);
 
-  /*
-   * Decrease the reference count on this object.
-   */
+  /* Decrease the reference count for current interface */
   ret = InterlockedDecrement(&this->ref);
 
-  /*
-   * If the reference count goes down to 0, perform suicide.
-   */
-  if (ret==0) OLEFontImpl_Destroy(this);
+  /* If the reference count goes down to 0, destroy. */
+  if (ret == 0)
+  {
+    ULONG fontlist_refs = InterlockedDecrement(&ifont_cnt);
+    /* Check if all HFONT list refs are zero */
+    if (fontlist_refs == 0)
+    {
+      EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+      LIST_FOR_EACH_ENTRY_SAFE(ptr, next, &OLEFontImpl_hFontList, HFONTItem, entry)
+        HFONTItem_Delete(ptr);
+      LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
+    }
+    OLEFontImpl_Destroy(this);
+  }
 
   return ret;
 }
@@ -1013,6 +1062,7 @@ static HRESULT WINAPI OLEFontImpl_get_hF
     LOGFONTW logFont;
     INT      fontHeight;
     CY       cySize;
+    PHFONTItem newEntry;
 
     /*
      * The height of the font returned by the get_Size property is the
@@ -1042,6 +1092,14 @@ static HRESULT WINAPI OLEFontImpl_get_hF
     strcpyW(logFont.lfFaceName,this->description.lpstrName);
 
     this->gdiFont = CreateFontIndirectW(&logFont);
+
+    /* Add font to the cache */
+    newEntry = HeapAlloc(GetProcessHeap(), 0, sizeof(HFONTItem));
+    newEntry->ref = 1;
+    newEntry->gdiFont = this->gdiFont;
+    EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+    list_add_tail(&OLEFontImpl_hFontList,&newEntry->entry);
+    LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
   }
 
   *phfont = this->gdiFont;
@@ -1062,6 +1120,8 @@ static HRESULT WINAPI OLEFontImpl_Clone(
   LOGFONTW logFont;
   INT      fontHeight;
   CY       cySize;
+  PHFONTItem newEntry;
+
   OLEFontImpl *this = (OLEFontImpl *)iface;
   TRACE("(%p)->(%p)\n", this, ppfont);
 
@@ -1110,6 +1170,15 @@ static HRESULT WINAPI OLEFontImpl_Clone(
 
   newObject->gdiFont = CreateFontIndirectW(&logFont);
 
+  /* Add font to the cache */
+  InterlockedIncrement(&ifont_cnt);
+  newEntry = HeapAlloc(GetProcessHeap(), 0, sizeof(HFONTItem));
+  newEntry->ref = 1;
+  newEntry->gdiFont = newObject->gdiFont;
+  EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+  list_add_tail(&OLEFontImpl_hFontList,&newEntry->entry);
+  LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
+
   /* create new connection points */
   newObject->pPropertyNotifyCP = NULL;
   newObject->pFontEventsCP = NULL;
@@ -1222,15 +1291,28 @@ static HRESULT WINAPI OLEFontImpl_AddRef
   HFONT hfont)
 {
   OLEFontImpl *this = (OLEFontImpl *)iface;
-  TRACE("(%p)->(%p) (lock=%d)\n", this, hfont, this->fontLock);
+  PHFONTItem ptr, next;
+  HRESULT hres = S_FALSE; /* assume not present */
 
-  if ( (hfont == 0) ||
-       (hfont != this->gdiFont) )
+  TRACE("(%p)->(%p)\n", this, hfont);
+
+  if (!hfont)
     return E_INVALIDARG;
 
-  this->fontLock++;
+  /* Check of the hFont is already in the list */
+  EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+  LIST_FOR_EACH_ENTRY_SAFE(ptr, next, &OLEFontImpl_hFontList, HFONTItem, entry)
+  {
+    if (ptr->gdiFont == hfont)
+    {
+      ptr->ref++;
+      hres = S_OK;
+      break;
+    }
+  }
+  LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
 
-  return S_OK;
+  return hres;
 }
 
 /************************************************************************
@@ -1243,24 +1325,33 @@ static HRESULT WINAPI OLEFontImpl_Releas
   HFONT hfont)
 {
   OLEFontImpl *this = (OLEFontImpl *)iface;
-  TRACE("(%p)->(%p) (lock=%d)\n", this, hfont, this->fontLock);
+  PHFONTItem ptr, next;
+  HRESULT hres = S_FALSE; /* assume not present */
 
-  if ( (hfont == 0) ||
-       (hfont != this->gdiFont) )
-    return E_INVALIDARG;
+  TRACE("(%p)->(%p)\n", this, hfont);
 
-  this->fontLock--;
+  if (!hfont)
+    return E_INVALIDARG;
 
-  /*
-   * If we just released our last font reference, destroy it.
-   */
-  if (this->fontLock==0)
+  /* Check of the hFont is already in the list */
+  EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+  LIST_FOR_EACH_ENTRY_SAFE(ptr, next, &OLEFontImpl_hFontList, HFONTItem, entry)
   {
-    DeleteObject(this->gdiFont);
-    this->gdiFont = 0;
+    if ((ptr->gdiFont == hfont) && ptr->ref)
+    {
+      /* Remove from cache and delete object if not referenced */
+      if (!--ptr->ref)
+      {
+        if (ptr->gdiFont == this->gdiFont)
+          this->gdiFont = NULL;
+      }
+      hres = S_OK;
+      break;
+    }
   }
+  LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
 
-  return S_OK;
+  return hres;
 }
 
 /************************************************************************
diff --git a/dlls/oleaut32/tests/olefont.c b/dlls/oleaut32/tests/olefont.c
index 7fc0422..c9ea99f 100644
--- a/dlls/oleaut32/tests/olefont.c
+++ b/dlls/oleaut32/tests/olefont.c
@@ -638,9 +638,9 @@ static void test_ReleaseHfont(void)
 
     /* Try to add a bad HFONT */
     hres = IFont_ReleaseHfont(ifnt1,(HFONT)32);
-    todo_wine {ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_ReleaseHfont: (Bad HFONT) Expected S_FALSE but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Release all refs */
     hres = IFont_ReleaseHfont(ifnt1,hfnt1);
@@ -655,15 +655,15 @@ static void test_ReleaseHfont(void)
 
     /* Check that both lists are empty */
     hres = IFont_ReleaseHfont(ifnt1,hfnt1);
-    todo_wine {ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_AddRefHfont: (Release ref) Expected S_FALSE but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* The list should be empty */
     hres = IFont_ReleaseHfont(ifnt2,hfnt2);
-    todo_wine {ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_AddRefHfont: (Release ref) Expected S_FALSE but got 0x%08x\n",
-        hres);}
+        hres);
 
     IFont_Release(ifnt1);
     IFont_Release(ifnt2);
@@ -672,9 +672,6 @@ static void test_ReleaseHfont(void)
 static void test_AddRefHfont(void)
 {
     FONTDESC fd;
-    LPVOID pvObj1 = NULL;
-    LPVOID pvObj2 = NULL;
-    LPVOID pvObj3 = NULL;
     IFont* ifnt1 = NULL;
     IFont* ifnt2 = NULL;
     IFont* ifnt3 = NULL;
@@ -695,12 +692,10 @@ static void test_AddRefHfont(void)
     fd.fStrikethrough = 0;
 
     /* Create HFONTs and IFONTs */
-    pOleCreateFontIndirect(&fd, &IID_IFont, &pvObj1);
-    ifnt1 = pvObj1;
+    pOleCreateFontIndirect(&fd, &IID_IFont, (void **)&ifnt1);
     IFont_get_hFont(ifnt1,&hfnt1);
     fd.lpstrName = arial_font;
-    pOleCreateFontIndirect(&fd, &IID_IFont, &pvObj2);
-    ifnt2 = pvObj2;
+    pOleCreateFontIndirect(&fd, &IID_IFont, (void **)&ifnt2);
     IFont_get_hFont(ifnt2,&hfnt2);
 
     /* Try invalid HFONT */
@@ -711,9 +706,9 @@ static void test_AddRefHfont(void)
 
     /* Try to add a bad HFONT */
     hres = IFont_AddRefHfont(ifnt1,(HFONT)32);
-    todo_wine{ ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_AddRefHfont: (Bad HFONT) Expected S_FALSE but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Add simple IFONT HFONT pair */
     hres = IFont_AddRefHfont(ifnt1,hfnt1);
@@ -723,9 +718,9 @@ static void test_AddRefHfont(void)
 
     /* IFONT and HFONT do not have to be the same (always looks at HFONT) */
     hres = IFont_AddRefHfont(ifnt2,hfnt1);
-    todo_wine {ok(hres == S_OK,
+    ok(hres == S_OK,
         "IFont_AddRefHfont: (Add ref) Expected S_OK but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Release all hfnt1 refs */
     hres = IFont_ReleaseHfont(ifnt1,hfnt1);
@@ -734,20 +729,20 @@ static void test_AddRefHfont(void)
         hres);
 
     hres = IFont_ReleaseHfont(ifnt1,hfnt1);
-    todo_wine {ok(hres == S_OK,
+    ok(hres == S_OK,
         "IFont_AddRefHfont: (Release ref) Expected S_OK but got 0x%08x\n",
-        hres);}
+        hres);
 
     hres = IFont_ReleaseHfont(ifnt1,hfnt1);
-    todo_wine {ok(hres == S_OK,
+    ok(hres == S_OK,
         "IFont_AddRefHfont: (Release ref) Expected S_OK but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Check if hfnt1 is empty */
     hres = IFont_ReleaseHfont(ifnt1,hfnt1);
-    todo_wine {ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_AddRefHfont: (Release ref) Expected S_FALSE but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Release all hfnt2 refs */
     hres = IFont_ReleaseHfont(ifnt2,hfnt2);
@@ -757,9 +752,9 @@ static void test_AddRefHfont(void)
 
     /* Check if hfnt2 is empty */
     hres = IFont_ReleaseHfont(ifnt2,hfnt2);
-    todo_wine {ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_AddRefHfont: (Release ref) Expected S_FALSE but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Show that releasing an IFONT does not always release it from the HFONT cache. */
 
@@ -767,15 +762,15 @@ static void test_AddRefHfont(void)
 
     /* Add a reference for destroyed hfnt1 */
     hres = IFont_AddRefHfont(ifnt2,hfnt1);
-    todo_wine {ok(hres == S_OK,
+    ok(hres == S_OK,
         "IFont_AddRefHfont: (Add ref) Expected S_OK but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Decrement reference for destroyed hfnt1 */
     hres = IFont_ReleaseHfont(ifnt2,hfnt1);
-    todo_wine {ok(hres == S_OK,
+    ok(hres == S_OK,
         "IFont_AddRefHfont: (Release ref) Expected S_OK but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Shows that releasing all IFONT's does clear the HFONT cache. */
 
@@ -783,23 +778,22 @@ static void test_AddRefHfont(void)
 
     /* Need to make a new IFONT for testing */
     fd.fUnderline = 1;
-    pOleCreateFontIndirect(&fd, &IID_IFont, &pvObj3);
-    ifnt3 = pvObj3;
+    pOleCreateFontIndirect(&fd, &IID_IFont, (void **)&ifnt3);
     IFont_get_hFont(ifnt3,&hfnt3);
 
     /* Add a reference for destroyed hfnt1 */
     hres = IFont_AddRefHfont(ifnt3,hfnt1);
-    todo_wine {ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_AddRefHfont: (Add ref) Expected S_OK but got 0x%08x\n",
-        hres);}
+        hres);
 
     /* Decrement reference for destroyed hfnt1 */
     hres = IFont_ReleaseHfont(ifnt3,hfnt1);
-    todo_wine {ok(hres == S_FALSE,
+    ok(hres == S_FALSE,
         "IFont_AddRefHfont: (Release ref) Expected S_OK but got 0x%08x\n",
-        hres);}
+        hres);
 
-    hres = IFont_Release(ifnt3);
+    IFont_Release(ifnt3);
 }
 
 START_TEST(olefont)


More information about the wine-patches mailing list