[Laszlo-dev] API change proposal: <node>/destroy behavior, when destroying child nodes recursively

[André Bargull]

On 1/29/2009 3:00 PM, P T Withington wrote:
>>>> How do you plan to find those cases?
>>> One way would be to set parent to null and then fix the bugs it reveals.
>>
>> It'd be great to have JS2 getters in this case. So you can generate a 
>> debugger warning in one release and by that give people the chance to 
>> update their sources. And in a next release, you'd apply the real 
>> proposed change. That way user applications continue to work.
> 
> If wishes were horses...
> 
> Given that we don't have setters, what do you propose?

Some passthroughs and runtime-dependent compilation, perhaps?
It'll work everywhere except in DHTML+IE, but that should be sufficient 
for ~99% of all users.

(lfc for swf9 compiles and smokecheck succeeds)
if ($as3) {
   #passthrough {
     var __parent;
     public function set parent (p:*) :void {
       this.__parent = p;
     }
     public function get parent () :* {
       return this.__parent;
     }
   }#
} else {
   var parent;
}

for dhtml/swf8 in LzNode#destroy:
if ($dhtml) {
   if (this.__defineGetter__) {
     this.__defineGetter__("parent", function () {...});
   }
} else if ($swf8) {
   this.addProperty("parent", function () {...}, function (p) {...});
}


>>
>> Adding the same if-condition to "__LZresolveReferences" as in 
>> "__LZapplyArgs" ('bail if deleted'), presumedly helps to reduce some 
>> useless operations when applying constraints to destroyed nodes..
> 
> Yes, but this is really just another band-aid for a problem that I am 
> trying to fix at the source.

But you need to add that if-condition to "__LZresolveReferences", or are 
you going to change the way constraints are compiled? (Because it's not 
possible for users to protect against nullpointer dereferencing in 
constraints, similar to LPP-6594)


> 
>> And the bug is not caused by the replication-manager, it's a bug in 
>> LzView. The view must check in its destroy method the current focus 
>> and if necessary clear focus.
> 
> This really depends on your point of view.  I see it as a bug in the 
> focus manager, or at least a bug by the person who wrote the focus 
> manager, which was added after views but failed to consider that views 
> are allowed to be destroyed.

I'm confused: first you blamed the replication-manager, then I replied 
that it's not about replication but about views. But now you're saying 
it's just the focus-manager. What happened to your point of view about 
the replication-manager?! ;-)


> I am sure there are lots of ways you can demonstrate that my proposal 
> will cause a runtime exception.  My claim is that for each one you 
> demonstrate, when we dig into it, we will find that there is a design 
> flaw at the root of it that at best will cause a memory leak, or worst 
> will cause the program to not behave as expected.

On the other hand, it's also possible that the proposed change will 
cause programs to not behave as expected. That's why I wanted more 
community participation... (hello, someone there?)


> 
> My premise is that when a node is destroyed, the intent is that the node 
> is disconnected from the LZX 'DOM-tree', hence any code that is trying 
> to navigate the DOM starting from that destroyed node is erroneous.  The 
> fact that _currently_ we leave the parent links pointing to what used to 
> be the parent of the node in the tree is broken, because that node is no 
> longer a child of the parent.

That implies another change: When a node is destroyed, you'll also 
remove the parent-pointer. In your original proposal, the parent-pointer 
was just removed for children of destroyed nodes.
I already wrote about that difference on 1/26/2009:
> And the argument concerning memory-leaks isn't really strong in my 
> opinion. When you use a different order of destroy-calls, you can still 
> get a memory leak, because for (2) it's possible to access B.parent 
> although both A and B are destroyed.
> 1) A.destroy(); assert (B.parent == null); // succeeds
> 2) B.destroy(); A.destroy(); assert (B.parent == null); // fails