264308 - Implement DOM Level 3 UserData API

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Peter Van der Beken [:peterv]]

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: WIP

May still crash and leak.

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: v1

This doesn't leak. Last remaining problem: we call the handler twice on
importNode (because it does a cloneNode internally). Not sure how to fix it
yet.

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: v2

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'll try to start looking at this this weekend

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v2.1

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 195743 [details] [diff] [review]
v2.1

> #define NS_IDOCUMENT_IID      \
>@@ -778,6 +780,39 @@ public:
>    */
>   virtual void NotifyURIVisitednessChanged(nsIURI* aURI) = 0;
> 
>+  nsresult SetUserData(const nsISupports *aObject, const nsAString &aKey,
>+                       nsIVariant *aData, nsIDOMUserDataHandler *aHandler,
>+                       nsIVariant **aResult)
>+  {
>+    nsCOMPtr<nsIAtom> key = do_GetAtom(aKey);
>+    if (!key) {
>+      return NS_ERROR_OUT_OF_MEMORY;
>+    }
>+
>+    return SetUserData(aObject, key, aData, aHandler, aResult);
>+  }
>+  virtual nsresult SetUserData(const nsISupports *aObject, nsIAtom *aKey,
>+                               nsIVariant *aData,
>+                               nsIDOMUserDataHandler *aHandler,
>+                               nsIVariant **aResult) = 0;
>+  nsresult GetUserData(nsISupports *aObject, const nsAString &aKey,
>+                       nsIVariant **aResult)
>+  {
>+    nsCOMPtr<nsIAtom> key = do_GetAtom(aKey);
>+    if (!key) {
>+      return NS_ERROR_OUT_OF_MEMORY;
>+    }
>+
>+    return GetUserData(aObject, key, aResult);
>+  }
>+  virtual nsresult GetUserData(const nsISupports *aObject, nsIAtom *aKey,
>+                               nsIVariant **aResult) = 0;
>+  virtual void CallUserDataHandler(PRUint16 aOperation,
>+                                   const nsISupports *aObject,
>+                                   nsIDOMNode *aSource, nsIDOMNode *aDest) = 0;
>+  virtual void TransferUserData(const nsISupports *aObject,
>+                                nsIDocument *aDestination) = 0;
>+

Could you add some documentation about these methods.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #6)

> Could you add some documentation about these methods.

http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-getUserData
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-setUserData
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#UserDataHandler

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #7)
> (In reply to comment #6)
> 
> > Could you add some documentation about these methods.
> 
>
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-getUserData
>
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-setUserData
> http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#UserDataHandler
> 

Those links applies to the methods in
http://lxr.mozilla.org/seamonkey/source/dom/public/idl/core/nsIDOM3Node.idl#82
etc. But I'd like to see comments what the methods in nsIDocument are doing
(not nsIDOM3Document, not nsIDOM3Node, but nsIDocument) ;)

Comment 9

[Peter Van der Beken [:peterv]]

Attachment: v2.2

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 199167 [details] [diff] [review]
v2.2

- In CallHandler() (in nsDocument.cpp):

+  nsresult rv = handlerData->mDocument->GetUserData(object, aKey,
+                                                    getter_AddRefs(data));
+  if (NS_SUCCEEDED(rv)) {
+    nsAutoString key;
+    aKey->ToString(key);
+    handler->Handle(handlerData->mOperation, key, data, handlerData->mSource,
+                    handlerData->mDest);

Shouldn't the condition for getting in here be if (data), or if (NS_SUCCEEDED(rv) && data)? Seems like GetUserData() always returns NS_OK, even if there is no data (as it IMO should)...

r+sr=jst

Comment 11

[Peter Van der Beken [:peterv]]

(In reply to comment #10)
> (From update of attachment 199167 [details] [diff] [review] [edit])
> - In CallHandler() (in nsDocument.cpp):
> 
> +  nsresult rv = handlerData->mDocument->GetUserData(object, aKey,
> +                                                    getter_AddRefs(data));
> +  if (NS_SUCCEEDED(rv)) {
> +    nsAutoString key;
> +    aKey->ToString(key);
> +    handler->Handle(handlerData->mOperation, key, data, handlerData->mSource,
> +                    handlerData->mDest);
> 
> Shouldn't the condition for getting in here be if (data), or if
> (NS_SUCCEEDED(rv) && data)? Seems like GetUserData() always returns NS_OK, even
> if there is no data (as it IMO should)...

I made sure that we only set a handler if we've set data, so I added an assertion here instead.
I also changed the IIDs for nsIContent, nsIDocument and nsIDOMUserDataHandler.

Comment 12

[Mark Mentovai]

Caused a crash [@ nsPropertyTable::DeleteAllPropertiesFor] in bug 314075?

Comment 13

[Alex Vincent [:WeirdAl]]

I'd like to have tests written for this.  Examples of setUserData() and getUserData() are not easy to find, even on codesearch.google.com.  A search on lxr shows only glazou's composer code is using it, and he's not setting any UserDataHandler objects in his calls.

Comment 14

[Peter Van der Beken [:peterv]]

There are at least some tests in the W3C DOM testsuite.

Comment 15

[Eric Shepherd [:sheppy]]

I'm not sure how or where to document these; there's very, very little information available about them.  Can someone point me in the direction of where to actually find those tests in the testsuite?  I don't happen to know where it is.

Comment 16

[Peter Van der Beken [:peterv]]

http://dev.w3.org/cvsweb/2001/DOM-Test-Suite/tests/level3/core/ contains some of the raw tests (look for files that contain userdata). I personally don't have the html versions of the tests anymore and we haven't imported the DOM Level 3 Core tests into our testsuite yet. I'll see if I can generate just the UserData ones.

Some spec links:

http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-getUserData
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-setUserData
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#UserDataHandler

Comment 17

[Eric Shepherd [:sheppy]]

Yeah, I'm looking at that Java code now.  I guess I'll sit down and do some HTML/JS code myself for this.

Comment 18

[Nickolay_Ponomarev]

FWIW:
https://developer.mozilla.org/en/DOM/Node.setUserData
https://developer.mozilla.org/en/DOM/Node.getUserData

Didn't proof-read those pages though.

Comment 19

[Florian Scholz (Open Web Docs)]

The following pages are in a good shape with at least some little examples:
https://developer.mozilla.org/en/DOM/UserDataHandler
https://developer.mozilla.org/en/DOM/DOMUserData
https://developer.mozilla.org/en/DOM/Node.getUserData
https://developer.mozilla.org/en/DOM/Node.setUserData

Marking this as completed finally.