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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-20396] Fix main-thread issues in callbacks #7847

Merged
merged 2 commits into from Apr 5, 2016
Merged

[TIMOB-20396] Fix main-thread issues in callbacks #7847

merged 2 commits into from Apr 5, 2016

Conversation

hansemannn
Copy link
Collaborator

@jhaynie
Copy link
Contributor

jhaynie commented Mar 15, 2016

curious why we aren't using GCD and are using threads instead here? wouldn't GCD be cleaner code and more performant?

@pec1985
Copy link
Contributor

pec1985 commented Mar 15, 2016

The [NSThread detachNewThreadSelector... is legacy code that needs to be removed once Kroll Thread is removed. If we're running on the main thread, we do not need to call this method on a new thread or async, since the callback and event system takes care of that in a different place.
We're placing an ifdef and introducing new code if on main thread.

@hansemannn
Copy link
Collaborator Author

Yes, everything in #ifdef TI_USE_KROLL_THREAD is legacy for people who don't want to use the main-thread in Ti.SDK >= 5.4.0. The delay in [self dispatchCallback:..] is necessary to fire the event after the modal transition finishes.

@pec1985
Copy link
Contributor

pec1985 commented Mar 16, 2016

@hansemannn there are some conflicts here. I will merge it once those conflicts are resolved

Merge branch 'master' of github.com:appcelerator/titanium_mobile into…
0103f6f 
… TIMOB-20396

# Conflicts:
#	iphone/Classes/MediaModule.m
@hansemannn
Copy link
Collaborator Author

Updated yesterday

@pec1985
Copy link
Contributor

pec1985 commented Apr 5, 2016

PR reviewed. APPROVED!

@pec1985 pec1985 merged commit 91d3369 into tidev:master Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hansemannn @jhaynie @pec1985