Make inactive nodes mark their children as dirty on setInactive

[gilberto-torrezan]

This moves the setting of dirty nodes out of the collectChanges method.

When using long-polling push, the client keeps asking for changes from
the server. The server then collects all the changes and sends to the
client. The problem is: a child node is always considered dirty, if
its parent is invisible, and the node is always included in the changes
sent to the client.

Since there's not a single moment where there's nothing to send, the
server keeps sending the changes, and the client keeps asking for the
changes. This creates a busy loop between client and server, which only
ends when the parent node is made visible again.

To prevent this, this patch changes this logic to make the children
nodes only dirty when the parent visibility changes.

[denis-anisimov]

Looks OK but several comments:

-Previously the code has been invoked when needed: if the node has been marked as dirty then it has been remarked again: As a result ONLY dirty nodes participated in the related functionality.

• Now every change in active state of the parent node causes the traversal of the whole subtree and every descendant node becomes dirty even if it had no any change at all. This obviously affect performance in a bad way. I'm not sure how serious it is: changes are not sent to the client if there is nothing to send anyway ( so excessive dirtyness doesn't create extra server->client communication). But this affects server side (may be acceptable)
• I don't see how exactly the previous state of the code causes issues. As a result nothing prevents to reintroduce similar code back which returns back the issue you are trying to fix.

So at least I would suggest to make some test which checks that the issue doesn't reappear back: it will also help me to understand what was the original problem with changing dirty flag in the collectChanges method.

[gilberto-torrezan]

Have added a test based on the code submitted in the ticket, that reproduces the bug.

[denis-anisimov]

I'm not sure whether this is feasible but generic test for no dirty flag is set inside collectChanges would be good.

Though it's not possible to make such test for every possible case: node should not modify dirty flag not only for itself but may be for any other node......

So may be IT test for this specific case is enough..