Make the node object aggregatable so that the element object (and in

dlls/msxml3/domdoc.c

@@ -23,6 +23,7 @@
 #include "config.h"
 
 #include <stdarg.h>
+#include <assert.h>
 #include "windef.h"
 #include "winbase.h"
 #include "winuser.h"
@@ -43,6 +44,7 @@ typedef struct _domdoc
     const struct IXMLDOMDocumentVtbl *lpVtbl;
     LONG ref;
     VARIANT_BOOL async;
+    IUnknown *node_unk;
     IXMLDOMNode *node;
 } domdoc;
 
@@ -83,12 +85,15 @@ static HRESULT WINAPI domdoc_QueryInterface( IXMLDOMDocument *iface, REFIID riid
     TRACE("%p %s %p\n", This, debugstr_guid( riid ), ppvObject );
 
     if ( IsEqualGUID( riid, &IID_IXMLDOMDocument ) ||
-         IsEqualGUID( riid, &IID_IXMLDOMNode ) ||
-         IsEqualGUID( riid, &IID_IDispatch ) ||
          IsEqualGUID( riid, &IID_IUnknown ) )
     {
         *ppvObject = iface;
     }
+    else if ( IsEqualGUID( riid, &IID_IXMLDOMNode ) ||
+              IsEqualGUID( riid, &IID_IDispatch ) )
+    {
+        return IUnknown_QueryInterface(This->node_unk, riid, ppvObject);
+    }
     else
         return E_NOINTERFACE;
 
@@ -118,8 +123,7 @@ static ULONG WINAPI domdoc_Release(
     ref = InterlockedDecrement( &This->ref );
     if ( ref == 0 )
     {
-        if ( This->node )
-            IXMLDOMElement_Release( This->node );
+        IUnknown_Release( This->node_unk );
         HeapFree( GetProcessHeap(), 0, This );
     }
 
@@ -527,6 +531,8 @@ static HRESULT WINAPI domdoc_get_documentElement(
     domdoc *This = impl_from_IXMLDOMDocument( iface );
     xmlDocPtr xmldoc = NULL;
     xmlNodePtr root = NULL;
+    IXMLDOMNode *element_node;
+    HRESULT hr;
 
     TRACE("%p\n", This);
 
@@ -543,9 +549,13 @@ static HRESULT WINAPI domdoc_get_documentElement(
     if ( !root )
         return S_FALSE;
 
-    *DOMElement = create_element( root );
+    element_node = create_node( root );
+    if(!element_node) return S_FALSE;
 
-    return S_OK;
+    hr = IXMLDOMNode_QueryInterface(element_node, &IID_IXMLDOMElement, (LPVOID*)DOMElement);
+    IXMLDOMNode_Release(element_node);
+
+    return hr;
 }
 
 
@@ -729,11 +739,11 @@ static HRESULT WINAPI domdoc_load(
 
     TRACE("type %d\n", V_VT(&xmlSource) );
 
-    if ( This->node )
-    {
-        IXMLDOMNode_Release( This->node );
-        This->node = NULL;
-    }
+    *isSuccessful = VARIANT_FALSE;
+
+    assert( This->node );
+
+    attach_xmlnode(This->node, NULL);
 
     switch( V_VT(&xmlSource) )
     {
@@ -745,17 +755,10 @@ static HRESULT WINAPI domdoc_load(
         return S_FALSE;
 
     xmldoc = doread( filename );
-    if ( !xmldoc ) {
-        *isSuccessful = VARIANT_FALSE;
-        return S_FALSE;
-    }
+    if ( !xmldoc ) return S_FALSE;
 
-    This->node = create_node( (xmlNodePtr) xmldoc );
-    if ( !This->node )
-    {
-        *isSuccessful = VARIANT_FALSE;
-        return S_FALSE;
-    }
+    xmldoc->_private = 0;
+    attach_xmlnode(This->node, (xmlNodePtr) xmldoc);
 
     *isSuccessful = VARIANT_TRUE;
     return S_OK;
@@ -850,11 +853,9 @@ static HRESULT WINAPI domdoc_loadXML(
 
     TRACE("%p %s %p\n", This, debugstr_w( bstrXML ), isSuccessful );
 
-    if ( This->node )
-    {
-        IXMLDOMNode_Release( This->node );
-        This->node = NULL;
-    }
+    assert ( This->node );
+
+    attach_xmlnode( This->node, NULL );
 
     if ( !isSuccessful )
         return S_FALSE;
@@ -869,11 +870,12 @@ static HRESULT WINAPI domdoc_loadXML(
 
     xmldoc = doparse( str, len );
     HeapFree( GetProcessHeap(), 0, str );
-
-    This->node = create_node( (xmlNodePtr) xmldoc );
-    if( !This->node )
+    if ( !xmldoc )
         return S_FALSE;
 
+    xmldoc->_private = 0;
+    attach_xmlnode( This->node, (xmlNodePtr) xmldoc );
+
     *isSuccessful = VARIANT_TRUE;
     return S_OK;
 }
@@ -1049,6 +1051,9 @@ const struct IXMLDOMDocumentVtbl domdoc_vtbl =
 HRESULT DOMDocument_create(IUnknown *pUnkOuter, LPVOID *ppObj)
 {
     domdoc *doc;
+    HRESULT hr;
+
+    TRACE("(%p,%p)\n", pUnkOuter, ppObj);
 
     doc = HeapAlloc( GetProcessHeap(), 0, sizeof (*doc) );
     if( !doc )
@@ -1057,10 +1062,27 @@ HRESULT DOMDocument_create(IUnknown *pUnkOuter, LPVOID *ppObj)
     doc->lpVtbl = &domdoc_vtbl;
     doc->ref = 1;
     doc->async = 0;
-    doc->node = NULL;
+
+    doc->node_unk = create_basic_node( NULL, (IUnknown*)&doc->lpVtbl );
+    if(!doc->node_unk)
+    {
+        HeapFree(GetProcessHeap(), 0, doc);
+        return E_FAIL;
+    }
+
+    hr = IUnknown_QueryInterface(doc->node_unk, &IID_IXMLDOMNode, (LPVOID*)&doc->node);
+    if(FAILED(hr))
+    {
+        IUnknown_Release(doc->node_unk);
+        HeapFree( GetProcessHeap(), 0, doc );
+        return E_FAIL;
+    }
+    /* The ref on doc->node is actually looped back into this object, so release it */
+    IXMLDOMNode_Release(doc->node);
 
     *ppObj = &doc->lpVtbl;
 
+    TRACE("returning iface %p\n", *ppObj);
     return S_OK;
 }
 

dlls/msxml3/element.c

@@ -41,6 +41,7 @@ typedef struct _domelem
 {
     const struct IXMLDOMElementVtbl *lpVtbl;
     LONG ref;
+    IUnknown *node_unk;
     IXMLDOMNode *node;
 } domelem;
 
@@ -59,15 +60,19 @@ static HRESULT WINAPI domelem_QueryInterface(
     REFIID riid,
     void** ppvObject )
 {
-    TRACE("%p %s %p\n", iface, debugstr_guid(riid), ppvObject);
+    domelem *This = impl_from_IXMLDOMElement( iface );
+    TRACE("%p %s %p\n", This, debugstr_guid(riid), ppvObject);
 
     if ( IsEqualGUID( riid, &IID_IXMLDOMElement ) ||
-         IsEqualGUID( riid, &IID_IUnknown ) ||
-         IsEqualGUID( riid, &IID_IDispatch ) ||
-         IsEqualGUID( riid, &IID_IXMLDOMNode ) )
+         IsEqualGUID( riid, &IID_IUnknown ) )
     {
         *ppvObject = iface;
     }
+    else if ( IsEqualGUID( riid, &IID_IDispatch ) ||
+              IsEqualGUID( riid, &IID_IXMLDOMNode ) )
+    {
+        return IUnknown_QueryInterface(This->node_unk, riid, ppvObject);
+    }
     else
         return E_NOINTERFACE;
 
@@ -92,7 +97,7 @@ static ULONG WINAPI domelem_Release(
     ref = InterlockedDecrement( &This->ref );
     if ( ref == 0 )
     {
-        IXMLDOMNode_Release( This->node );
+        IUnknown_Release( This->node_unk );
         HeapFree( GetProcessHeap(), 0, This );
     }
 
@@ -568,25 +573,36 @@ static const struct IXMLDOMElementVtbl domelem_vtbl =
     domelem_normalize,
 };
 
-IXMLDOMElement* create_element( xmlNodePtr element )
+IUnknown* create_element( xmlNodePtr element )
 {
     domelem *This;
+    HRESULT hr;
 
     This = HeapAlloc( GetProcessHeap(), 0, sizeof *This );
     if ( !This )
         return NULL;
 
     This->lpVtbl = &domelem_vtbl;
-    This->node = create_node( element );
     This->ref = 1;
 
-    if ( !This->node )
+    This->node_unk = create_basic_node( element, (IUnknown*)&This->lpVtbl );
+    if(!This->node_unk)
+    {
+        HeapFree(GetProcessHeap(), 0, This);
+        return NULL;
+    }
+
+    hr = IUnknown_QueryInterface(This->node_unk, &IID_IXMLDOMNode, (LPVOID*)&This->node);
+    if(FAILED(hr))
     {
+        IUnknown_Release(This->node_unk);
         HeapFree( GetProcessHeap(), 0, This );
         return NULL;
     }
+    /* The ref on This->node is actually looped back into this object, so release it */
+    IXMLDOMNode_Release(This->node);
 
-    return (IXMLDOMElement*) &This->lpVtbl;
+    return (IUnknown*) &This->lpVtbl;
 }
 
 #endif

dlls/msxml3/msxml_private.h

@@ -31,11 +31,14 @@
 extern IUnknown         *create_domdoc( void );
 extern IUnknown         *create_xmldoc( void );
 extern IXMLDOMNode      *create_node( xmlNodePtr node );
-extern IXMLDOMElement   *create_element( xmlNodePtr element );
+extern IUnknown         *create_basic_node( xmlNodePtr node, IUnknown *pUnkOuter );
+extern IUnknown         *create_element( xmlNodePtr element );
 extern IXMLDOMNodeList  *create_nodelist( xmlNodePtr node );
 extern IXMLDOMNamedNodeMap *create_nodemap( IXMLDOMNode *node );
 extern IXMLDOMNodeList  *create_filtered_nodelist( xmlNodePtr, const xmlChar * );
 
+extern void attach_xmlnode( IXMLDOMNode *node, xmlNodePtr xmlnode );
+
 /* data accessors */
 xmlNodePtr xmlNodePtr_from_domnode( IXMLDOMNode *iface, xmlElementType type );
 

dlls/msxml3/node.c

@@ -42,6 +42,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(msxml);
 typedef struct _xmlnode
 {
     const struct IXMLDOMNodeVtbl *lpVtbl;
+    const struct IUnknownVtbl *lpInternalUnkVtbl;
+    IUnknown *pUnkOuter;
     LONG ref;
     xmlNodePtr node;
 } xmlnode;
@@ -51,6 +53,11 @@ static inline xmlnode *impl_from_IXMLDOMNode( IXMLDOMNode *iface )
     return (xmlnode *)((char*)iface - FIELD_OFFSET(xmlnode, lpVtbl));
 }
 
+static inline xmlnode *impl_from_InternalUnknown( IUnknown *iface )
+{
+    return (xmlnode *)((char*)iface - FIELD_OFFSET(xmlnode, lpInternalUnkVtbl));
+}
+
 xmlNodePtr xmlNodePtr_from_domnode( IXMLDOMNode *iface, xmlElementType type )
 {
     xmlnode *This;
@@ -65,49 +72,43 @@ xmlNodePtr xmlNodePtr_from_domnode( IXMLDOMNode *iface, xmlElementType type )
     return This->node;
 }
 
+void attach_xmlnode( IXMLDOMNode *node, xmlNodePtr xml )
+{
+    xmlnode *This = impl_from_IXMLDOMNode( node );
+
+    if(This->node)
+        xmldoc_release(This->node->doc);
+
+    This->node = xml;
+    if(This->node)
+        xmldoc_add_ref(This->node->doc);
+
+    return;
+}
+
 static HRESULT WINAPI xmlnode_QueryInterface(
     IXMLDOMNode *iface,
     REFIID riid,
     void** ppvObject )
 {
-    TRACE("%p %s %p\n", iface, debugstr_guid(riid), ppvObject);
-
-    if ( IsEqualGUID( riid, &IID_IUnknown ) ||
-         IsEqualGUID( riid, &IID_IDispatch ) ||
-         IsEqualGUID( riid, &IID_IXMLDOMNode ) )
-    {
-        *ppvObject = iface;
-    }
-    else
-        return E_NOINTERFACE;
-
-    IXMLDOMElement_AddRef( iface );
+    xmlnode *This = impl_from_IXMLDOMNode( iface );
+    TRACE("%p %s %p\n", This, debugstr_guid(riid), ppvObject);
 
-    return S_OK;
+    return IUnknown_QueryInterface(This->pUnkOuter, riid, ppvObject);
 }
 
 static ULONG WINAPI xmlnode_AddRef(
     IXMLDOMNode *iface )
 {
     xmlnode *This = impl_from_IXMLDOMNode( iface );
-    return InterlockedIncrement( &This->ref );
+    return IUnknown_AddRef(This->pUnkOuter);
 }
 
 static ULONG WINAPI xmlnode_Release(
     IXMLDOMNode *iface )
 {
     xmlnode *This = impl_from_IXMLDOMNode( iface );
-    ULONG ref;
-
-    ref = InterlockedDecrement( &This->ref );
-    if ( ref == 0 )
-    {
-        assert( This->node->doc );
-        xmldoc_release( This->node->doc );
-        HeapFree( GetProcessHeap(), 0, This );
-    }
-
-    return ref;
+    return IUnknown_Release(This->pUnkOuter);
 }
 
 static HRESULT WINAPI xmlnode_GetTypeInfoCount(
@@ -700,32 +701,114 @@ static const struct IXMLDOMNodeVtbl xmlnode_vtbl =
     xmlnode_transformNodeToObject,
 };
 
-IXMLDOMNode *create_node( xmlNodePtr node )
+static HRESULT WINAPI Internal_QueryInterface(
+    IUnknown *iface,
+    REFIID riid,
+    void** ppvObject )
 {
-    xmlnode *This;
+    xmlnode *This = impl_from_InternalUnknown( iface );
 
-    if ( !node )
-        return NULL;
+    TRACE("%p %s %p\n", iface, debugstr_guid(riid), ppvObject);
 
-    assert( node->doc );
 
-    This = HeapAlloc( GetProcessHeap(), 0, sizeof *This );
-    if ( !This )
-        return NULL;
+    if ( IsEqualGUID( riid, &IID_IUnknown ))
+        *ppvObject = iface;
+    else if ( IsEqualGUID( riid, &IID_IDispatch ) ||
+              IsEqualGUID( riid, &IID_IXMLDOMNode ) )
+        *ppvObject = &This->lpVtbl;
+    else
+    {
+        *ppvObject = NULL;
+        return E_NOINTERFACE;
+    }
 
-    if ( node->type == XML_DOCUMENT_NODE )
+    IUnknown_AddRef( (IUnknown*)*ppvObject );
+
+    return S_OK;
+}
+
+static ULONG WINAPI Internal_AddRef(
+                 IUnknown *iface )
+{
+    xmlnode *This = impl_from_InternalUnknown( iface );
+    return InterlockedIncrement( &This->ref );
+}
+
+static ULONG WINAPI Internal_Release(
+    IUnknown *iface )
+{
+    xmlnode *This = impl_from_InternalUnknown( iface );
+    ULONG ref;
+
+    ref = InterlockedDecrement( &This->ref );
+    if ( ref == 0 )
     {
-        assert( node->doc == (xmlDocPtr) node );
-        node->doc->_private = 0;
+        assert( This->node->doc );
+        xmldoc_release( This->node->doc );
+        HeapFree( GetProcessHeap(), 0, This );
     }
 
+    return ref;
+}
+
+static const struct IUnknownVtbl internal_unk_vtbl =
+{
+    Internal_QueryInterface,
+    Internal_AddRef,
+    Internal_Release
+};
+
+IUnknown *create_basic_node( xmlNodePtr node, IUnknown *pUnkOuter )
+{
+    xmlnode *This;
+
+    This = HeapAlloc( GetProcessHeap(), 0, sizeof *This );
+    if ( !This )
+        return NULL;
+
+    if(node)
         xmldoc_add_ref( node->doc );
 
     This->lpVtbl = &xmlnode_vtbl;
+    This->lpInternalUnkVtbl = &internal_unk_vtbl;
+
+    if(pUnkOuter)
+        This->pUnkOuter = pUnkOuter; /* Don't take a ref on outer Unknown */
+    else
+        This->pUnkOuter = (IUnknown *)&This->lpInternalUnkVtbl;
+
     This->ref = 1;
     This->node = node;
 
-    return (IXMLDOMNode*) &This->lpVtbl;
+    return (IUnknown*)&This->lpInternalUnkVtbl;
 }
 
+IXMLDOMNode *create_node( xmlNodePtr node )
+{
+    IUnknown *pUnk;
+    IXMLDOMNode *ret;
+    HRESULT hr;
+
+    if ( !node )
+        return NULL;
+
+    TRACE("type %d\n", node->type);
+    switch(node->type)
+    {
+    case XML_ELEMENT_NODE:
+        pUnk = create_element( node );
+        break;
+    case XML_DOCUMENT_NODE:
+        ERR("shouldn't be here!\n");
+        return NULL;
+    default:
+        FIXME("only creating basic node for type %d\n", node->type);
+        pUnk = create_basic_node( node, NULL );
+    }
+
+    hr = IUnknown_QueryInterface(pUnk, &IID_IXMLDOMNode, (LPVOID*)&ret);
+    IUnknown_Release(pUnk);
+    if(FAILED(hr)) return NULL;
+    return ret;
+}
 #endif

dlls/msxml3/tests/domdoc.c

@@ -72,11 +72,6 @@ static const WCHAR szdl[] = { 'd','l',0 };
 static const WCHAR szlc[] = { 'l','c',0 };
 static const WCHAR szbs[] = { 'b','s',0 };
 
-const GUID CLSID_DOMDocument = 
-    { 0x2933BF90, 0x7B36, 0x11d2, {0xB2,0x0E,0x00,0xC0,0x4F,0x98,0x3E,0x60}};
-const GUID IID_IXMLDOMDocument = 
-    { 0x2933BF81, 0x7B36, 0x11d2, {0xB2,0x0E,0x00,0xC0,0x4F,0x98,0x3E,0x60}};
-
 void test_domdoc( void )
 {
     HRESULT r;
@@ -516,6 +511,7 @@ static void test_refs(void)
     IXMLDOMNode *node = NULL, *node2;
     IXMLDOMNodeList *node_list = NULL;
     LONG ref;
+    IUnknown *unk, *unk2;
 
     r = CoCreateInstance( &CLSID_DOMDocument, NULL, 
         CLSCTX_INPROC_SERVER, &IID_IXMLDOMDocument, (LPVOID*)&doc );
@@ -576,8 +572,21 @@ static void test_refs(void)
     todo_wine {
     ok( ref == 3, "ref %ld\n", ref );
     }
-
     IXMLDOMElement_Release( element );
+
+    /* IUnknown must be unique however we obtain it */
+    r = IXMLDOMElement_QueryInterface( element, &IID_IUnknown, (LPVOID*)&unk );
+    ok( r == S_OK, "rets %08lx\n", r );
+    r = IXMLDOMElement_QueryInterface( element, &IID_IXMLDOMNode, (LPVOID*)&node );
+    ok( r == S_OK, "rets %08lx\n", r );
+    r = IXMLDOMNode_QueryInterface( node, &IID_IUnknown, (LPVOID*)&unk2 );
+    ok( r == S_OK, "rets %08lx\n", r );
+    ok( unk == unk2, "unk %p unk2 %p\n", unk, unk2 );
+
+    IUnknown_Release( unk2 );
+    IUnknown_Release( unk );
+    IXMLDOMNode_Release( node );
+
     IXMLDOMElement_Release( element );
 
 }