Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-8065] Implemented checks against laying out destroyed children. #1711

Merged
merged 5 commits into from Mar 18, 2012
Merged

[TIMOB-8065] Implemented checks against laying out destroyed children. #1711

merged 5 commits into from Mar 18, 2012

Conversation

nebrius
Copy link
Contributor

@nebrius nebrius commented Mar 16, 2012

No description provided.

}
this._alive = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure why we need this, but we'll leave it in just in case. I wonder if we should put it before the for loop, so that it certainly would overwrite whatever _alive value was there prevously, then it would get deleted by the for loop. We can revisit later.

@cb1kenobi
Copy link
Contributor

After doing some tests, this solution seems to fix the issue where a View is marked for layout, but then is deleted before the setTimeout fires to do the actual layout.

This method does the check in the layout and requires 1 function call to verifyChild() which has 2 conditional checks per node to layout. Generally, all nodes will be valid.

It might be better for Views to explicitly remove themselves from the nodesToLayout array. Instead of markedForLayout being true/false, it could be -1 if not marked for layout or 0+ for the index in the nodesToLayout array. When a view is remove()'d, it would do a quick !~markedForLayout and if true, set nodesToLayout[markedForLayout] = null.

What is less code? What is more performant?

@nebrius
Copy link
Contributor Author

nebrius commented Mar 18, 2012

I tried removing the nodes from the marked for layout list, but unfortunately that doesn't help. The problem is that these nodes that are not alive but not removed are not in the marked for layout list, but instead are dependent on a node in the list. The dependencies are not calculated until later, after the node has already been destroyed, so the node has no way of knowing if it will be laid out when it is destroyed.

@cb1kenobi
Copy link
Contributor

OK, then that makes good sense.

Code reviewed and tested. Request accepted.

cb1kenobi added a commit that referenced this pull request Mar 18, 2012
[TIMOB-8065] Implemented checks against laying out destroyed children.
@cb1kenobi cb1kenobi merged commit f7a741b into tidev:master Mar 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants