896103 - NodeListActor's items() response has unnecessary nesting

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Heather Arthur [:harth]]

Response from an items() request:

{"nodes":{"nodes":[{"actor":"conn3.domnode16","parent":"conn3.domnode19","nodeType":1,"nodeName":"DIV","numChildren":0,"attrs":[{"name":"class","value":"item"},{"name":"id","value":"test1"}]},{"actor":"conn3.domnode17","parent":"conn3.domnode19","nodeType":1,"nodeName":"DIV","numChildren":5,"attrs":[{"name":"class","value":"item"},{"name":"id","value":"test2"}]},{"actor":"conn3.domnode18","parent":"conn3.domnode19","nodeType":1,"nodeName":"DIV","numChildren":0,"attrs":[{"name":"class","value":"item"},{"name":"id","value":"test3"}]}],"newNodes":[{"actor":"conn3.domnode19","parent":"conn3.domnode20","nodeType":1,"nodeName":"SECTION","numChildren":7,"attrs":[{"name":"id","value":"test-section"}]},{"actor":"conn3.domnode20","parent":"conn3.domnode21","nodeType":1,"nodeName":"MAIN","numChildren":3,"attrs":[]},{"actor":"conn3.domnode21","parent":"conn3.domnode22","nodeType":1,"nodeName":"BODY","numChildren":3,"attrs":[]},{"actor":"conn3.domnode22","parent":"conn3.domnode13","nodeType":1,"nodeName":"HTML","numChildren":3,"attrs":[],"isDocumentElement":true}]},"from":"conn3.domnodelist15"}

Notice the nodes.nodes and nodes.newNodes. Seems like it could just be nodes and newNodes.

I wish we had a single field for the return value across all the actors, like "result"

What is newNodes for?

Comment 1

[Dave Camp (:dcamp)]

Yeah, we should get rid of the useless extra nesting.

`nodes` is an array of nodes that answer the question being asked (like the querySelector nodes).

To make things sane for the client, we make sure that when you have a node reference on the client side, you also have a reference to all of its parents (so that parentNode can be sync).  newNodes fills in any parents that the client doesn't know about yet.

Comment 2

[Heather Arthur [:harth]]

(In reply to Dave Camp (:dcamp) from comment #1)
> To make things sane for the client, we make sure that when you have a node
> reference on the client side, you also have a reference to all of its
> parents (so that parentNode can be sync).  newNodes fills in any parents
> that the client doesn't know about yet.

Ah, maybe newNodes should be named something else?

I'm curious about this use case. Why you'd need it to be sync over other things like getting children that are async?

Comment 3

[Dave Camp (:dcamp)]

(In reply to Heather Arthur [:harth] from comment #2)
> Ah, maybe newNodes should be named something else?

Yeah, any suggestions?

> I'm curious about this use case. Why you'd need it to be sync over other
> things like getting children that are async?

It's mostly related to the algorithm we use to release memory for disconnected nodes - when a node is disconnected from the dom tree we want to be able to free the client objects for all the children nodes.

So to be able to answer "all the children of a given node that we have seen on the client side", we guarantee that every time we've seen a node, we connect it up through its parents.

Clients don't need to implement the collection algorithm.  In that case they're free to ignore the newNodes (maybe newParents? unseenParents?).

I'll submit a patch that puts this documentation in the file somewhere.

Comment 4

[Dave Camp (:dcamp)]

Attachment: v1`

Comment 5

[Heather Arthur [:harth]]

Comment on attachment 778866 [details] [diff] [review]
v1`

Review of attachment 778866 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, I see.

Looks good, thanks!

Comment 6

[Dave Camp (:dcamp)]

https://hg.mozilla.org/integration/fx-team/rev/05ff12c00114

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/05ff12c00114