Remove necessity to use view to access rangeController in ASTableNode, ASCollectionNode #1103
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maicki
force-pushed
the
MSAccessRangeControllerWithoutViewNeeded
branch
from
004e02b
to
7d860d1
Compare
maicki
force-pushed
the
MSAccessRangeControllerWithoutViewNeeded
branch
2 times, most recently
from
6bf8ee6
to
5c983e6
Compare
|
@nguyenhuy @Adlai-Holler Would appreciate a review. Thanks! |
Adlai-Holler
approved these changes
Sep 18, 2018
| @property (weak, nonatomic) id <ASCollectionDataSource> dataSource; | ||
| @interface _ASCollectionPendingState : NSObject { | ||
| @public | ||
| std::vector<std::vector<ASRangeTuningParameters>> _tuningParameters; |
There was a problem hiding this comment.
Not specific to this diff, but we should probably use ASRangeTuningParameters[ASLayoutRangeModeCount][ASLayoutRangeTypeCount] _tuningParameters here so we get static allocation.
maicki
force-pushed
the
MSAccessRangeControllerWithoutViewNeeded
branch
from
fd7ecae
to
9f605c0
Compare
…, ASCollectionNode
maicki
force-pushed
the
MSAccessRangeControllerWithoutViewNeeded
branch
from
9f605c0
to
f4e4475
Compare
mikezucc
pushed a commit
to mikezucc/Texture
that referenced
this pull request
Oct 2, 2018
…, ASCollectionNode (TextureGroup#1103)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While setting the interface state of a node in certain circumstances we hit the path that first grabs the instance lock and calls
-[ASCollectionNode clearContents]while the view is not loaded. WithinclearContentswe need access to the rangeController from the view by calling self.view.rangeController what will trigger creating the view and the assert linked in the issue will be thrown asdidLoadshould not be called with the instance locked held.For now I removed the need to use the view to access the
rangeControllerwhat should fix the situation described above and linked here: #1094Further todos should be that we improve the locking impelementation around this area. Currently
-[ASDisplayNode clearContents]is accessing instance variables and therefore should be prefixed withlocked_, but-[ASCollectionNode clearContents]is accessing the rangeController by calling self instead of using the instance variable. The follow up issue is here: #1104