Nikolay Sivov : msxml3: Fix reference counting behaviour for named map.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 68cc66d8000bb7b0acd13986b54a95155b434790
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=68cc66d8000bb7b0acd13986b5...

Author: Nikolay Sivov <nsivov(a)codeweavers.com>
Date:   Thu Mar 10 21:56:51 2011 +0300

msxml3: Fix reference counting behaviour for named map.

---

 dlls/msxml3/element.c       |    6 ++--
 dlls/msxml3/msxml_private.h |    2 +-
 dlls/msxml3/nodemap.c       |   46 ++++++++++--------------------------------
 dlls/msxml3/tests/domdoc.c  |   45 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c
index 8d81f67..4c3a9b3 100644
--- a/dlls/msxml3/element.c
+++ b/dlls/msxml3/element.c
@@ -314,13 +314,13 @@ static HRESULT WINAPI domelem_get_nextSibling(
 
 static HRESULT WINAPI domelem_get_attributes(
     IXMLDOMElement *iface,
-    IXMLDOMNamedNodeMap** attributeMap)
+    IXMLDOMNamedNodeMap** map)
 {
     domelem *This = impl_from_IXMLDOMElement( iface );
 
-    TRACE("(%p)->(%p)\n", This, attributeMap);
+    TRACE("(%p)->(%p)\n", This, map);
 
-    *attributeMap = create_nodemap((IXMLDOMNode*)&This->IXMLDOMElement_iface);
+    *map = create_nodemap(This->node.node);
     return S_OK;
 }
 
diff --git a/dlls/msxml3/msxml_private.h b/dlls/msxml3/msxml_private.h
index 72de7ba..4e3b35b 100644
--- a/dlls/msxml3/msxml_private.h
+++ b/dlls/msxml3/msxml_private.h
@@ -186,7 +186,7 @@ extern IUnknown         *create_pi( xmlNodePtr pi );
 extern IUnknown         *create_comment( xmlNodePtr comment );
 extern IUnknown         *create_cdata( xmlNodePtr text );
 extern IXMLDOMNodeList  *create_children_nodelist( xmlNodePtr );
-extern IXMLDOMNamedNodeMap *create_nodemap( IXMLDOMNode *node );
+extern IXMLDOMNamedNodeMap *create_nodemap( const xmlNodePtr );
 extern IUnknown         *create_doc_Implementation(void);
 extern IUnknown         *create_doc_fragment( xmlNodePtr fragment );
 extern IUnknown         *create_doc_entity_ref( xmlNodePtr entity );
diff --git a/dlls/msxml3/nodemap.c b/dlls/msxml3/nodemap.c
index 660eab6..20475ca 100644
--- a/dlls/msxml3/nodemap.c
+++ b/dlls/msxml3/nodemap.c
@@ -48,7 +48,8 @@ typedef struct _xmlnodemap
     IXMLDOMNamedNodeMap IXMLDOMNamedNodeMap_iface;
     ISupportErrorInfo ISupportErrorInfo_iface;
     LONG ref;
-    IXMLDOMNode *node;
+
+    xmlNodePtr node;
     LONG iterator;
 } xmlnodemap;
 
