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

[ASTextKitComponents] Make sure Main Thread Checker isn't triggered during background calculations #trivial #612

Merged

Conversation

nguyenhuy
Copy link
Member

@nguyenhuy nguyenhuy commented Oct 11, 2017

Currently the checker is triggered either when the components need to get the backing text view's bounds, or when the text view's delegate is cleared out during deallocation.

In the first case, I add a new subclass of UITextView that has a thread-safe bounds property and is used exclusively by ASTextKitComponents.

The second case is handled in #598. However, a new main thread assertion added in #603 causes background deallocations of temporary components to fail. #610 attempts to fix that, but since temporary components don't have a reference to a text view, it's fine to avoid the assertion altogether.

… upsetting Main Thread Checker

- Add a new thread-safe text view bounds.
- Temporary components stack doesn't have a text view so it can be safely deallocated off main.
@nguyenhuy nguyenhuy changed the title [ASTextKitComponents] Make sure Main Thread Checker isn't triggered during background calculations [ASTextKitComponents] Make sure Main Thread Checker isn't triggered during background calculations #trivial Oct 11, 2017
@ghost
Copy link

ghost commented Oct 11, 2017

🚫 CI failed with log

Copy link
Contributor

@maicki maicki left a comment

Choose a reason for hiding this comment

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

LGTM


#import <tgmath.h>

@interface ASTextKitComponentsTextView ()
@property (nonatomic, assign) CGRect threadSafeBounds;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be atomic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point!

@nguyenhuy nguyenhuy merged commit c12509e into TextureGroup:master Oct 17, 2017
@nguyenhuy nguyenhuy deleted the HNThreadSafeTextKitComponents branch October 17, 2017 13:20
bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…uring background calculations #trivial (TextureGroup#612)

* Add unit test

* Make sure TextKit components can calculate size in background without upsetting Main Thread Checker
- Add a new thread-safe text view bounds.
- Temporary components stack doesn't have a text view so it can be safely deallocated off main.

* Add ASTextKitComponentsTextView

* Remove unnecessary change

* Fix minor mistake

* ASTextKitComponentsTextView has only 1 initializer

* Minor change

* Switch to atomic property

* Remove manual synthesization
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.

2 participants