oleaut32: Fixes HFONT handling for AddRefHfont and ReleaseHfont
[RESEND]
Benjamin Arai
me at benjaminarai.com
Thu Aug 17 19:57:24 CDT 2006
Hi,
Fixes bug http://bugs.winehq.org/show_bug.cgi?id=4784
The conformance test for AddRefHfont confirms that when a HFONT ref
reaches 0 it is not removed from the list, it actually waits for all
of the IFONTs to reach a ref count of 0 before clearing the HFONT
list. The conformance test for AddRefHfont (lines: 768-806) shows
that Windows waits until all IFONTs have a ref count of 0 before
releasing the HFONT list.
This is tested at the bottom of the AddRefHfont conformance test by
decrementing a HFONT down to 0 then attempting to increment it again,
since this works the HFONT must still exist after reaching a ref count
of 0. In addition, I tested to see what happens when all of the
IFONTs reach a ref count of zero and as expected in this case all the
HFONTs are cleared. Thanks to Rob Shearman for the helpful emails.
ChangeLog:
- Modifies ReleaseHfont & AddRefHfont to update HFONT handling
- Updates IFont_Release to cleanup HFONT list if all IFONT's have
been released.
- Adds HFONT list inserts in IFont_Clone and IFont_get_Font.
- Makes all list and counters thread-safe (removed interlocked
increment/decrement for "ref" and "ifont_cnt" because the critical
section covers all sections where interlocked... was used)
- Removes all todo's from AddRefHfont and ReleaseHfont conformance tests
Licencse: LGPL
Copyright Google
---
dlls/oleaut32/olefont.c | 166 +++++++++++++++++++++++++++++++++--------
dlls/oleaut32/tests/olefont.c | 52 ++++++-------
2 files changed, 161 insertions(+), 57 deletions(-)
--
Benjamin Arai
http://www.benjaminarai.com
-------------- next part --------------
diff --git a/dlls/oleaut32/olefont.c b/dlls/oleaut32/olefont.c
index ac0bc8f..1af8213 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,41 @@ #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 _WINE_HFONTITEM
+{
+ struct list entry;
+
+ /* Reference count for that instance of the class. */
+ LONG ref;
+
+ /* Contain the font associated with this object. */
+ HFONT gdiFont;
+
+} WINE_HFONTITEM, *PWINE_HFONTITEM;
+
+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 };
+
/***********************************************************************
* Declaration of the implementation class for the IFont interface
*/
@@ -87,11 +123,6 @@ struct OLEFontImpl
HFONT gdiFont;
/*
- * Font lock count.
- */
- DWORD fontLock;
-
- /*
* Size ratio
*/
long cyLogical;
@@ -327,6 +358,10 @@ HRESULT WINAPI OleCreateFontIndirect(
if (ppvObj==0)
return E_POINTER;
+ EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+ ifont_cnt++;
+ LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
+
*ppvObj = 0;
if (!lpFontDesc) {
@@ -406,6 +441,7 @@ static void OLEFont_SendNotify(OLEFontIm
CONNECTDATA CD;
HRESULT hres;
+ this->gdiFont = 0;
hres = IConnectionPoint_EnumConnections(this->pPropertyNotifyCP, &pEnum);
if (SUCCEEDED(hres))
{
@@ -509,7 +545,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;
@@ -617,8 +652,12 @@ ULONG WINAPI OLEFontImpl_AddRef(
IFont* iface)
{
OLEFontImpl *this = (OLEFontImpl *)iface;
+ ULONG ret;
TRACE("(%p)->(ref=%ld)\n", this, this->ref);
- return InterlockedIncrement(&this->ref);
+ EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+ ret = ++this->ref;
+ LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
+ return ret;
}
/************************************************************************
@@ -631,17 +670,30 @@ ULONG WINAPI OLEFontImpl_Release(
{
OLEFontImpl *this = (OLEFontImpl *)iface;
ULONG ret;
+ PWINE_HFONTITEM ptr, next;
TRACE("(%p)->(ref=%ld)\n", this, this->ref);
- /*
- * Decrease the reference count on this object.
- */
- ret = InterlockedDecrement(&this->ref);
+ EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+ /* Decrease the reference count for current interface */
+ ret = --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 (this->ref == 0)
+ {
+ ifont_cnt--;
+ /* Check if all HFONT list refs are zero */
+ if (ifont_cnt == 0)
+ {
+ LIST_FOR_EACH_ENTRY_SAFE(ptr, next, &OLEFontImpl_hFontList, WINE_HFONTITEM, entry)
+ {
+ list_remove(&ptr->entry);
+ DeleteObject(&ptr->gdiFont);
+ HeapFree(GetProcessHeap(), 0 , ptr);
+ }
+ }
+ OLEFontImpl_Destroy(this);
+ }
+ LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
return ret;
}
@@ -1013,6 +1065,7 @@ static HRESULT WINAPI OLEFontImpl_get_hF
LOGFONTW logFont;
INT fontHeight;
CY cySize;
+ PWINE_HFONTITEM newEntry;
/*
* The height of the font returned by the get_Size property is the
@@ -1042,6 +1095,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(WINE_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 +1123,7 @@ static HRESULT WINAPI OLEFontImpl_Clone(
LOGFONTW logFont;
INT fontHeight;
CY cySize;
+ PWINE_HFONTITEM newEntry;
OLEFontImpl *this = (OLEFontImpl *)iface;
TRACE("(%p)->(%p)\n", this, ppfont);
@@ -1110,6 +1172,15 @@ static HRESULT WINAPI OLEFontImpl_Clone(
newObject->gdiFont = CreateFontIndirectW(&logFont);
+ /* Add font to the cache */
+ newEntry = HeapAlloc(GetProcessHeap(), 0, sizeof(WINE_HFONTITEM));
+ newEntry->ref = 1;
+ newEntry->gdiFont = newObject->gdiFont;
+ EnterCriticalSection(&OLEFontImpl_csHFONTLIST);
+ ifont_cnt++;
+ list_add_tail(&OLEFontImpl_hFontList,&newEntry->entry);
+ LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
+
/* create new connection points */
newObject->pPropertyNotifyCP = NULL;
newObject->pFontEventsCP = NULL;
@@ -1222,15 +1293,30 @@ static HRESULT WINAPI OLEFontImpl_AddRef
HFONT hfont)
{
OLEFontImpl *this = (OLEFontImpl *)iface;
- TRACE("(%p)->(%p) (lock=%ld)\n", this, hfont, this->fontLock);
+ PWINE_HFONTITEM ptr, next;
+ PWINE_HFONTITEM foundEntry = NULL;
+ TRACE("(%p)->(%p)\n", this, hfont);
- if ( (hfont == 0) ||
- (hfont != this->gdiFont) )
+ if (hfont == 0)
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, WINE_HFONTITEM, entry)
+ {
+ if (ptr->gdiFont == hfont)
+ {
+ foundEntry = ptr;
+ break;
+ }
+ }
- return S_OK;
+ if (foundEntry)
+ foundEntry->ref++;
+
+ LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
+
+ return foundEntry ? S_OK : S_FALSE;
}
/************************************************************************
@@ -1243,24 +1329,42 @@ static HRESULT WINAPI OLEFontImpl_Releas
HFONT hfont)
{
OLEFontImpl *this = (OLEFontImpl *)iface;
- TRACE("(%p)->(%p) (lock=%ld)\n", this, hfont, this->fontLock);
+ PWINE_HFONTITEM ptr, next;
+ PWINE_HFONTITEM foundEntry = NULL;
+ HRESULT hres;
+
+ TRACE("(%p)->(%p)\n", this, hfont);
- if ( (hfont == 0) ||
- (hfont != this->gdiFont) )
+ if (hfont == 0)
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, WINE_HFONTITEM, entry)
+ {
+ if (ptr->gdiFont == hfont)
+ {
+ foundEntry = ptr;
+ break;
+ }
+ }
- /*
- * If we just released our last font reference, destroy it.
- */
- if (this->fontLock==0)
+ /* Check if the font is not locked in the cache. */
+ if (!foundEntry || foundEntry->ref == 0)
+ hres = S_FALSE;
+ else
{
- DeleteObject(this->gdiFont);
- this->gdiFont = 0;
+ /* Remove from cache and delete object if not referenced */
+ if (--foundEntry->ref == 0)
+ {
+ DeleteObject(this->gdiFont);
+ this->gdiFont = 0;
+ }
+ hres = S_OK;
}
+ LeaveCriticalSection(&OLEFontImpl_csHFONTLIST);
- return S_OK;
+ return hres;
}
/************************************************************************
diff --git a/dlls/oleaut32/tests/olefont.c b/dlls/oleaut32/tests/olefont.c
index 1192dc9..bc0aa9b 100644
--- a/dlls/oleaut32/tests/olefont.c
+++ b/dlls/oleaut32/tests/olefont.c
@@ -640,9 +640,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%08lx\n",
- hres);}
+ hres);
/* Release all refs */
hres = IFont_ReleaseHfont(ifnt1,hfnt1);
@@ -657,15 +657,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%08lx\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%08lx\n",
- hres);}
+ hres);
IFont_Release(ifnt1);
IFont_Release(ifnt2);
@@ -715,9 +715,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%08lx\n",
- hres);}
+ hres);
/* Add simple IFONT HFONT pair */
hres = IFont_AddRefHfont(ifnt1,hfnt1);
@@ -727,9 +727,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%08lx\n",
- hres);}
+ hres);
/* Release all hfnt1 refs */
hres = IFont_ReleaseHfont(ifnt1,hfnt1);
@@ -738,20 +738,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%08lx\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%08lx\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%08lx\n",
- hres);}
+ hres);
/* Release all hfnt2 refs */
hres = IFont_ReleaseHfont(ifnt2,hfnt2);
@@ -761,9 +761,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%08lx\n",
- hres);}
+ hres);
/* Show that releasing an IFONT does not always release it from the HFONT cache. */
@@ -771,15 +771,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%08lx\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%08lx\n",
- hres);}
+ hres);
/* Shows that releasing all IFONT's does clear the HFONT cache. */
@@ -793,15 +793,15 @@ static void test_AddRefHfont(void)
/* 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%08lx\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%08lx\n",
- hres);}
+ hres);
hres = IFont_Release(ifnt3);
}
--
1.4.0
More information about the wine-patches
mailing list