@@ -109,7 +110,7 @@ static ULONG WINAPI xmlnodemap_Release(
     TRACE("(%p)->(%d)\n", This, ref);
     if ( ref == 0 )
     {
-        IXMLDOMNode_Release( This->node );
+        xmldoc_release( This->node->doc );
         heap_free( This );
     }
 
@@ -217,7 +218,6 @@ static HRESULT WINAPI xmlnodemap_setNamedItem(
 {
     xmlnodemap *This = impl_from_IXMLDOMNamedNodeMap( iface );
     xmlNodePtr nodeNew;
-    xmlNodePtr node;
     xmlnode *ThisNew;
 
     TRACE("(%p)->(%p %p)\n", This, newItem, namedItem );
@@ -227,10 +227,6 @@ static HRESULT WINAPI xmlnodemap_setNamedItem(
 
     if(namedItem) *namedItem = NULL;
 
-    node = xmlNodePtr_from_domnode( This->node, 0 );
-    if ( !node )
-        return E_FAIL;
-
     /* Must be an Attribute */
     ThisNew = get_node_obj( newItem );
     if(!ThisNew) return E_FAIL;
@@ -242,7 +238,7 @@ static HRESULT WINAPI xmlnodemap_setNamedItem(
         if(xmldoc_remove_orphan(ThisNew->node->doc, ThisNew->node) != S_OK)
             WARN("%p is not an orphan of %p\n", ThisNew->node, ThisNew->node->doc);
 
-    nodeNew = xmlAddChild(node, ThisNew->node);
+    nodeNew = xmlAddChild(This->node, ThisNew->node);
 
     if(namedItem)
         *namedItem = create_node( nodeNew );
@@ -265,7 +261,6 @@ static HRESULT WINAPI xmlnodemap_get_item(
     IXMLDOMNode** listItem)
 {
     xmlnodemap *This = impl_from_IXMLDOMNamedNodeMap( iface );
-    xmlNodePtr node;
     xmlAttrPtr curr;
     LONG attrIndex;
 
@@ -276,8 +271,7 @@ static HRESULT WINAPI xmlnodemap_get_item(
     if (index < 0)
         return S_FALSE;
 
-    node = xmlNodePtr_from_domnode( This->node, 0 );
-    curr = node->properties;
+    curr = This->node->properties;
 
     for (attrIndex = 0; attrIndex < index; attrIndex++) {
         if (curr->next == NULL)
@@ -295,7 +289,6 @@ static HRESULT WINAPI xmlnodemap_get_length(
     IXMLDOMNamedNodeMap *iface,
     LONG *listLength)
 {
-    xmlNodePtr node;
     xmlAttrPtr first;
     xmlAttrPtr curr;
     LONG attrCount;
@@ -307,11 +300,7 @@ static HRESULT WINAPI xmlnodemap_get_length(
     if( !listLength )
         return E_INVALIDARG;
 
-    node = xmlNodePtr_from_domnode( This->node, 0 );
-    if ( !node )
-        return E_FAIL;
-
-    first = node->properties;
+    first = This->node->properties;
     if (first == NULL) {
 	*listLength = 0;
 	return S_OK;
@@ -336,7 +325,6 @@ static HRESULT WINAPI xmlnodemap_getQualifiedItem(
 {
     xmlnodemap *This = impl_from_IXMLDOMNamedNodeMap( iface );
     xmlAttrPtr attr;
-    xmlNodePtr node;
     xmlChar *href;
     xmlChar *name;
 
@@ -344,10 +332,6 @@ static HRESULT WINAPI xmlnodemap_getQualifiedItem(
 
     if (!baseName || !qualifiedItem) return E_INVALIDARG;
 
-    node = xmlNodePtr_from_domnode(This->node, XML_ELEMENT_NODE);
-    if ( !node )
-        return E_FAIL;
-
     if (namespaceURI && *namespaceURI)
     {
         href = xmlChar_from_wchar(namespaceURI);
@@ -363,7 +347,7 @@ static HRESULT WINAPI xmlnodemap_getQualifiedItem(
         return E_OUTOFMEMORY;
     }
 
-    attr = xmlHasNsProp(node, name, href);
+    attr = xmlHasNsProp(This->node, name, href);
 
     heap_free(name);
     heap_free(href);
@@ -387,7 +371,6 @@ static HRESULT WINAPI xmlnodemap_removeQualifiedItem(
 {
     xmlnodemap *This = impl_from_IXMLDOMNamedNodeMap( iface );
     xmlAttrPtr attr;
-    xmlNodePtr node;
     xmlChar *name;
     xmlChar *href;
 
@@ -395,10 +378,6 @@ static HRESULT WINAPI xmlnodemap_removeQualifiedItem(
 
     if (!baseName) return E_INVALIDARG;
 
-    node = xmlNodePtr_from_domnode( This->node, XML_ELEMENT_NODE );
-    if ( !node )
-        return E_FAIL;
-
     if (namespaceURI && *namespaceURI)
     {
         href = xmlChar_from_wchar(namespaceURI);
@@ -414,7 +393,7 @@ static HRESULT WINAPI xmlnodemap_removeQualifiedItem(
         return E_OUTOFMEMORY;
     }
 
-    attr = xmlHasNsProp( node, name, href );
+    attr = xmlHasNsProp( This->node, name, href );
 
     heap_free(name);
     heap_free(href);
@@ -445,7 +424,6 @@ static HRESULT WINAPI xmlnodemap_nextNode(
     IXMLDOMNode** nextItem)
 {
     xmlnodemap *This = impl_from_IXMLDOMNamedNodeMap( iface );
-    xmlNodePtr node;
     xmlAttrPtr curr;
     LONG attrIndex;
 
@@ -453,8 +431,7 @@ static HRESULT WINAPI xmlnodemap_nextNode(
 
     *nextItem = NULL;
 
-    node = xmlNodePtr_from_domnode( This->node, 0 );
-    curr = node->properties;
+    curr = This->node->properties;
 
     for (attrIndex = 0; attrIndex < This->iterator; attrIndex++) {
         if (curr->next == NULL)
@@ -551,7 +528,7 @@ static const struct ISupportErrorInfoVtbl support_error_vtbl =
     support_error_InterfaceSupportsErrorInfo
 };
 
-IXMLDOMNamedNodeMap *create_nodemap( IXMLDOMNode *node )
+IXMLDOMNamedNodeMap *create_nodemap( const xmlNodePtr node )
 {
     xmlnodemap *nodemap;
 
@@ -565,8 +542,7 @@ IXMLDOMNamedNodeMap *create_nodemap( IXMLDOMNode *node )
     nodemap->ref = 1;
     nodemap->iterator = 0;
 
-    IXMLDOMNode_AddRef( node );
-    /* Since we AddRef a node here, we don't need to call xmldoc_add_ref() */
+    xmldoc_add_ref(node->doc);
 
     return &nodemap->IXMLDOMNamedNodeMap_iface;
 }
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c
index 42fd22a..370c56e 100644
--- a/dlls/msxml3/tests/domdoc.c
+++ b/dlls/msxml3/tests/domdoc.c
@@ -8329,6 +8329,7 @@ static void test_get_nodeTypeString(void)
         ok(!lstrcmpW(str, _bstr_(entry->string)), "got string %s, expected %s. node type %d\n",
             wine_dbgstr_w(str), entry->string, entry->type);
         SysFreeString(str);
+        IXMLDOMNode_Release(node);
 
         entry++;
     }
@@ -8358,14 +8359,15 @@ static void test_get_attributes(void)
     const get_attributes_t *entry = get_attributes;
     IXMLDOMNamedNodeMap *map;
     IXMLDOMDocument *doc;
-    IXMLDOMNode *node;
+    IXMLDOMNode *node, *node2;
     VARIANT_BOOL b;
     HRESULT hr;
     BSTR str;
+    LONG length;
 
     doc = create_document(&IID_IXMLDOMDocument);
 
-    str = SysAllocString( szComplete3 );
+    str = SysAllocString( szComplete4 );
     hr = IXMLDOMDocument_loadXML(doc, str, &b);
     SysFreeString(str);
 
@@ -8403,6 +8405,43 @@ static void test_get_attributes(void)
 
     IXMLDOMNode_Release(node);
 
+    /* last child is element */
+    EXPECT_REF(doc, 1);
+    hr = IXMLDOMDocument_get_lastChild(doc, &node);
+    ok(hr == S_OK, "got %08x\n", hr);
+    EXPECT_REF(doc, 1);
+
+    EXPECT_REF(node, 1);
+    hr = IXMLDOMNode_get_attributes(node, &map);
+    ok(hr == S_OK, "got %08x\n", hr);
+    EXPECT_REF(node, 1);
+    EXPECT_REF(doc, 1);
+
+    EXPECT_REF(map, 1);
+    hr = IXMLDOMNamedNodeMap_get_item(map, 0, &node2);
+    ok(hr == S_OK, "got %08x\n", hr);
+    EXPECT_REF(node, 1);
+    EXPECT_REF(node2, 1);
+    EXPECT_REF(map, 1);
+    EXPECT_REF(doc, 1);
+    IXMLDOMNode_Release(node2);
+
+    /* release node before map release, map still works */
+    IXMLDOMNode_Release(node);
+
+    length = 0;
+    hr = IXMLDOMNamedNodeMap_get_length(map, &length);
+    ok(hr == S_OK, "got %08x\n", hr);
+    ok(length == 1, "got %d\n", length);
+
+    node2 = NULL;
+    hr = IXMLDOMNamedNodeMap_get_item(map, 0, &node2);
+    ok(hr == S_OK, "got %08x\n", hr);
+    EXPECT_REF(node2, 1);
+    IXMLDOMNode_Release(node2);
+
+    IXMLDOMNamedNodeMap_Release(map);
+
     while (entry->type)
     {
         VARIANT var;
@@ -8423,6 +8462,8 @@ static void test_get_attributes(void)
             hr, entry->hr, entry->type);
         ok(map == NULL, "got %p\n", map);
 
+        IXMLDOMNode_Release(node);
+
         entry++;
     }