Changing the Check State from InitNode may leave the tree in "updating" state

[obones]

Following work on issue #820, a change to the CheckState of a given node triggers is a BeginUpdate/EndUpdate call pair to avoid slow propagation to subnodes.
One can set the CheckState from the OnInitNode event handler to have a default state for a given node.
Also, one can access TotalCount which inits all the nodes to get its value. It does so within a try..finally block that directly changes the value of FUpdateCount
This means that if one is to change CheckState from OnInitNode and that the latter is called because of an access to TotalCount, then the (newly) added BeginUpdate/EndUpdate pair sees a value of FUpdateCount that is higher than 0. This has not much impact on BeginUpdate but this prevents EndUpdate from removing the tsUpdating flag from FStates
And once this flag is set, the tree view thinks it is still updating and does not update properly anymore.

This is what the following test application shows: VTVCheckStateInInitNode.zip

Looking at the history of code, it appears GetTotalCount has been doing this direct change of FUpdateCount forever so there must have been a reason not to use BeginUpdate/EndUpdate but I can't figure out why. I mean, even tsUpdating has been available forever.

In my real world application, I was able to avoid reading TotalCount, but my initial change proposal was for GetTotalCount to use BeginUpdate/EndUpdate instead of directly modifying FUpdateCount

What's worrying me, though, is that there are other places where FUpdateCount is changed directly, which means the change introduced by #820 is likely to trigger the same issue with those ones too. Here is the list of those I found:

• GetTotalCount
• DeleteChildren
• SortTree

The last one has a clear explanation as to why it avoids EndUpdate and this leads me to believe that maybe GetTotalCount should be left alone and it's the code inside ChangeCheckState that should do like the three others and directly change the value of FUpdateCount instead of calling BeginUpdate/EndUpdate

What do you think?

[joachimmarder]

I mean, even tsUpdating has been available forever.

I wondered more than once: Why is tsUpdating needed at all? Shouldn't it be equal to FUpdateCount > 0, which we could extract into an IsUpdating method? That would fix the scrollbar issue you reported.

Looking at the history of code, it appears GetTotalCount has been doing this direct change of FUpdateCount forever so there must have been a reason not to use BeginUpdate/EndUpdate but I can't figure out why.

I can only guess. Maybe the idea was: EndUpdate() may involve some costly operations that are unnecessary if just the total node count is queried and the node structure hasn't been changed.